Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated linters #1633

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RainbowMango
Copy link
Member

@RainbowMango RainbowMango commented Mar 20, 2025

What this PR does / why we need it:
This PR updates unsupported configurations of golangci-lint and replaces deprecated linter.

The exportloopref has been deprecated from golangci-lint@v1.60.2 and replaced by copyloopvar.

The typecheck has been removed from golangci-lint:

-bash-5.0# golangci-lint linters | grep typecheck | wc -l 
0
-bash-5.0# 
-bash-5.0# golangci-lint version
golangci-lint has version 1.61.0 built with go1.23.1 from a1d6c560 on 2024-09-09T17:44:42Z

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 20, 2025
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 20, 2025
@richabanker
Copy link

/triage accepted
/assign @dgrisonnet

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 20, 2025
@RainbowMango
Copy link
Member Author

I'm exploring this weird failing test:

pkg/api/pod_test.go:34:11: could not import k8s.io/apimachinery/pkg/types (-: could not load export data: internal error in importing "k8s.io/apimachinery/pkg/types" (unsupported version: 2); please report an issue) (typecheck)
	apitypes "k8s.io/apimachinery/pkg/types"
	         ^
pkg/api/node.go:89:3: undefined: klog (typecheck)
		klog.ErrorS(err, "Failed reading nodes metrics")
		^
pkg/api/node.go:105:3: undefined: klog (typecheck)
		klog.ErrorS(err, "Failed listing nodes", "labelSelector", labelSelector)
		^
pkg/api/node.go:1[22](https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_metrics-server/1633/pull-metrics-server-verify/1902712244030935040#1:build-log.txt%3A22):3: undefined: klog (typecheck)
		klog.ErrorS(err, "Failed getting node", "node", klog.KRef("", name))

@RainbowMango RainbowMango force-pushed the pr_remove_deprecated_linter branch from f2a8693 to b3b5499 Compare March 21, 2025 02:17
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: RainbowMango
Once this PR has been reviewed and has the lgtm label, please ask for approval from dgrisonnet. For more information see the Code Review Process.

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

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

@RainbowMango RainbowMango force-pushed the pr_remove_deprecated_linter branch from b3b5499 to 6d8aaf5 Compare March 21, 2025 02:26
@RainbowMango
Copy link
Member Author

@dgrisonnet Could you please take a look at this? this is the blocker of #1585.

@@ -1,5 +1,5 @@
run:
deadline: 2m
timeout: 10m
Copy link
Member

Choose a reason for hiding this comment

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

10 minutes is a huge bump, could we perhaps squeeze it down to 5 minutes and the jobs would still pass?

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow, I just noticed that the timeout in the configuration file is being overwritten by the --timeout flag in the Makefile:

$(GOPATH)/bin/golangci-lint run --timeout 10m || (echo 'Run "make update"' && exit 1)

So, this is not a bump at all :) :) :)

I just removed the --timeout flag from Makefile, it makes it easier to manage the golangci-lint configurations from one place.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to squeeze it down to 5 minutes, but the jobs failed due to exceeding the timeout:
echo from https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_metrics-server/1633/pull-metrics-server-verify/1903279512716578816

golangci/golangci-lint info found version: 1.64.8 for v1.64.8/linux/amd64
/home/prow/go/bin/golangci-lint run || (echo 'Run "make update"' && exit 1)
level=error msg="Timeout exceeded: try increasing it by passing --timeout option"

@RainbowMango RainbowMango force-pushed the pr_remove_deprecated_linter branch 2 times, most recently from e819a7d to 99dd431 Compare March 22, 2025 02:56
Signed-off-by: RainbowMango <qdurenhongcai@gmail.com>
@RainbowMango RainbowMango force-pushed the pr_remove_deprecated_linter branch from 99dd431 to f60ef2f Compare March 22, 2025 03:06
@RainbowMango
Copy link
Member Author

@dgrisonnet Could you please take another look?

By the way, I‘d like to help bump the Kubernetes dependencies to v1.32, before that, we need to resolve the blocking CI issues.
Would be appreciated if I could get your support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants