-
Notifications
You must be signed in to change notification settings - Fork 124
feat: generate crd with version annotation. #1134
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
Conversation
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Hi @zetxqx. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
I guess with this self-written code generator approach, you need to change the |
@zetxqx can you please explain the motivation behind this PR? why should we move from a the common code generation libraries which are widely used in the community to having our own code for generation? |
@nirrozenbaum I was following what we have in the gateway-api(https://github.com/kubernetes-sigs/gateway-api/blob/main/pkg/generator/main.go) Currently we only have one active version and do not have any channels so this PR does not have much benefits. However, if we have more channels and versions to maintain, the benefit is we can have a single place to "consts.go" in this PR to change the version related stuff. CC: @robscott |
If we want to keep the common code generation libraries which are widely used in the community, one approach I can think of is that keep the pkg/consts.go and let conformance test use it, make sure that the version in pkg.consts.go be the same with the version in kubebuilder annotation comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zetxqx!
RELEASE.md
Outdated
@@ -2,6 +2,7 @@ | |||
|
|||
The Kubernetes Template Project is released on an as-needed basis. The process is as follows: | |||
|
|||
1. Update `pkg/consts/consts.go` with the new semver tag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this they'll also have to manually regenerate the CRDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zetxqx!
@@ -95,12 +95,8 @@ help: ## Display this help. | |||
|
|||
##@ Development | |||
|
|||
.PHONY: manifests | |||
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kfswain @nirrozenbaum Do you need anything beyond CRDs that could have been generated here? (I'm assuming not, certainly not a webhook, and probably not RBAC, but just want to make sure I'm not missing some EPP dependencies here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing that I'm aware of.
sorry for the delayed response, missed the question.
/hold hold to coordinate with #1156 not affecting the review cc: @capri-xiyue |
@zetxqx #1156 got merged. Seems like you need to merge with the main branch and then run |
/lgtm |
Add the CRD annotation for gateway-api-inferecence-extention CRD
The generator is mostly copied from https://github.com/kubernetes-sigs/gateway-api/blob/main/pkg/generator/main.go but removed the channel support. Since the version is needed for conformance tests. I removed the channel support to make this a minimum change.