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

⚠ Bump k8s.io/ to v1.30.0-beta.0 #2693

Merged
merged 3 commits into from Mar 22, 2024

Conversation

twz123
Copy link
Contributor

@twz123 twz123 commented Feb 22, 2024

Kubernetes 1.30 has a breaking API change around leader election metrics, introduced in kubernetes/kubernetes#122069. Not sure if there's a trick to fixing this in a way that's compatible with both 1.29 and 1.30 at the same time. Not sure when controller-runtime wants to jump to k8s 1.30, so leaving this as a draft.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 22, 2024
@k8s-ci-robot
Copy link
Contributor

Welcome @twz123!

It looks like this is your first PR to kubernetes-sigs/controller-runtime 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-runtime has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Contributor

Hi @twz123. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 22, 2024
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 22, 2024
@sbueringer
Copy link
Member

sbueringer commented Mar 14, 2024

@twz123 Do you have time to rebase to v1.30.0-beta.0? In general I would bump to v1.30 now for early feedback.

I'm not sure I understand what the breaking change around leader election metrics is

EDIT: Ah that they renamed & changed the SwitchMetric/LeaderMetric interface? I don't think we can mitigate that. The problem is that we can't add an additional method which returns leaderelection.SwitchMetric, because this interface doesn't exist anymore in the client-go version we import

@sbueringer
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 14, 2024
@sbueringer sbueringer marked this pull request as ready for review March 14, 2024 09:54
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 14, 2024
@sbueringer
Copy link
Member

/retest

@twz123 twz123 changed the title ⚠ Bump k8s.io/ to v1.30.0-alpha.2 ⚠ Bump k8s.io/ to v1.30.0-beta.0 Mar 14, 2024
@twz123
Copy link
Contributor Author

twz123 commented Mar 14, 2024

Is there some way to let Prow use Go 1.22? Kubernetes 1.30 requires it.

@sbueringer
Copy link
Member

sbueringer commented Mar 14, 2024

Is there some way to let Prow use Go 1.22? Kubernetes 1.30 requires it.

Yes, you have to bump the images in:

Can you open a PR for that? (then I can approve)

go.mod Outdated
Comment on lines 3 to 5
go 1.22.0

toolchain go1.21.0
toolchain go1.22.1
Copy link
Member

Choose a reason for hiding this comment

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

Do you know what the exact implications are of using go 1.22.0 and toolchain go1.22.1?

(specifically the exact patch version in each)

Copy link
Contributor Author

@twz123 twz123 Mar 14, 2024

Choose a reason for hiding this comment

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

According to https://go.dev/doc/toolchain#config, IIUC, the go directive specifies the minimum required version, while the toolchain directive specifies the suggested version. Perhaps the toolchain directive could be removed altogether here?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe. Is it automatically added by go mod tidy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've seen this added automatically sometimes, but I haven't caught the pattern yet. It definitely won't be added by go mod tidy if everything is in sync, at least not for me.

Copy link
Member

Choose a reason for hiding this comment

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

Did a bit of reserach:

  • if go and toolchain use the same version, go mod tidy will remove toolchain (i.e. it's only preserved if the versions are different)
  • we introduced toolchain when bumping to Go 1.21. I believe either this was added because we used 1.21 (instead of 1.21.0) for the go directive or because the cleanup in go mod tidy wasn't implemented yet.

Kubernetes today only sets the Go directive to 1.22.0 and omits the toolchain directive: https://github.com/kubernetes/kubernetes/blob/master/go.mod#L9

I would like to follow this pattern for now, if we don't have a good reason to set the toolchain directive as well.

Copy link
Member

Choose a reason for hiding this comment

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

Here is more context on the whole topic: https://go.dev/doc/toolchain

My tl;dr for here is basically I don't think we need a different minimum and preferred toolchain

Copy link
Member

Choose a reason for hiding this comment

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

This one is interesting

Commands that incorporate new module versions that require new Go versions write the new minimum go version requirement to the current workspace’s go.work file or the main module’s go.mod file, updating the go line. For repeatability, any command that updates the go line also updates the toolchain line to record its own toolchain name. The next time the go command runs in that workspace or module, it will use that updated toolchain line during toolchain selection.

@twz123
Copy link
Contributor Author

twz123 commented Mar 14, 2024

There you go:

@sbueringer
Copy link
Member

/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2024
All the tool binaries are managed via go install nowadays, so no need to
keep the module files around.

Add a .keep file to keep the now empty tools directory. It will still
be used by the build to store the tool binaries. Even though it would be
created automatically, having it as part of the git tree makes it more
obvious that it's still being used by the build.

See: 9847243 ("Use go-install for versioned dependencies")
Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
This is a prerequisite for Kubernetes 1.30.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
Kubernetes 1.30 has a breaking API change around leader election
metrics. Adapt to those API changes accordingly.

Signed-off-by: Tom Wieczorek <twieczorek@mirantis.com>
gibizer added a commit to gibizer/placement-operator that referenced this pull request Mar 22, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9
booxter added a commit to openstack-k8s-operators/neutron-operator that referenced this pull request Mar 22, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9

Co-Authored-By: Balazs Gibizer <gibi@redhat.com>
booxter added a commit to booxter/ovn-operator that referenced this pull request Mar 22, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9

Co-Authored-By: Balazs Gibizer <gibi@redhat.com>
hongweiliu17 added a commit to hongweiliu17/integration-service that referenced this pull request Mar 25, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9

Signed-off-by: Hongwei Liu <hongliu@redhat.com>
hongweiliu17 added a commit to hongweiliu17/integration-service that referenced this pull request Mar 25, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9

Signed-off-by: Hongwei Liu <hongliu@redhat.com>
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/placement-operator that referenced this pull request Mar 26, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/nova-operator that referenced this pull request Mar 27, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9
jpodivin added a commit to jpodivin/infra-operator that referenced this pull request Mar 27, 2024
Change[0] in controller-runtime is causing our tests to fail with go version mismatch.

This version restriction should be removed once the issue is fixed in the controller-runtime

[0] kubernetes-sigs/controller-runtime#2693

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
jpodivin added a commit to jpodivin/infra-operator that referenced this pull request Mar 27, 2024
Change[0] in controller-runtime is causing our tests to fail with go version mismatch.

This version restriction should be removed once the issue is fixed in the controller-runtime

[0] kubernetes-sigs/controller-runtime#2693

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
vakwetu added a commit to vakwetu/keystone-operator that referenced this pull request Mar 27, 2024
kubernetes-sigs/controller-runtime#2693 broke the content provider while
doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9
raukadah added a commit to raukadah/barbican-operator that referenced this pull request Mar 28, 2024
kubernetes-sigs/controller-runtime#2693
broke
the functional tests running downstream with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

Links:
[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9

Signed-off-by: Chandan Kumar <raukadah@gmail.com>
millevy added a commit to millevy/barbican-operator that referenced this pull request Mar 28, 2024
kubernetes-sigs/controller-runtime#2693 broke the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9
stuggi pushed a commit to stuggi/openstack-operator that referenced this pull request Apr 9, 2024
kubernetes-sigs/controller-runtime#2693 broke
the content provider while doing docker-build with following issue[1]

In order to unblock the CI, we are pinning
sigs.k8s.io/controller-runtime/tools/setup-envtest at previous working
commit[2]

[1]. kubernetes-sigs/controller-runtime#2720
[2]. kubernetes-sigs/controller-runtime@c7e1dc9

Signed-off-by: Chandan Kumar <chkumar@redhat.com>
@pmalek
Copy link

pmalek commented Apr 22, 2024

Can we have this in release-0.17 to prevent

Error: ../../../go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.3/pkg/metrics/leaderelection.go:26:71: undefined: leaderelection.SwitchMetric

with recent k8s 1.30 packages?

@sbueringer
Copy link
Member

No because this would make CR v0.17.x incompatible with client-go v0.29

@pmalek
Copy link

pmalek commented Apr 22, 2024

ACK. When do we expect to ship 0.18?

@sbueringer
Copy link
Member

Current plan is ~ this week. Can't guarantee it though.

Mostly depends on #2783 as far as I can tell

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants