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

Upgrade Kubernetes to v1.30.7 #2463

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Electronic-Waste
Copy link
Member

@Electronic-Waste Electronic-Waste commented Dec 9, 2024

What this PR does / why we need it:

  • Kubernetes upgrade to v1.30.7
  • controller-runtime upgrade to v0.18.5
  • Adapt controllers to use controller-runtime generics API
  • Adapt code generation to new kube_codegen.sh script

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 #2457

Checklist:

  • Docs included if any changes are user facing

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@Electronic-Waste
Copy link
Member Author

/rerun-all

@Electronic-Waste Electronic-Waste marked this pull request as ready for review December 9, 2024 15:23
@Electronic-Waste
Copy link
Member Author

PTAL if you have time👀 @kubeflow/wg-automl-leads

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 this @Electronic-Waste!
I left my initial comments.
/assign @kubeflow/wg-training-leads @helenxie-bit

@@ -33,6 +33,6 @@ jobs:
strategy:
fail-fast: false
matrix:
kubernetes-version: ["v1.27.11", "v1.28.7", "v1.29.2"]
kubernetes-version: ["v1.28.7", "v1.29.2", "v1.30.7"]
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to migrate to k8s 1.31 in the followup PR ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'd like to. But I'm busy this week. If it's not urgent, I'll raise another PR to migrate to k8s 1.31 next week.

Copy link
Member

Choose a reason for hiding this comment

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

I think that step by step upgrading would be better so that we can easily pick the commit when we find the issue on the specific K8s version.

So, I would recommend upgrading 1 K8s version in 1 PR.

go.mod Outdated Show resolved Hide resolved
hack/update-openapigen.sh Outdated Show resolved Hide resolved
@@ -0,0 +1,39 @@
API rule violation: list_type_missing,github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1,AlgorithmSpec,AlgorithmSettings
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to track in a issue to resolve these violations similar to Training: kubeflow/training-operator#2317

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I'll create an issue describing this after this PR get merged.

@helenxie-bit
Copy link
Contributor

Thank you for the contribution! LGTM 🎉

go.mod Outdated
k8s.io/apimachinery v0.30.7
k8s.io/client-go v0.30.7
k8s.io/code-generator v0.30.7
k8s.io/klog/v2 v2.130.1
Copy link
Member

Choose a reason for hiding this comment

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

Reverting klog version, then create dedicated issue for the klog version updating.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it

Signed-off-by: Electronic-Waste <[email protected]>
@@ -33,6 +33,6 @@ jobs:
strategy:
fail-fast: false
matrix:
kubernetes-version: ["v1.27.11", "v1.28.7", "v1.29.2"]
kubernetes-version: ["v1.28.7", "v1.29.2", "v1.30.7"]
Copy link
Member

Choose a reason for hiding this comment

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

We can also add one that tests whatever is the cutting edge version so we can get notified of any incompatibility earlier

Copy link
Member Author

@Electronic-Waste Electronic-Waste Dec 16, 2024

Choose a reason for hiding this comment

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

SGTM. The newest version of k8s is 1.32.0 now. WDYT👀 @kubeflow/wg-automl-leads

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late response. We upgrade the Kube version step by step.
So, I think that it would be great to use the 1.32 in separate issue.

I believe that @shaowei-su can open a PR to update Kube to 1.31 after this PR.
Then you can open PR to support 4th supporting Kube version, which meant that we support 1.29, 1.30, 1.31, 1.32 versions the same as training-operator.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we discussed it today during the call.
Let's initially support 1.28 - 1.31 in Katib 0.18 as we discussed here: #2457.
In the future, we can migrate to k8s 1.32.

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM. Can we work on this PR and merge it as soon as possible? The deadline for Kubeflow 1.10 feature freeze is Jan 20th. We may need to reserve some time for 1.31 version:)

@Electronic-Waste
Copy link
Member Author

/rerun-all

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 this @Electronic-Waste
Overall lgtm.
/assign @tenzen-y @helenxie-bit @kubeflow/wg-training-leads

@@ -82,7 +82,7 @@ sync-go-mod:
CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
.PHONY: controller-gen
controller-gen:
@GOBIN=$(shell pwd)/bin GO111MODULE=on go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.14.0
@GOBIN=$(shell pwd)/bin GO111MODULE=on go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.16.5

# Run this if you update any existing controller APIs.
# 1. Generate deepcopy, clientset, listers, informers for the APIs (hack/update-codegen.sh)
Copy link
Member

Choose a reason for hiding this comment

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

Please can we add the go-mod-download instruction to make generate similar to Training: https://github.com/kubeflow/training-operator/blob/master/Makefile#L50.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it! I will add it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Kubernetes v1.28 - v1.31
5 participants