Skip to content

fix type detection#33

Merged
k8s-ci-robot merged 3 commits into
kubernetes-sigs:mainfrom
pohly:fix-type-alias-support
Feb 18, 2026
Merged

fix type detection#33
k8s-ci-robot merged 3 commits into
kubernetes-sigs:mainfrom
pohly:fix-type-alias-support

Conversation

@pohly

@pohly pohly commented Feb 17, 2026

Copy link
Copy Markdown
Contributor

Recent Go packages report klog's Logger = logr.Logger type alias as a types.Alias instead of the types.Named that the code supported. As a result, checking of calls through that alias wasn't working anymore in Kubernetes because it builds the plugin with Go 1.25 and more recent dependencies.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 17, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

This issue is currently awaiting triage.

If logtools contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@github-project-automation github-project-automation Bot moved this to Needs Triage in SIG Instrumentation Feb 17, 2026
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 17, 2026
@pohly pohly force-pushed the fix-type-alias-support branch 2 times, most recently from 573b649 to 05f9cf9 Compare February 17, 2026 11:13
Recent Go changed how type aliases are handled. We need to test with it...
Using the original type or the alias may make a difference...
With recent dependencies (= `go get -u ./...`), type aliases are reported as
`types.Alias` instead of `types.Named`. Found initially in Kubernetes,
confirmed with unit tests:

    --- FAIL: TestAnalyzer (3.05s)
        --- FAIL: TestAnalyzer/Function_call_parameters (0.20s)
            analysistest.go:713: parameters/parameters.go:54: no diagnostic was reported matching `Additional arguments to Info should always be Key Value pairs. Please check if there is any key or value missing.`
            analysistest.go:713: parameters/parameters.go:55: no diagnostic was reported matching `Additional arguments to Error should always be Key Value pairs. Please check if there is any key or value missing.`
            analysistest.go:713: parameters/parameters.go:65: no diagnostic was reported matching `Key positional arguments are expected to be inlined constant strings. Please replace 1 provided with string value`
            analysistest.go:713: parameters/parameters.go:67: no diagnostic was reported matching `Key positional arguments are expected to be inlined constant strings. Please replace 1 provided with string value`
            analysistest.go:713: parameters/parameters.go:80: no diagnostic was reported matching `Additional arguments to Info should always be Key Value pairs. Please check if there is any key or value missing.`
        --- FAIL: TestAnalyzer/helpers (0.16s)
            analysistest.go:713: helpers/doNotAllowDirectCalls.go:34: no diagnostic was reported matching `function "WithName" should be called through klogr.LoggerWithName`
            analysistest.go:713: helpers/doNotAllowDirectCalls.go:35: no diagnostic was reported matching `function "WithValues" should be called through klogr.LoggerWithValues`
        --- FAIL: TestAnalyzer/Detect_incomplete_fmt.Stringer (0.21s)
            analysistest.go:713: stringer/stringer.go:27: no diagnostic was reported matching `The type \*stringer.config inherits \(\*k8s.io/apimachinery/pkg/apis/meta/v1.TypeMeta\).String as implementation of fmt.Stringer, which covers only a subset of the value. Implement String\(\) for the type or wrap it with TODO.`
    FAIL
    FAIL	sigs.k8s.io/logtools/logcheck	3.059s

We need to resolve type aliases. We can't just call Underlying, that
resolves to an unnamed type. We need to use Rhs instead, which depends
on Go 1.23.
@pohly pohly force-pushed the fix-type-alias-support branch from 05f9cf9 to 7c2a63c Compare February 17, 2026 11:25
@pohly

pohly commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

Errors that went undetected in Kubernetes because of this:

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/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: 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:602: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/client-go/transport/cache.go:122:13: function "WithName" should be called through klogr.LoggerWithName (logcheck)
ERROR: 		logger := klog.Background().WithName("tls-transport-cache")
ERROR: 		          ^
ERROR: staging/src/k8s.io/component-base/featuregate/feature_gate.go:884: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: 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/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: 	^
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: 		         ^

We can probably relax the should be called through klogr.LoggerWith because contextual logging is beta, so I won't fix those and instead relax the logcheck configuration. Some of the others are real.

@pohly

pohly commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

/assign @dims

@dims dims left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, pohly

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pohly

pohly commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

@serathius: can I still count on you for a formal LGTM?

@pohly

pohly commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

/assign @serathius
/cc @tallclair

@serathius

Copy link
Copy Markdown
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 18, 2026
@k8s-ci-robot k8s-ci-robot merged commit bc83553 into kubernetes-sigs:main Feb 18, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants