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

✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.0 to 1.23.5 #2575

Merged
merged 1 commit into from Mar 30, 2022
Merged

✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.0 to 1.23.5 #2575

merged 1 commit into from Mar 30, 2022

Conversation

NikhilSharmaWe
Copy link
Member

Fixes #2502

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 30, 2022
@camilamacedo86
Copy link
Member

camilamacedo86 commented Mar 30, 2022 •
edited

@NikhilSharmaWe,

Just 3 nits.

a) Since it is just a bump and when we see that the k8s dep was from MAJOR.MINOR.z to z+ we know that it is only to get bug fixes. Then, in this way the title here would be something like:

✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.9 to 1.23.5

b) We also need to bump it for the tests and release

c) Would be great we also update the k8s version used in the ci test (e2e) done with prow, see: https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes-sigs/kubebuilder/kubebuilder-presubmits.yaml#L40

I think the projects /scaffold will only change the dep when we bump controller runtime.
So, we need to bump the controller-runtime as a follow up to close the issue.

Could you help us with that?

@NikhilSharmaWe NikhilSharmaWe changed the title 🐛 Bumped sigs.k8s.io/controller-runtime and k8s dependencies to get bug fixes ✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.9 to 1.23.5 Mar 30, 2022
@NikhilSharmaWe NikhilSharmaWe changed the title ✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.9 to 1.23.5 ✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.3 to 1.23.5 Mar 30, 2022
camilamacedo86
camilamacedo86 reviewed Mar 30, 2022
View reviewed changes
pkg/plugins/golang/v3/commons.go Outdated
@@ -78,7 +78,7 @@ manifests: controller-gen`
}

if err := util.ReplaceInFile("Makefile",
"ENVTEST_K8S_VERSION = 1.23",
"ENVTEST_K8S_VERSION = 1.23.5",
Copy link
Member

Choose a reason for hiding this comment

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

@NikhilSharmaWe sorry, this one we will only update when we update the plugin scaffolds
This implementation is because we have a feature to still use v1beta1 CRD APIs and it is removed from the latest versions so that if someone uses the feature we need to replace the scaffold to try to downgrade it all.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@NikhilSharmaWe
Copy link
Member Author

NikhilSharmaWe commented Mar 30, 2022 •
edited

@camilamacedo86 Updated k8s version as suggested to 1.23.5, but some tests are failing, what could be the reason?

camilamacedo86
camilamacedo86 reviewed Mar 30, 2022
View reviewed changes
pkg/plugins/golang/v3/commons.go Outdated
@@ -78,7 +78,7 @@ manifests: controller-gen`
}

if err := util.ReplaceInFile("Makefile",
"ENVTEST_K8S_VERSION = 1.23",
"ENVTEST_K8S_VERSION = 1.23.3",
Copy link
Member

Choose a reason for hiding this comment

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

this change will also change the plugin scaffolds so we need to do in the follow up,
See that it is failing in the test data generate check: https://github.com/kubernetes-sigs/kubebuilder/runs/5760100478?check_suite_focus=true#step:6:895

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, by mistake.

@camilamacedo86
Copy link
Member

/test pull-kubebuilder-test

@camilamacedo86
Copy link
Member

/test pull-kubebuilder-e2e-k8s-1-23-3

camilamacedo86
camilamacedo86 reviewed Mar 30, 2022
View reviewed changes
test/common.sh Outdated
@@ -25,7 +25,7 @@ function convert_to_tools_ver {
# Tests in 1.20 and 1.21 with their counterpart version's apiserver.
"1.20"|"1.21") echo "1.19.2";;
"1.22") echo "1.22.1";;
"1.23") echo "1.23.3";;
"1.23") echo "1.23.5";;
Copy link
Member

Choose a reason for hiding this comment

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

@NikhilSharmaWe
Sorry, this one will need to keep in a follow-up after we have it. That was my mistake . Could we revert?
See that we are facing an issue because we are trying to get kubebuilder-tools-1.23.5-darwin-amd64.tar.gz

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was thinking that too

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@camilamacedo86 camilamacedo86 changed the title ✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.3 to 1.23.5 ✨ (Kubebuilder CLI) upgrade k8s deps from 1.23.0 to 1.23.5 Mar 30, 2022
camilamacedo86
camilamacedo86 approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

That shows great 🥇

/approved

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 30, 2022
rashmigottipati
rashmigottipati approved these changes Mar 30, 2022
View reviewed changes
Copy link
Contributor

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, NikhilSharmaWe, rashmigottipati

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 merged commit 3209f54 into kubernetes-sigs:master Mar 30, 2022
@rashmigottipati
Copy link
Contributor

@NikhilSharmaWe when you submit a PR to update KIND_K8S_VERSION, you need to update the following:
line 28 and line 48

@NikhilSharmaWe NikhilSharmaWe mentioned this pull request Mar 31, 2022
Merged
@NikhilSharmaWe NikhilSharmaWe mentioned this pull request Apr 7, 2022
Merged
@ryantking ryantking mentioned this pull request Apr 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rashmigottipati rashmigottipati rashmigottipati approved these changes

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@estroz estroz Awaiting requested review from estroz

Assignees

@rashmigottipati rashmigottipati

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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bump k8s 1.23.3 to get bug fixes
4 participants
@NikhilSharmaWe @camilamacedo86 @k8s-ci-robot @rashmigottipati