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

✨ (go/v3, go/v4-alpha and declarative/v1): Update k8s deps by upgrading controller-runtime from v0.12.2 to 0.13.0 and kubebuilder-declarative-pattern from fe5be9431eae158f86f9de23000a9a2ec06745fc to e0605f0e1a40f97293cb3773f57de695c8bc76af #2920

Merged
merged 1 commit into from
Sep 11, 2022

Conversation

oscr
Copy link
Contributor

@oscr oscr commented Sep 9, 2022

Updates:

  • controller-runtime
  • kubebuilder-declarative-pattern

There was no update for controller-tools, however I am watching it for releases.

I hope I correctly updated declarative-pattern

cc @camilamacedo86

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 9, 2022
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 9, 2022
@oscr
Copy link
Contributor Author

oscr commented Sep 9, 2022

/test pull-kubebuilder-e2e-k8s-1-25-0

@oscr
Copy link
Contributor Author

oscr commented Sep 9, 2022

Question: it seems like updating the dependencies fail with go1.17. I tried it locally and it seems to be the same result. Can we bump it?

@everettraven
Copy link
Contributor

@oscr It looks like we may need to update the Legacy unit tests action to use Go 1.19.0 instead of 1.17.3

@laxmikantbpandhare
Copy link
Member

@oscr It looks like we may need to update the Legacy unit tests action to use Go 1.19.0 instead of 1.17.3

@everettraven Looks like @oscr created another PR for the same #2911

@oscr
Copy link
Contributor Author

oscr commented Sep 9, 2022

@laxmikantbpandhare @everettraven Sorry I asked to break down #2911 but I was slow to close it.

@oscr
Copy link
Contributor Author

oscr commented Sep 9, 2022

@oscr It looks like we may need to update the Legacy unit tests action to use Go 1.19.0 instead of 1.17.3

No problem! I will update it immediately

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 9, 2022

@oscr: The following test 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-kubebuilder-e2e-k8s-1-19-16 d0dd84f link false /test pull-kubebuilder-e2e-k8s-1-19-16

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.

go.mod Outdated Show resolved Hide resolved
@oscr oscr mentioned this pull request Sep 10, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2022
@@ -84,7 +84,7 @@ manifests: controller-gen`
}

// latest version of controller-runtime where v1beta1 is supported
const controllerRuntimeVersionForVBeta1 = "v0.9.2"
const controllerRuntimeVersionForVBeta1 = "v0.13.0"
Copy link
Member

Choose a reason for hiding this comment

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

@oscr this one we cannot update.
The kubebuilder CLI has an option to create the projects using crd/webhooks API v1beta1 which is no longer provided since k8s 1.22. This logic will update the go.mod file to use the deps that works with v1beta1 if/when someone calls the command to create api [options] crd-version=v1beta1

Therefore, could you please revert it and also add a comment on the top to explain it so that others in the future does not try to update this one?

Example (something like):

// DO NOT UPDATE THIS VERSION
// Note that this implementation will update the go.mod to downgrade the versions for those that are
// compatible v1beta1 CRD/Webhooks k8s core APIs if/when an user tries to create a API with
// create api [options] crd-version=v1beta1. The flag/feature is deprecated. however, to ensure that backwards 
// compatible we must introduce this logic. Also, note that when we bump the k8s dependencies we need to
// ensure that the following replacements will be done accordingly to downgrade the versions. 
// The next version of the Golang base plugin (go/v4-alpha) no longer provide this feature. 

Copy link
Member

Choose a reason for hiding this comment

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

/hold

Copy link
Member

Choose a reason for hiding this comment

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

Btw, we have been removing the testdata and samples with this option because it is deprecated.
So, that it did not fail in the tests but we need to ensure that we have a testdata that checks it.
The ci should fail with this change. I am adding an issue so we are able to ensure that.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2022
@@ -34,7 +34,7 @@ import (

const (
// ControllerRuntimeVersion is the kubernetes-sigs/controller-runtime version to be used in the project
ControllerRuntimeVersion = "v0.12.2"
ControllerRuntimeVersion = "v0.13.0"
// ControllerToolsVersion is the kubernetes-sigs/controller-tools version to be used in the project
ControllerToolsVersion = "v0.9.2"
Copy link
Member

@camilamacedo86 camilamacedo86 Sep 11, 2022

Choose a reason for hiding this comment

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

@oscr could we also update the controller tools here to the latest version which uses k8s 1.25?
We need to ask for a new release, see: https://github.com/kubernetes-sigs/controller-tools
I added an issue for that see: #2926

@camilamacedo86 camilamacedo86 changed the title ✨ Update controller-runtime, kubebuilder-declarative-… ✨ (go/v3, go/v4-alpha and declarative/v1): Update k8s deps by upgrading controller-runtime from v0.12.2 to 0.13.0 and kubebuilder-declarative-pattern from fe5be9431eae158f86f9de23000a9a2ec06745fc to e0605f0e1a40f97293cb3773f57de695c8bc76af Sep 11, 2022
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.

/hold cancel

/approved
/lgtm

@oscr thank you for the contribution 🥇
PS.: I hope that you do not mind but I just update the title for we are able to generate the release notes automatically with the info (see that I added the name of the plugins changed/affected by)

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 11, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, oscr

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 Sep 11, 2022
@camilamacedo86 camilamacedo86 merged commit e7c871e into kubernetes-sigs:master Sep 11, 2022
@oscr oscr deleted the controller-runtime branch September 11, 2022 17:01
@oscr
Copy link
Contributor Author

oscr commented Sep 11, 2022

@camilamacedo86 No, I don't mind at all. Thank you for updating the title to better reflect the content.

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

Successfully merging this pull request may close these issues.

5 participants