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

Drop Kubernetes v1.24 and support Kubernetes v1.27 #2182

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

tenzen-y
Copy link
Member

What this PR does / why we need it:
I upgraded the Kubernetes dependencies to v1.27. By this PR, we drop support for the Kubernetes v1.24.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2155

Blocked on #2177 and #2178
/hold

Checklist:

  • Docs included if any changes are user facing

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tenzen-y

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

@@ -114,33 +114,19 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
// addWatch adds a new Controller to mgr with r as the reconcile.Reconciler
func addWatch(mgr manager.Manager, c controller.Controller) error {
// Watch for changes to Experiment
err := c.Watch(&source.Kind{Type: &experimentsv1beta1.Experiment{}}, &handler.EnqueueRequestForObject{})
err := c.Watch(source.Kind(mgr.GetCache(), &experimentsv1beta1.Experiment{}), &handler.EnqueueRequestForObject{})
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +124 to +125
eventHandler := handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &experimentsv1beta1.Experiment{}, handler.OnlyControllerOwner())
if err = c.Watch(source.Kind(mgr.GetCache(), &trialsv1beta1.Trial{}), eventHandler); err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

The handler.EnqueueRequestForOwner{} typed was replaced with handler.EnqueueRequestForOwner() function:

https://github.com/kubernetes-sigs/controller-runtime/blob/116a1b831fffe7ccc3c8145306c3e1a3b1b14ffa/pkg/handler/enqueue_owner.go#L50-L62

@tenzen-y tenzen-y changed the title Drop Kubernetes v1.24 and support Kubernetes v1.27 WIP: Drop Kubernetes v1.24 and support Kubernetes v1.27 Jul 31, 2023
@andreyvelich
Copy link
Member

Thank you for this update @tenzen-y!
I will review this PR soon.
Should we combine your changes to drop support of K8s 1.23 and 1.24 in this single PR (Instead of this one: #2177)?

@tenzen-y
Copy link
Member Author

tenzen-y commented Jul 31, 2023

Thank you for this update @tenzen-y! I will review this PR soon. Should we combine your changes to drop support of K8s 1.23 and 1.24 in this single PR (Instead of this one: #2177)?

@andreyvelich Thanks for the quick response.
I think #2177 is worth it because we can easily roll back the library version if this change causes the bug.

@tenzen-y tenzen-y force-pushed the upgrade-k8s-v1.27 branch 5 times, most recently from ce91b7b to d933f58 Compare August 1, 2023 04:23
@tenzen-y tenzen-y changed the title WIP: Drop Kubernetes v1.24 and support Kubernetes v1.27 Drop Kubernetes v1.24 and support Kubernetes v1.27 Aug 1, 2023
@tenzen-y tenzen-y changed the title Drop Kubernetes v1.24 and support Kubernetes v1.27 WIP: Drop Kubernetes v1.24 and support Kubernetes v1.27 Aug 1, 2023
Comment on lines -43 to -51
// ExperimentDefaulter implements admission.DecoderInjector.
// A decoder will be automatically injected.

// InjectDecoder injects the decoder.
func (e *ExperimentDefaulter) InjectDecoder(d *admission.Decoder) error {
e.decoder = d
return nil
}

Copy link
Member Author

Choose a reason for hiding this comment

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


"github.com/kubeflow/katib/pkg/webhook/v1beta1/experiment"
"github.com/kubeflow/katib/pkg/webhook/v1beta1/pod"
)

func AddToManager(mgr manager.Manager, port int) error {
// Create a webhook server.
hookServer := &webhook.Server{
Copy link
Member Author

Choose a reason for hiding this comment

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

@tenzen-y tenzen-y changed the title WIP: Drop Kubernetes v1.24 and support Kubernetes v1.27 Drop Kubernetes v1.24 and support Kubernetes v1.27 Aug 1, 2023
@tenzen-y
Copy link
Member Author

tenzen-y commented Aug 1, 2023

@andreyvelich @johnugeorge This PR is ready for review. PTAL.

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
@tenzen-y
Copy link
Member Author

tenzen-y commented Aug 1, 2023

/hold cancel

Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

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

Thank you for updating K8s version @tenzen-y!
I left a few comments.

cmd/katib-controller/v1beta1/main.go Show resolved Hide resolved
go.mod Show resolved Hide resolved
@andreyvelich
Copy link
Member

Thank you @tenzen-y!
/lgtm
/hold for the review
/assign @johnugeorge @gaocegege

@johnugeorge
Copy link
Member

/lgtm

@andreyvelich
Copy link
Member

Let's merge it 🎉
/hold cancel

@google-oss-prow google-oss-prow bot merged commit f1e3f3a into kubeflow:master Aug 1, 2023
59 checks passed
@tenzen-y tenzen-y deleted the upgrade-k8s-v1.27 branch August 1, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubernetes versions for the next katib version
4 participants