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

k8s: Add support for Kubernetes 1.30.0 #31687

Merged
merged 2 commits into from
Apr 24, 2024
Merged

k8s: Add support for Kubernetes 1.30.0 #31687

merged 2 commits into from
Apr 24, 2024

Conversation

christarazi
Copy link
Member

@christarazi christarazi commented Mar 30, 2024

Update to k8s 1.30.0 and fix code-generator scripts.

Also, bump sigs.k8s.io/controller-runtime to the latest main version as
of this commit (v0.17.1-0.20240327193027-21368602d84b) to workaround a
breaking change introduced in 1.30 libraries.

Co-authored-by: André Martins andre@cilium.io
Signed-off-by: Chris Tarazi chris@isovalent.com

@christarazi christarazi added sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/misc This PR makes changes that have no direct user impact. labels Mar 30, 2024
@christarazi christarazi force-pushed the pr/christarazi/k8s-1.30 branch 2 times, most recently from 62d3b4e to a08fdd9 Compare April 1, 2024 23:42
@christarazi christarazi added release-note/major This PR introduces major new functionality to Cilium. and removed release-note/misc This PR makes changes that have no direct user impact. labels Apr 1, 2024
@christarazi christarazi force-pushed the pr/christarazi/k8s-1.30 branch 4 times, most recently from 186512a to 9e23df8 Compare April 2, 2024 01:04
.github/actions/aws/k8s-versions.yaml Outdated Show resolved Hide resolved
.github/actions/ginkgo/main-focus.yaml Show resolved Hide resolved
.github/actions/ginkgo/main-k8s-versions.yaml Outdated Show resolved Hide resolved
.github/actions/ginkgo/main-k8s-versions.yaml Outdated Show resolved Hide resolved
.github/actions/ginkgo/main-k8s-versions.yaml Outdated Show resolved Hide resolved
.github/actions/ginkgo/main-k8s-versions.yaml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
test/provision/k8s_install.sh Show resolved Hide resolved
@aanm

This comment was marked as outdated.

@christarazi christarazi force-pushed the pr/christarazi/k8s-1.30 branch 2 times, most recently from ee143b5 to 688dcd3 Compare April 9, 2024 06:41
@christarazi christarazi requested a review from aanm April 16, 2024 19:34
@christarazi

This comment was marked as outdated.

@christarazi christarazi marked this pull request as ready for review April 16, 2024 19:34
@christarazi christarazi requested review from a team as code owners April 16, 2024 19:34
@christarazi christarazi force-pushed the pr/christarazi/k8s-1.30 branch 3 times, most recently from 4afcaa1 to ae70e45 Compare April 18, 2024 17:05
@christarazi christarazi requested a review from aanm April 18, 2024 17:07
@christarazi
Copy link
Member Author

/test

@christarazi
Copy link
Member Author

@tklauser @jschwinger233, could I get your sign off here?

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Some minor nits and observations from the Make target pieces. I feel like the make targets we have here are fighting with the upstream k8s codegen tooling a bit. Non-blocking feedback, we can iterate further in-tree.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@joestringer
Copy link
Member

joestringer commented Apr 18, 2024

Are these errors printed in the (passing) Generate k8s API CI workflow a problem?

contrib/scripts/k8s-code-gen.sh "/tmp/cilium.tmphCFo2Q9B"
SCRIPT_ROOT=/home/runner/work/cilium/cilium/src/github.com/cilium/cilium/contrib/scripts/../..
Generating client code for 4 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/slim/k8s/client/clientset: No such file or directory
Generating lister code for 4 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/slim/k8s/client/listers: No such file or directory
Generating informer code for 4 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/slim/k8s/client/informers: No such file or directory
Generating client code for 1 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/slim/k8s/apiextensions-client/clientset: No such file or directory
Generating lister code for 1 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/slim/k8s/apiextensions-client/listers: No such file or directory
Generating informer code for 1 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/slim/k8s/apiextensions-client/informers: No such file or directory
Generating client code for 2 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/client/clientset: No such file or directory
Generating lister code for 2 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/client/listers: No such file or directory
Generating informer code for 2 targets
grep: /tmp/cilium.tmphCFo2Q9B/github.com/cilium/cilium/pkg/k8s/client/informers: No such file or directory

pkg/k8s/slim/README.md Show resolved Hide resolved
@christarazi
Copy link
Member Author

christarazi commented Apr 19, 2024

@joestringer From my understanding, these errors are expected. I analyszed the exectuion of the script using set -x and traced it down to the following snippet:

    ( kube::codegen::internal::grep -l --null \
        -e '^// Code generated by client-gen. DO NOT EDIT.$' \
        -r "${out_dir}/${clientset_subdir}" \
        --include '*.go' \
        || true \
    ) | xargs -0 rm -f

It seems to me that this snippet is trying to find any files under "${out_dir}/${clientset_subdir}" as long as they match the regex string, then delete them, hence the xargs rm pipe. Notice the true at the end of the grep command, which suggests that the overall script should not fail if the grep fails, presumably if the path does not exist. However, the path is not expected to exist because the subsequent step after the snippet above is to generate the code. In other words, this snippet is to remove any leftover files before beginning the code generation. These errors are expected in summary.

We did not see these errors prior to this PR because we did not directly use this script.

@aanm
Copy link
Member

aanm commented Apr 22, 2024

@christarazi needs rebase

@christarazi
Copy link
Member Author

christarazi commented Apr 22, 2024

christarazi and others added 2 commits April 23, 2024 11:49
Update to k8s 1.30.0 and fix code-generator scripts.

Also, bump sigs.k8s.io/controller-runtime to the latest main version as
of this commit (v0.17.1-0.20240327193027-21368602d84b) to workaround a
breaking change introduced in 1.30 libraries.

Co-authored-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
While updating the slim structures, there are a few gotchas to keep in
mind. If a developer fails to follow the advice, the generated changes
from `make generate-k8s-api` leads to very hard to debug/understand
errors.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi
Copy link
Member Author

Updated to final v1.30.0 from the rc.1. No changes in the slim structures as well.

@christarazi
Copy link
Member Author

christarazi commented Apr 23, 2024

/test

Edit:

Copy link
Member

@ysksuzuki ysksuzuki left a comment

Choose a reason for hiding this comment

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

CEGP CRD change looks good to me

@aanm aanm added this pull request to the merge queue Apr 24, 2024
Merged via the queue into main with commit 5d61adf Apr 24, 2024
276 of 278 checks passed
@aanm aanm deleted the pr/christarazi/k8s-1.30 branch April 24, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/major This PR introduces major new functionality to Cilium. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants