search

kubernetes-sigs / kubebuilder-declarative-pattern

A toolkit for building declarative operators with kubebuilder
Apache License 2.0
255 stars 84 forks source link

bump k8s to 1.22 #169

Closed atoato88 closed 3 years ago

atoato88 commented 3 years ago

What this PR does / why we need it:

This PR bumps k8s modules to 1.22.0

Special notes for your reviewer:

If this PR is too fast to merge because of other repository doesn't bump modules to 1.22, please add /hold tag.

Update: I close this PR and will create other PR once controller-runtime release-0.10 tag is released.

atoato88 commented 3 years ago

Oh, should I update controller-runtime version in this PR?

justinsb commented 3 years ago

Thanks for doing this @atoato88 ... I do think you're right in that we should try to follow the same k8s version as the controller-runtime version we're using. My reasoning is that because client-go doesn't use true go module versioning, if there was a breaking change between client-go 1.21 and 1.22, and we were using 1.22 but controller-runtime was using 1.21, we might end up where the code can't be compiled.

It does look like controller-runtime was recently updated (in https://github.com/kubernetes-sigs/controller-runtime/pull/1626), but it also looks like that version hasn't yet been tagged.

I suggest once the version of controller-runtime is tagged, that we merge this change and update controller-runtime at the same time. However, we could update controller-runtime now by pinning to a SHA...

I also suggest that we should start thinking about versioning and tagging in this repository also. I think it is probably easiest to align with controller runtime. which describes their strategy here: https://github.com/kubernetes-sigs/controller-runtime/blob/master/VERSIONING.md But in short I suggest we maintain a release-0.9 branch that works with controller-runtime-0.9, and in future a release-0.10 branch that works with controller-runtime 0.10.

If we want to do that, the big upside of doing that would be that master would probably be the 0.10 branch before it is released, which would use a SHA reference to the controller-runtime master branch.

That's a lot of words to say I think (1) we should think about release branches here and (2) if we are going to do release branches, we can merge this PR (but you should also update controller-runtime to the latest from the master branch in this PR).

atoato88 commented 3 years ago

Thanks for doing this @atoato88 ... I do think you're right in that we should try to follow the same k8s version as the controller-runtime version we're using. My reasoning is that because client-go doesn't use true go module versioning, if there was a breaking change between client-go 1.21 and 1.22, and we were using 1.22 but controller-runtime was using 1.21, we might end up where the code can't be compiled. It does look like controller-runtime was recently updated (in https://github.com/kubernetes-sigs/controller-runtime/pull/1626), but it also looks like that version hasn't yet been tagged.

Yes I agree with you and my PR was too fast timing. 😅

I suggest once the version of controller-runtime is tagged, that we merge this change and update controller-runtime at the same time. However, we could update controller-runtime now by pinning to a SHA...

Yes, I think that we should update modules for this repository once controller-runtime is tagged.

As of now, should I close this PR and create other PR on that time? or hold this PR temporary?

justinsb commented 3 years ago

As of now, should I close this PR and create other PR on that time? or hold this PR temporary?

Actually I think if you want to update this PR to pull in the latest controller-runtime on the main/master branch (which uses k8s 1.22), then I think we can merge this. If we agree that the versioning procedure I outline above makes sense, our master branch will become release-0.10 once controller-runtime tags their release-0.10.

We can also take a release-0.9 branch from immediately before this PR merges, if we decide we need that. But I think just aiming for a solid release-0.10 (and then tagging 0.10.0 etc from it) is probably better.

atoato88 commented 3 years ago

Actually I think if you want to update this PR to pull in the latest controller-runtime on the main/master branch (which uses k8s 1.22), then I think we can merge this. If we agree that the versioning procedure I outline above makes sense, our master branch will become release-0.10 once controller-runtime tags their release-0.10.

The motivation that I create this PR is catch up with latest controller-runtime which is tagged. Previously, some PR bumping some modules was along with same policy, I think.

I agree with your versioning procedure comment. So, I close this PR and create other PR once controller-runtime release-0.10 tag is released.

atoato88 commented 3 years ago

/close Thank you to discussion. I close this PR and will create other PR once controller-runtime release-0.10 tag is released.

k8s-ci-robot commented 3 years ago

@atoato88: Closed this PR.

In response to [this](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/pull/169#issuecomment-900693742): >/close >Thank you to discussion. >I close this PR and will create other PR once controller-runtime release-0.10 tag is released. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
varshaprasad96 commented 3 years ago

@atoato88 controller-runtime released 0.10.0 just yesterday. Can we reopen the PR?

atoato88 commented 3 years ago

/reopen Latest controller-runtime which taged v0.10.0 is released. I reopen this PR.

k8s-ci-robot commented 3 years ago

@atoato88: Reopened this PR.

In response to [this](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/pull/169#issuecomment-912326736): >/reopen >Latest controller-runtime which taged `v0.10.0` is released. >I reopen this PR. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
atoato88 commented 3 years ago

/retest

175 is merged, I retest on this PR.

atoato88 commented 3 years ago

Oh, it isn't prow job.. I've rebased now. :sweat_smile:

k8s-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atoato88, estroz

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: - ~~[OWNERS](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/master/OWNERS)~~ [atoato88,estroz] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
estroz commented 3 years ago

/hold

estroz commented 3 years ago

/hold cancel