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: Fix CRDs Go code generation #2339

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

lambdanis
Copy link
Contributor

@lambdanis lambdanis commented Apr 17, 2024

Commit a9c509f replaced execution of deprecated scripts with
k8s.io/code-generator/kube_codegen.sh. However, kube_codegen.sh doesn't generate
code on its own, but rather defines helper functions. To migrate to
kube_codegen.sh we'll need to call these functions from another script.
Additionally, it'll change with version 1.30 of k8s libraries - that's why I
opted to simply revert the change for now. When k8s libraries are updated to
1.30, the code generation scripts should be updated too.

This reverts commit a9c509f.

The reverted commit replaced execution of deprecated scripts with
k8s.io/code-generator/kube_codegen.sh. However, kube_codegen.sh doesn't generate
code on its own, but rather defines helper functions. To migrate to
kube_codegen.sh we'll need to call these functions from another script.
Additionally, it'll change with version 1.30 of k8s libraries - that's why I
opted to simply revert the change for now. When k8s libraries are updated to
1.30, the code generation scripts should be updated too.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
@lambdanis lambdanis added kind/bug Something isn't working release-note/misc This PR makes changes that have no direct user impact. labels Apr 17, 2024
@lambdanis lambdanis requested a review from a team as a code owner April 17, 2024 13:23
@lambdanis lambdanis requested a review from olsajiri April 17, 2024 13:23
CRD Go code generation wasn't working recently, so some changes to CRDs aren't
reflected in the generated helpers. This commit fixes that.

Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

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

Thanks Anna!

@mtardy
Copy link
Member

mtardy commented Apr 18, 2024

I can create an issue so that we don't forget to update, to note that it's marked as deprecated if you want :)

@lambdanis lambdanis merged commit 989c775 into cilium:main Apr 18, 2024
35 checks passed
@lambdanis
Copy link
Contributor Author

@mtardy sure, feel free to open an issue. K8s dependencies are generally updated together with cilium/cilium, right?

@mtardy
Copy link
Member

mtardy commented Apr 19, 2024

@mtardy sure, feel free to open an issue. K8s dependencies are generally updated together with cilium/cilium, right?

yet it should be for minor versions.

@mtardy
Copy link
Member

mtardy commented Apr 19, 2024

Feel free to edit the content if it's unclear #2349.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants