-
Notifications
You must be signed in to change notification settings - Fork 152
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
Distribute CRDs using helm chart #1244
Conversation
CRDs for now were being deployed using controller, we should make sure CRDs are being distributed by helm chart preferrably but if we are consuming kanister not as sub chart in that case they can set an env var in the kanister pod to make sure CRDs are being created.
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 for working on this. I am sort of hesitant to let Helm manage CRDs because by default, Helm can't upgrade nor delete CRDs in future chart versions. We are not gaining much other than the "one chart" installation experience, and now users become responsible for upgrading the CRDs.
I wonder if we should default createCRDs
to true
to continue let the controller manage the CRDs, with false
being the 'break glass' approach. (Also see also comment below on upgrading existing Helm releases.) WDYT?
- rename helm field `createCRDs` to `updateCRDs` - refactor order of embedded files
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.
Looks great to me!
When we didn't have this `updateCRDs` flag in values.yaml controller was responsible to create/update the CRDs. To make sure that we follow the same default path we are changing the default value of `updateCRDs` to `true`, so that kanister controller will be responsible to manager crds. If thats not required/possible, this flag can be set to false.
I will let @pavannd1 have a look and then |
Change Overview
CRDs for now were being deployed using controller, we
should make sure CRDs are being distributed by helm
chart preferrably but if we are consuming kanister not
as sub chart in that case they can set an env var
in the kanister pod to make sure CRDs are being created.
Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan
Helm install manifests
the manifest can be found here.
with
createCRDs: true
the generated manifest can be found here