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 17 commits into
base: master
Choose a base branch
from

Conversation

Electronic-Waste
Copy link
Member

What this PR does / why we need it:

  • Kubernetes upgrade to v1.30.7
  • controller-runtime upgrade to v0.18.5
  • klog upgrade to v2
  • 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

Signed-off-by: Electronic-Waste <2690692950@qq.com>
…all.

Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
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

Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
@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
go 1.22
go 1.22.0

toolchain go1.23.2
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess, it is added automatically by my go environment. I'll remove it.


echo "Generating swagger file for ${VERSION} ..."
go run "${PROJECT_ROOT}/hack/swagger/main.go" "${VERSION}-${SWAGGER_VERSION}" "${VERSION}" >"${SWAGGER_CODEGEN_FILE}"
kube::codegen::gen_openapi \
Copy link
Member

Choose a reason for hiding this comment

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

Let's use the openapi binary directly here instead of kube::codegen similar to Training Operator as we discussed here: kubeflow/training-operator#2324 (comment)

Copy link
Member Author

@Electronic-Waste Electronic-Waste Dec 11, 2024

Choose a reason for hiding this comment

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

SGTM. I'll change to use openapi binary.

@@ -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.

Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the files in pkg/client/controller/applyconfiguration/common/v1beta1 no longer needed, or have they been replaced/generated in another location?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess, they are no longer needed since there are no +genclient annotations in pkg/apis/controller/common/v1beta1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it! That makes sense.

@helenxie-bit
Copy link
Contributor

Thank you for the contribution! LGTM 🎉

Signed-off-by: Electronic-Waste <2690692950@qq.com>
Signed-off-by: Electronic-Waste <2690692950@qq.com>
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 <2690692950@qq.com>
@@ -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

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