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.

Sign up for GitHub

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

Jump to bottom

🌱 Upgrade to controller-runtime v0.12.2 and k8s.io/* v0.24.* #6467

Merged
merged 1 commit into from Jun 7, 2022
Merged

🌱 Upgrade to controller-runtime v0.12.2 and k8s.io/* v0.24.* #6467

merged 1 commit into from Jun 7, 2022

Conversation

palnabarun
Copy link
Member

@palnabarun palnabarun commented Apr 30, 2022 •
edited

What this PR does / why we need it:

The latest tag of controller-runtime is https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.12.1

Also, bumps the same for hack/tools and test.

Which issue(s) this PR fixes:

None

/assign @sbueringer

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 30, 2022
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 30, 2022
@palnabarun palnabarun force-pushed the bump-cr-latest branch 2 times, most recently from c72e731 to c2d3bd2 Compare April 30, 2022 14:37
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 30, 2022
@sbueringer
Copy link
Member

Thx, looks good overall one nit

killianmuldoon
killianmuldoon reviewed May 4, 2022
View reviewed changes
Copy link
Contributor

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

Now that Kubernetes 1.24 is released maybe we should wait for a controller runtime release incorporating the new K8s version ?

@palnabarun
Copy link
Member Author

@killianmuldoon -- A PR is open to update the same in CR: kubernetes-sigs/controller-runtime#1885

I agree we can wait for that land.

@palnabarun
Copy link
Member Author

CR master now has Kubernetes 1.24 packages. I have updated the PR.

Note: I will squash the commits and organize them into 2 or 3 based on functionality after another round of review.

@k8s-ci-robot k8s-ci-robot changed the title 🌱 Upgrade controller-runtime to latest master for bringing in Kubernetes 1.24.0-rc.1 changes seedling Upgrade controller-runtime to latest master for bringing in Kubernetes 1.24.0 changes May 5, 2022
@killianmuldoon
Copy link
Contributor

/retitle 🌱 Upgrade controller-runtime to latest master for bringing in Kubernetes 1.24.0 changes

@k8s-ci-robot k8s-ci-robot changed the title seedling Upgrade controller-runtime to latest master for bringing in Kubernetes 1.24.0 changes 🌱 Upgrade controller-runtime to latest master for bringing in Kubernetes 1.24.0 changes May 5, 2022
@killianmuldoon killianmuldoon mentioned this pull request May 5, 2022
Closed
killianmuldoon
killianmuldoon reviewed May 5, 2022
View reviewed changes
Copy link
Contributor

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

There's a big-ish API diff because of the deprecation of ClusterName in metadata. I think we can safely ignore that as the fielld wasn't (correctly) used at any point.

@palnabarun Could you maybe add a not to the migration guide (https://main.cluster-api.sigs.k8s.io/developer/providers/v1.1-to-v1.2.html) with a short note about the ClusterName deprecation?

@killianmuldoon killianmuldoon mentioned this pull request May 5, 2022
Closed
33 tasks
@sbueringer
Copy link
Member

sbueringer commented May 10, 2022 •
edited

@palnabarun lgtm from my side +/- squash and the note Killian mentioned would be good (mentioning the upgraded dependencies (like for the previous release) + a short note that ClusterName has been removed would be good)

@sbueringer
Copy link
Member

@palnabarun just fyi. controller-runtime v0.12 was just released. If you don't mind, we could directly upgrade to the release.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2022
@shubham14bajpai shubham14bajpai mentioned this pull request May 18, 2022
Closed
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 19, 2022
@sbueringer
Copy link
Member

sbueringer commented Jun 1, 2022 •
edited

@palnabarun Do you have time to follow-up on this PR?
We were planing on doing a v1.2 RC in ~ two weeks and it would be great to run CAPI with the new dependencies in CI for a few days / weeks before the CAPI release.
(especially new controller-runtime versions are comparatively risky)

@batistein batistein mentioned this pull request Jun 3, 2022
Closed
@sbueringer
Copy link
Member

@palnabarun Looks good.
Can you please restore the zz_generated.openapi.go files? I assume they got lost during make generate because of #6526.

@palnabarun
Copy link
Member Author

@sbueringer -- Hey, sorry for not following up on this earlier. I was travelling/out of the office most of the last month.

I have updated the docs.

I think the GetOpenAPIDefinitions not declared by package v1beta1 (typecheck) error is due to the removal of the generated OpenAPI definitions when make generate ran. It may be related to #6462.

@palnabarun palnabarun force-pushed the bump-cr-latest branch 2 times, most recently from 43c7fcf to 6f42356 Compare June 6, 2022 12:08
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 6, 2022
@palnabarun
Copy link
Member Author

Squashed changes to a single commit.

@sbueringer sbueringer changed the title 🌱 Upgrade controller-runtime to latest master for bringing in Kubernetes 1.24.0 changes 🌱 Upgrade to controller-runtime v0.12.2 and k8s.io/* v0.24.* Jun 6, 2022
@sbueringer
Copy link
Member

Thx!

/lgtm
/assign @fabriziopandini

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2022
@sbueringer
Copy link
Member

/retest
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-workload-upgrade-1-24-latest-main

@sbueringer
Copy link
Member

/retest
/test pull-cluster-api-e2e-full-main

@sedefsavas sedefsavas mentioned this pull request Jun 6, 2022
Closed
4 tasks
@sbueringer
Copy link
Member

pull-cluster-api-e2e-workload-upgrade-1-24-latest-main is expected (also fails on main)

/test pull-cluster-api-e2e-full-main
(probably flake)

@palnabarun
Update controller-runtime to 0.12.1
1167c97 
- refactor code wherever needed
- recreate generated files
- book: document dependency updates and field removals

Signed-off-by: Nabarun Pal <pal.nabarun95@gmail.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2022
@k8s-ci-robot
Copy link
Contributor

@palnabarun: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-e2e-workload-upgrade-1-24-latest-main 6f42356 link false /test pull-cluster-api-e2e-workload-upgrade-1-24-latest-main
pull-cluster-api-apidiff-main 1167c97 link false /test pull-cluster-api-apidiff-main

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@sbueringer
Copy link
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 Jun 6, 2022
@sbueringer
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2022
@k8s-ci-robot k8s-ci-robot merged commit 82ee8f0 into kubernetes-sigs:main Jun 7, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.2 milestone Jun 7, 2022
@sbueringer
Copy link
Member

@palnabarun Thank you very much!

@sbueringer
Copy link
Member

xref #5968

@palnabarun palnabarun deleted the bump-cr-latest branch June 7, 2022 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbueringer sbueringer sbueringer left review comments

@fabriziopandini fabriziopandini fabriziopandini left review comments

@killianmuldoon killianmuldoon killianmuldoon left review comments

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

@stmcginnis stmcginnis Awaiting requested review from stmcginnis

Assignees

@sbueringer sbueringer

@fabriziopandini fabriziopandini

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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@palnabarun @sbueringer @killianmuldoon @k8s-ci-robot @fabriziopandini