Fix deprecated, missing error and type cast lint checks #413
Conversation
There was a problem hiding this comment.
Btw, we have a make target make lint that runs the default golangci-lint checkers.
I don't think your changes here resolve 100% everything there, but if you want to add linting to the Github workflows, that could also be something we enforce continually.
| @@ -371,12 +371,6 @@ func (r *collectionReconciler) makeCollectorConfig(ctx context.Context, spec *mo | |||
| cfgs, err := pmon.ScrapeConfigs(projectID, location, cluster) | |||
| if err != nil { | |||
| msg := "generating scrape config failed for PodMonitoring endpoint" | |||
| cond = &monitoringv1.MonitoringCondition{ | |||
There was a problem hiding this comment.
I actually think this is a bug. We never update the status on a bad config.
Can you add a TODO to fix with your deletion?
// TODO: surface ScrapeConfigError to PodMonitoring status here.| @@ -410,12 +404,6 @@ func (r *collectionReconciler) makeCollectorConfig(ctx context.Context, spec *mo | |||
| cfgs, err := cmon.ScrapeConfigs(projectID, location, cluster) | |||
| if err != nil { | |||
| msg := "generating scrape config failed for PodMonitoring endpoint" | |||
| cond = &monitoringv1.MonitoringCondition{ | |||
There was a problem hiding this comment.
Same with this, this should be surfaced to users instead of nacked.
pkg/operator/e2e/context.go
Outdated
| @@ -256,23 +262,20 @@ func (tctx *testContext) createBaseResources(ctx context.Context) ([]metav1.Owne | |||
| if err != nil { | |||
| return nil, errors.Wrap(err, "deserializing alertmanager manifest") | |||
| } | |||
| switch obj.(type) { | |||
| switch obj := obj.(type) { | |||
| var podMonitorings monitoringv1.PodMonitoringList | ||
| kubeClient.List(ctx, &podMonitorings) | ||
| if err := kubeClient.List(ctx, &podMonitorings); err != nil { | ||
| return false, nil |
There was a problem hiding this comment.
nit: By returning nil here, we're effectively nacking any errors when doing the list. Is that the desired behavior? Or should we error and exit immediately in the test?
TheSpiritXIII
force-pushed
the
TheSpiritXIII/lint-check
branch
from
cf2d487
to
f481348
Compare
TheSpiritXIII
force-pushed
the
TheSpiritXIII/lint-check
branch
from
f481348
to
8770d0b
Compare
This fixes all lints warnings in the operator package and a few in other packages.