-
Notifications
You must be signed in to change notification settings - Fork 104
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
Generate KUDO Manifests #1582
Generate KUDO Manifests #1582
Conversation
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
…ith dev Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Makefile
Outdated
rm -rf hack/code-gen | ||
rm -rf ./hack/code-gen | ||
|
||
generate-manifests: |
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.
This target makes a lot of sense to me but why do we need to commit the manifests as part of the repo? We should have only one source of truth and that's kudo init
for now.
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.
this is exactly consistent with our CRD generation
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.
if anything, I'd like to get rid of the config/crds
folder altogether. we only need for tests but once we have a beforeAll
step in kuttl, we could remove the in favor of kudo init
call. I'd rather not add new manifests there
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.
I mostly agree - things that can be generated shouldn't be checked in usually.
I'm ok with having them checked in, especially the CRDs, as the controller-gen step takes quite a while. Would be nicer to not have them checked in though.
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.
The generate-manifests
target is great, however, I don't think we should have manifests as part of the repo.
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
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.
🚢
Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
We already generate CRDs and have a target for that. For dev only, it makes sense to generate needed manifests. This target and script, generates all the kube objects manifests, separates them by file and renames them to logical names.
When we have agreement on this.... we can add another target to quickly setup a cluster for dev. It would likely be something like:
Then we can update #1581 to use this manifest file for updates.
The startup of a dev env should be something like:
Signed-off-by: Ken Sipe kensipe@gmail.com