This fixes a bug that caused log calls involving `klog.Logger` to not be
checked.
As a result we have to fix some code that is now considered faulty:
ERROR: pkg/controller/serviceaccount/tokens_controller.go:382:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (e *TokensController) generateTokenIfNeeded(ctx context.Context, logger klog.Logger, serviceAccount *v1.ServiceAccount, cachedSecret *v1.Secret) ( /* retry */ bool, error) {
ERROR: ^
ERROR: pkg/controller/storageversionmigrator/storageversionmigrator.go:299:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (svmc *SVMController) runMigration(ctx context.Context, logger klog.Logger, gvr schema.GroupVersionResource, resourceMonitor *garbagecollector.Monitor, toBeProcessedSVM *svmv1beta1.StorageVersionMigration, listResourceVersion string) (err error, failed bool) {
ERROR: ^
ERROR: pkg/proxy/node.go:121:3: logging function "Error" should not use format specifier "%q" (logcheck)
ERROR: klog.FromContext(ctx).Error(nil, "Timed out waiting for node %q to exist", nodeName)
ERROR: ^
ERROR: pkg/proxy/node.go:123:3: logging function "Error" should not use format specifier "%q" (logcheck)
ERROR: klog.FromContext(ctx).Error(nil, "Timed out waiting for node %q to be assigned IPs", nodeName)
ERROR: ^
ERROR: pkg/scheduler/backend/queue/scheduling_queue.go:610:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (p *PriorityQueue) runPreEnqueuePlugin(ctx context.Context, logger klog.Logger, pl fwk.PreEnqueuePlugin, pInfo *framework.QueuedPodInfo, shouldRecordMetric bool) *fwk.Status {
ERROR: ^
ERROR: pkg/scheduler/framework/plugins/dynamicresources/extendeddynamicresources.go:286:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (pl *DynamicResources) deleteClaim(ctx context.Context, claim *resourceapi.ResourceClaim, logger klog.Logger) error {
ERROR: ^
ERROR: pkg/scheduler/framework/plugins/dynamicresources/extendeddynamicresources.go:499:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (pl *DynamicResources) waitForExtendedClaimInAssumeCache(
ERROR: ^
ERROR: pkg/scheduler/framework/plugins/dynamicresources/extendeddynamicresources.go:528:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (pl *DynamicResources) createExtendedResourceClaimInAPI(
ERROR: ^
ERROR: pkg/scheduler/framework/plugins/dynamicresources/extendeddynamicresources.go:592:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (pl *DynamicResources) unreserveExtendedResourceClaim(ctx context.Context, logger klog.Logger, pod *v1.Pod, state *stateData) {
ERROR: ^
ERROR: pkg/scheduler/framework/runtime/batch.go:171:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (b *OpportunisticBatch) batchStateCompatible(ctx context.Context, logger klog.Logger, pod *v1.Pod, signature fwk.PodSignature, cycleCount int64, state fwk.CycleState, nodeInfos fwk.NodeInfoLister) bool {
ERROR: ^
ERROR: staging/src/k8s.io/component-base/featuregate/feature_gate.go:890:4: Additional arguments to Info should always be Key Value pairs. Please check if there is any key or value missing. (logcheck)
ERROR: logger.Info("Warning: SetEmulationVersionAndMinCompatibilityVersion will change already queried feature", "featureGate", feature, "oldValue", oldVal, newVal)
ERROR: ^
ERROR: test/images/sample-device-plugin/sampledeviceplugin.go:108:2: logging function "Info" should not use format specifier "%s" (logcheck)
ERROR: logger.Info("pluginSocksDir: %s", pluginSocksDir)
ERROR: ^
ERROR: test/images/sample-device-plugin/sampledeviceplugin.go:123:2: logging function "Info" should not use format specifier "%s" (logcheck)
ERROR: logger.Info("CDI_ENABLED: %s", cdiEnabled)
ERROR: ^
While waiting for this to merge, another call was added which also doesn't
follow conventions:
ERROR: pkg/kubelet/kubelet.go:2454:1: A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that. (logcheck)
ERROR: func (kl *Kubelet) deletePod(ctx context.Context, logger klog.Logger, pod *v1.Pod) error {
ERROR: ^
Contextual logging has been beta and enabled by default for several releases
now. It's mostly just a matter of wrapping up and declaring it GA. Therefore
the calls which directly call WithName or WithValues (always have an effect)
are left as-is instead of converting them to use the klog wrappers (support
disabling the effect). To allow that, the linter gets reconfigured to not
complain about this anymore, anywhere.
The calls which would have to be fixed otherwise are:
ERROR: pkg/kubelet/cm/dra/claiminfo.go:170:11: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger = logger.WithName("dra-claiminfo")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/healthinfo.go:45:11: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger = logger.WithName("dra-healthinfo")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/healthinfo.go:89:11: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger = logger.WithName("dra-healthinfo")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/healthinfo.go:157:11: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger = logger.WithName("dra-healthinfo")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/manager.go:175:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-manager")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/manager.go:239:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-manager")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/manager.go:593:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-manager")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/manager.go:781:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(context.Background()).WithName("dra-manager")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/manager.go:898:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-manager")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/manager_test.go:1638:15: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(streamCtx).WithName(st.Name())
ERROR: ^
ERROR: pkg/kubelet/cm/dra/plugin/dra_plugin.go:77:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-plugin")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/plugin/dra_plugin.go:108:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-plugin")
ERROR: ^
ERROR: pkg/kubelet/cm/dra/plugin/dra_plugin.go:161:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger := klog.FromContext(ctx).WithName("dra-plugin")
ERROR: ^
ERROR: staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker.go:695:14: function "WithValues" should be called through klogr.LoggerWithValues (logcheck)
ERROR: logger := logger.WithValues("device", deviceID)
ERROR: ^
ERROR: test/integration/apiserver/watchcache_test.go:42:54: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: etcd0URL, stopEtcd0, err := framework.RunCustomEtcd(klog.FromContext(ctx).WithName("etcd0"), "etcd_watchcache0", etcdArgs)
ERROR: ^
ERROR: test/integration/apiserver/watchcache_test.go:47:54: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: etcd1URL, stopEtcd1, err := framework.RunCustomEtcd(klog.FromContext(ctx).WithName("etcd1"), "etcd_watchcache1", etcdArgs)
ERROR: ^
ERROR: test/integration/scheduler_perf/scheduler_perf.go:1149:12: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: logger = logger.WithName(tCtx.Name())
ERROR: ^
bloop was disabled because of its impact on small benchmarks (see
267fc6b81a
for details), this removes it from the disabled entries.
stringscut was introduced in x/tools 0.40.0 and is disabled for
existing code.
ginkgolinter's Success matcher check doesn't know about TContext, so
it's disabled for k/k.
Signed-off-by: Stephen Kitt <skitt@redhat.com>
The update logcheck linter will warn about logger.V().Error by default. Such
calls make no sense because logr by design treats this like logger.Error. In
Kubernetes, they tend to be the result of an incorrect conversion of
klog.V().Error[S], which does check the verbosity. With logger, Error needs to
be replaced by Info.
mockery has introduced breaking changes and switched to a v3 branch,
this migrates to that, mostly using the built-in migration tool. Mocks
are now generated in single files per package, except in packages
containing mocks for multiple interface packages (in
pkg/kubelet/container/testing).
Signed-off-by: Stephen Kitt <skitt@redhat.com>
The advantage is that it separates the dependencies of the different tools
better. golangci-lint in particular has many dependencies and is sometimes
sensitive to the exact version being used. This way, "go get" bumps up
dependencies exactly as defined by the upstream golangci-lint module.
It's not quite self-contained because logcheck as a Go plugin for golangci-lint
must be built from the same dependencies. But it only adds one and does not
change any of the others.
While at it, the Go 1.24 "tools" directive gets used instead of the traditional
tools.go approach.
github.com/client9/misspell was archived by the owner on Mar 26, 2025.
The golangci-lint team maintains a fork.
The newer code finds some misspellings which where missed before.
Filtering the output with grep leads to hard to read log output, e.g. from
pull-kubernetes-unit:
+++ [0613 15:32:48] Running tests without code coverage and with -race
{"Time":"2024-06-13T15:33:47.845457374Z","Action":"output","Package":"k8s.io/kubernetes/cluster/gce/cos","Test":"TestCreateMasterAuditPolicy","Output":" /tmp/configure-helper-test47992121/kube-env: line 1: `}'\n"}
{"Time":"2024-06-13T15:33:49.053732803Z","Action":"output","Package":"k8s.io/kubernetes/cluster/gce/cos","Output":"ok \tk8s.io/kubernetes/cluster/gce/cos\t2.906s\n"}
We can do better than that. When feeding the output of the "go test" command(s)
into gotestsum *while it runs*, we can use --format=standard-quiet (= normal go
test output) or --format=standard-verbose (= `go test -v`) when FULL_LOG is
requested to get nicer output.
This works when testing everything at once. This was said to be not possible
when doing coverage profiling. But recent Go no longer has that limitation, so
the xargs trick gets removed. All that we need to do for coverage profiling is
to add some additional parameters and the conversion to HTML.
testify is used throughout the codebase; this switches mocks from
gomock to testify with the help of mockery for code generation.
Handlers and mocks in test/utils/oidc are moved to a new package:
mockery operates package by package, and requires packages to build
correctly; test/utils/oidc/testserver.go relies on the mocks and fails
to build when they are removed. Moving the interface and mocks to a
different package allows mockery to process that package without
having to build testserver.go.
Signed-off-by: Stephen Kitt <skitt@redhat.com>
See https://github.com/golang/mock#gomock: golang/mock is no longer
maintained, and should be replaced by go.uber.org/mock.
This allows golang/mock to be dropped from the status and vendored
fields in unwanted-dependencies.json.
Signed-off-by: Stephen Kitt <skitt@redhat.com>
This creates go.work and enables Go Workspaces. This is a file that
includes info on k/k and all the staging modules.
This depends on go 1.22 and setting FORCE_HOST_GO=true (for kube
scripts, which try to be hermetic).
Make this part of the normal update/verify sequence.
The top-level go.work file contains no replace statements. Instead, the
replace statements in the individual go.mod files are used. For this to
work, replace statements in the individual go.mod files have to be
consistent.
hack/tools has different dependencies and can't be in the main
workspace, so this adds a go.work just for that. Without this, go tries
to consider all deps in all modules and pick one that works for all.
This is problematic because there are so many of them that it is
difficult to manage.
Likewise for k8s.io/code-generator/examples and
k8s.io/kms/internal/plugins/_mock - add trivial go.work files.
For example k/k depends on an older version of a lib that gloangci-lint
needs (transitively) and it breaks.
This also updates vendor (needed to make go happy), and removes
vendor'ed symlinks. This breaks a LOT of our build tools, which will be
fixed subsequently.
Result: `go` commands work across modules:
Before:
```
$ go list ./pkg/proxy/iptables/ ./staging/src/k8s.io/api/core/v1/
main module (k8s.io/kubernetes) does not contain package k8s.io/kubernetes/staging/src/k8s.io/api/core/v1
$ go build ./pkg/proxy/iptables/ ./staging/src/k8s.io/api
main module (k8s.io/kubernetes) does not contain package k8s.io/kubernetes/staging/src/k8s.io/api
$ go test ./pkg/proxy/iptables/ ./staging/src/k8s.io/api
main module (k8s.io/kubernetes) does not contain package k8s.io/kubernetes/staging/src/k8s.io/api
```
After:
```
$ go list ./pkg/proxy/iptables/ ./staging/src/k8s.io/api/core/v1/
k8s.io/kubernetes/pkg/proxy/iptables
k8s.io/api/core/v1
$ go build ./pkg/proxy/iptables/ ./staging/src/k8s.io/api
$ go test ./pkg/proxy/iptables/ ./staging/src/k8s.io/api
ok k8s.io/kubernetes/pkg/proxy/iptables 0.360s
ok k8s.io/api 2.302s
```
Result: `make` fails:
```
$ make
go version go1.22rc1 linux/amd64
+++ [0106 12:11:03] Building go targets for linux/amd64
k8s.io/kubernetes/cmd/kube-proxy (static)
k8s.io/kubernetes/cmd/kube-apiserver (static)
k8s.io/kubernetes/cmd/kube-controller-manager (static)
k8s.io/kubernetes/cmd/kubelet (non-static)
k8s.io/kubernetes/cmd/kubeadm (static)
k8s.io/kubernetes/cmd/kube-scheduler (static)
k8s.io/component-base/logs/kube-log-runner (static)
k8s.io/kube-aggregator (static)
k8s.io/apiextensions-apiserver (static)
k8s.io/kubernetes/cluster/gce/gci/mounter (static)
k8s.io/kubernetes/cmd/kubectl (static)
k8s.io/kubernetes/cmd/kubectl-convert (static)
github.com/onsi/ginkgo/v2/ginkgo (non-static)
k8s.io/kubernetes/test/e2e/e2e.test (test)
k8s.io/kubernetes/test/conformance/image/go-runner (non-static)
k8s.io/kubernetes/cmd/kubemark (static)
github.com/onsi/ginkgo/v2/ginkgo (non-static)
k8s.io/kubernetes/test/e2e_node/e2e_node.test (test)
test/e2e/e2e.go:35:2: cannot find package "k8s.io/api/apps/v1" in any of:
/home/thockin/src/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/k8s.io/api/apps/v1 (vendor tree)
/home/thockin/src/kubernetes/_output/local/.gimme/versions/go1.22rc1.linux.amd64/src/k8s.io/api/apps/v1 (from $GOROOT)
/home/thockin/src/kubernetes/_output/local/go/src/k8s.io/api/apps/v1 (from $GOPATH)
... more ...
... more ...
... more ...
!!! [0106 12:13:41] Call tree:
!!! [0106 12:13:41] 1: /home/thockin/src/kubernetes/hack/lib/golang.sh:948 kube::golang::build_binaries_for_platform(...)
!!! [0106 12:13:41] 2: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
!!! [0106 12:13:41] Call tree:
!!! [0106 12:13:41] 1: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
!!! [0106 12:13:41] Call tree:
!!! [0106 12:13:41] 1: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
make: *** [Makefile:96: all] Error 1
```
Again, this requires go 1.22 (e.g. gotip), as go 1.21.x does not have
`go work vendor` support.
TO REPEAT:
( \
./hack/update-go-workspace.sh; \
./hack/update-vendor.sh; \
./hack/update-go-workspace.sh; \
)
