TLDR: golangci-lint complains about one thing which can’t possibly be true but really something else completely different is wrong.

 

 

Joy of joys, I got to spend my entire Monday merging in dozens of dependabot PRs across 2 repos.

As much as I tried to cheat and use various toolings to smoosh them all together into just a couple of PRs, alas there were enough significant bumps in things that they required actual manual intervention and, you know, thought.

I hate thinking on a Monday.

I especially hate something happening which I know I have seen before but cannot place it, so I have to think hard about something which I am pretty sure I have already thought hard about before and I am annoyed that I am paying this price twice.

Anyway, so today’s rant is brought to you by golangci-lint. Bless its heart, it does its best, but sometimes it just gets confused, and it reacts to that by confusing everyone around it.

In one particular repo dependabot wanted to bump go-logr/logr from 0.4.0 to 1.2.3. So yeh, with a bump that big naturally breaking changes were inevitable.

I do the bump, make the changes which are obvious, and run make lint.

$ make lint
hack/tools/bin/golangci-lint run -v --fast=false
INFO [config_reader] Config search paths: [./ /home/claudia/workspace/cluster-api-provider-microvm /home/claudia/workspace /home/claudia /home /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 70 linters: [asciicheck bodyclose cyclop deadcode depguard dogsled dupl durationcheck errcheck errorlint exhaustive exportloopref forbidigo forcetypeassert funlen gci gochecknoglobals gochecknoinits gocognit goconst gocritic gocyclo godot godox goerr113 gofmt gofumpt goheader goimports gomnd gomoddirectives gomodguard goprintffuncname gosec gosimple govet ifshort importas ineffassign lll makezero misspell nakedret nestif nilerr nlreturn noctx nolintlint paralleltest prealloc predeclared promlinter revive rowserrcheck sqlclosecheck staticcheck structcheck stylecheck testpackage thelper tparallel typecheck unconvert unparam unused varcheck wastedassign whitespace wrapcheck wsl]
INFO [loader] Go packages loading at mode 575 (compiled_files|files|imports|name|types_sizes|deps|exports_file) took 337.351663ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 1.370246ms
INFO [linters context] importas settings found, but no aliases listed. List aliases under alias: key.
WARN [linters context] running gomoddirectives failed: failed to get module file: command go list: exit status 1: go list -f cannot be used with -json: if you are not using go modules it is suggested to disable this linter
INFO [linters context/goanalysis] analyzers took 408.455454ms with top 10 stages: the_only_name: 61.427431ms, buildir: 15.006766ms, buildssa: 14.172397ms, SA2003: 11.678665ms, dupl: 7.537806ms, gci: 7.223583ms, gocyclo: 7.205052ms, godot: 7.076175ms, inspect: 5.398899ms, nolintlint: 5.189287ms
INFO [runner] Issues before processing: 1745, after processing: 6
INFO [runner] Processors filtering stat (out/in): path_prettifier: 1745/1745, skip_files: 1745/1745, skip_dirs: 1745/1745, max_same_issues: 6/6, filename_unadjuster: 1745/1745, identifier_marker: 1745/1745, diff: 6/6, path_shortener: 6/6, path_prefixer: 6/6, cgo: 1745/1745, nolint: 1736/1736, uniq_by_line: 6/1736, source_code: 6/6, autogenerated_exclude: 1745/1745, exclude: 1745/1745, exclude-rules: 1736/1745, max_per_file_from_linter: 6/6, max_from_linter: 6/6, severity-rules: 6/6, sort_results: 6/6
INFO [runner] processing took 89.084262ms with stages: exclude-rules: 62.517085ms, identifier_marker: 23.620578ms, nolint: 1.547657ms, path_prettifier: 766.437µs, skip_dirs: 179.739µs, autogenerated_exclude: 126.787µs, cgo: 114.394µs, filename_unadjuster: 80.475µs, uniq_by_line: 66.164µs, source_code: 59.341µs, path_shortener: 1.481µs, max_same_issues: 1.247µs, max_per_file_from_linter: 1.131µs, skip_files: 369ns, diff: 290ns, severity-rules: 289ns, max_from_linter: 270ns, exclude: 244ns, sort_results: 205ns, path_prefixer: 79ns
INFO [runner] linters took 1.869317959s with stages: goanalysis_metalinter: 1.78015255s
api/v1alpha1/microvmcluster_webhook.go:45:32: r.GroupVersionKind undefined (type *MicrovmCluster has no field or method GroupVersionKind) (typecheck)
        return apierrors.NewInvalid(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
                                      ^
api/v1alpha1/microvmmachine_webhook.go:47:48: r.Name undefined (type *MicrovmMachine has no field or method Name) (typecheck)
        machineLog.Info("validate upadate", "name", r.Name)
                                                      ^
api/v1alpha1/microvmmachine_webhook.go:57:30: r.GroupVersionKind undefined (type *MicrovmMachine has no field or method GroupVersionKind) (typecheck)
        return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
                                    ^
controllers/microvmcluster_controller.go:63:11: r.Get undefined (type *MicrovmClusterReconciler has no field or method Get) (typecheck)
        err := r.Get(ctx, req.NamespacedName, mvmCluster)
                 ^
controllers/microvmcluster_controller.go:168:2: clusterKey declared but not used (typecheck)
        clusterKey := client.ObjectKey{
        ^
controllers/microvmmachine_controller.go:65:14: r.Get undefined (type *MicrovmMachineReconciler has no field or method Get) (typecheck)
        if err := r.Get(ctx, req.NamespacedName, mvmMachine); err != nil {
                    ^
INFO File cache stats: 6 entries of total size 28.8KiB
INFO Memory: 24 samples, avg is 207.8MB, max is 289.8MB
INFO Execution took 2.212200133s
make: *** [Makefile:70: lint] Error 1

“Okay so those funcs no longer exist?”, I think and go in to change them. Except they totally do exist. For example this error:

controllers/microvmcluster_controller.go:168:2: clusterKey declared but not used (typecheck)
        clusterKey := client.ObjectKey{
        ^

clusterKey is totally in use, literally on the next line:

alt text

alt text

After a few minutes of making unrelated changes and running the same thing again and again hoping it would return a different result (maybe if I snuck up on it?), I started to feel déjà vu.

It came to me in the end. golangci-lint was trying to lint a thing which would not compile, but it didn’t know that it could not compile, so it did the best it could with what it had. Garbage basically.

I found the correct make target to build the thing and saw that I was right:

$ make managers
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags "-X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.buildDate=2022-05-23T14:30:27Z' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitCommit=caa8d3b638c50797b38283c0ec7ae7abf49742c2' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitTreeState=dirty' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitMajor=0' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitMinor=5' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitVersion=v0.5.0-5-caa8d3b638c507-dirty' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.buildDate=2022-05-23T14:30:27Z' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitCommit=caa8d3b638c50797b38283c0ec7ae7abf49742c2' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitTreeState=dirty' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitMajor=0' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitMinor=5' -X 'github.com/weaveworks-liquidmetal/cluster-api-provider-microvm/version.gitVersion=v0.5.0-5-caa8d3b638c507-dirty' -extldflags '-static'" -o bin/manager .
# sigs.k8s.io/controller-runtime/pkg/log
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/deleg.go:79:12: undefined: logr.WithCallDepth
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/deleg.go:163:2: cannot use res (type *DelegatingLogger) as type logr.Logger in return argument
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/deleg.go:179:2: cannot use res (type *DelegatingLogger) as type logr.Logger in return argument
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/deleg.go:195:2: cannot use res (type *DelegatingLogger) as type logr.Logger in return argument
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/log.go:67:26: cannot use NullLogger{} (type NullLogger) as type logr.Logger in argument to Log.Fulfill
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/log.go:82:41: cannot use NullLogger{} (type NullLogger) as type logr.Logger in argument to NewDelegatingLogger
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/log.go:86:6: cannot use Log (type *DelegatingLogger) as type logr.Logger in assignment
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/log.go:88:13: assignment mismatch: 1 variable but logr.FromContext returns 2 values
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/null.go:30:5: cannot use NullLogger{} (type NullLogger) as type logr.Logger in assignment
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/null.go:49:2: cannot use log (type NullLogger) as type logr.Logger in return argument
../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.10.3/pkg/log/null.go:49:2: too many errors
# k8s.io/klog/v2
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:705:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:724:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:742:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:763:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:782:11: invalid operation: loggr != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:783:3: undefined: logr.WithCallDepth
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:794:11: invalid operation: loggr != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:795:3: undefined: logr.WithCallDepth
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:915:9: invalid operation: log != nil (mismatched types logr.Logger and nil)
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:919:4: undefined: logr.WithCallDepth
../../go/pkg/mod/k8s.io/klog/v2@v2.9.0/klog.go:919:4: too many errors
# k8s.io/klog/klogr
../../go/pkg/mod/k8s.io/klog@v1.0.0/klogr/klogr.go:18:16: cannot use klogger{...} (type klogger) as type logr.Logger in return argument
../../go/pkg/mod/k8s.io/klog@v1.0.0/klogr/klogr.go:168:31: undefined: logr.InfoLogger
../../go/pkg/mod/k8s.io/klog@v1.0.0/klogr/klogr.go:183:2: cannot use new (type klogger) as type logr.Logger in return argument
../../go/pkg/mod/k8s.io/klog@v1.0.0/klogr/klogr.go:189:2: cannot use new (type klogger) as type logr.Logger in return argument
../../go/pkg/mod/k8s.io/klog@v1.0.0/klogr/klogr.go:192:5: cannot use klogger{} (type klogger) as type logr.Logger in assignment
../../go/pkg/mod/k8s.io/klog@v1.0.0/klogr/klogr.go:193:7: undefined: logr.InfoLogger
make: *** [Makefile:80: managers] Error 2

So much noise, all of it largely irrelevant since I own exactly none of this.

The too many errors may explain why my text editor did not let me know what was up, clearly it got overwhelmed when it tried to build. Actually now I am thinking about it, an error did pop up and I got irritated and ignored it.

What’s happened is that the logr bump did have a breaking change (a year ago? Why am I only hearing of this now?)… and not just for our code. Some other things which we depend on also use that package, so they needed to also be bumped at the same time.

This is why dependabot only helps you so far, especially in the k8s ecosystem.

After a bit more fiddling about, I merged all my related dependency changes into one, and the linter was happy.

So the moral of this story is: if linting is failing for weird reasons, trying building instead to get a more useful error.

Please remember this next time Claudia, I won’t have this conversation again.