-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[bitnami/zookeeper] allow for overriding namespace in zookeeper helm chart #3881
Conversation
@juan131 I think you may be interested in this one :-) Personally I like the idea, with some changes:
Note: I'm not Bitnami employee nor this repo co-maintainer, this is only my opinion, so don't treat this propositions as obligatory. |
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.
Hi @monicaruttle ,
Thank you very much for the PR!
Please take a look at the files carefully, there are some places where the namespace is used that also need to be changed.
Apart from that, I agree with @glothriel in use namespaceOverride
and a template to select the namespace instead of repeating the logic in all the files.
…template, found a few missed namespace references
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.
Hi @monicaruttle thanks for applying the changes.
Please, take a look at my comments.
Co-authored-by: Daniel Arteaga <dani8art.da@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.
LGTM
Description of the change
Allow for overriding the namespace that the ZooKeeper helm chart resources use.
Benefits
When ZooKeeper is included as a dependency of another helm chart, the preferred behaviour may be for it to be released into a different namespace than the parent. This change will allow for the parent chart values to override the namespace.
Possible drawbacks
Unknown
Applicable issues
Additional information
By default behaviour will not change from taking the Release.Namespace value for the namespace.
Checklist
Chart.yaml
according to semver.[bitnami/chart]
)values-production.yaml
apart fromvalues.yaml
, ensure that you implement the changes in both files