-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
upgrade base image and tooling dependencies #3816
Conversation
looks like we'll have to upgrade golangci-lint |
ok, go updates are going to be a slightly bigger can of worms ... |
will revisit sometime tomorrow |
https://github.com/opencontainers/runc/releases/tag/v1.2.3 dropped since I started this PR :-) |
runc looks important, several fixes for regressions |
The good news is all the prow e2e CI passed flawlessly. The bad news is https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kind/3816/pull-kind-verify/1866663406874923008
$ git diff
diff --git a/hack/tools/go.mod b/hack/tools/go.mod
index ee468e10..ce7cba29 100644
--- a/hack/tools/go.mod
+++ b/hack/tools/go.mod
@@ -1,6 +1,8 @@
module sigs.k8s.io/kind/hack/tools
-go 1.17
+go 1.22.1
+
+toolchain go1.23.4
require (
github.com/golangci/golangci-lint v1.62.2 |
Do we need to be as concerned with |
I think this is OK, but I haven't root-caused yet, and it means we'll have to start duplicating the toolchain version |
I'm looking at |
Just updating the tools module language version to 1.23 works as expected, and I think that makes the most sense. At some point we'll want to bump the main module up, but we don't have a strong use case for any of the new-fangled features for the CLI binary and we will break people doing |
fixed |
6e84a06
to
8f97ed0
Compare
aside: I don't generally recommend "big update all the dependencies" PRs, but these are high-touch and the last incoming PR was unresponsive about reviewing release notes. At this point a number of them are inter-related (like go1.23 and some of the base image bumps), and we don't risk disrupting 1.32 release anymore, and I have some time to review the downstream changes and test things out now ... still WIP, will need new base image and other follow-ups (given the scope of this change I'd rather test the new base image before merge than use post-merge automation and a follow-up PR) |
[I'm intending that we roll this + the |
very happy to see actions (with assorted host runtimes etc) working with updated runc in particular via WIP commit node image (309db31) |
looks like golangci-lint timed out: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kind/3816/pull-kind-verify/1866972860258455552 |
golangci-lint timeout is not a flake. seems likely to be related to 8f97ed0 given it was passing last night and the dockerfile changes should be unrelated |
looks like it's while linting kindnetd |
we did increase the size of that binary recently, maybe just bump the linter timeout. through 2m feels somewhat generous given the amount of in-repo source code. |
the whole verify job only took 4m so I guess that's fine https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kind/3816/pull-kind-verify/1866986720868700160 |
OK, now everything is working. I think we should also upgrade kindnetd and local-path-provisioner but this is already pretty huge. I'm going to drop the temp image bump commits and we'll get new images built normally in postsubmit / follow-ups, I'm fairly satisfied that we've exercised runc+containerd in CI in particular. (1265c24 and 309db31) |
56b06ad
to
20b3d8e
Compare
I think let's split the rest into further PRs |
/retest NOTE: I dropped the image bump PRs, so that's not caused by this PR, just a flaky test |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, BenTheElder The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I've been reading through the release notes for these one by one ...
I think we should also do local-path-provisioner but I haven't finished reviewing that and I want to let some of the CI go ahead and test this much. NOTE: that doesn't yet include the github actions which won't build a node image.