-
Notifications
You must be signed in to change notification settings - Fork 1.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
[WIP] Improve configmap with helm #15825
Conversation
Signed-off-by: Masaki Muranaka <[email protected]>
This is just a example for discussing configMapHelper. Do not apply this patch to the master branch. Signed-off-by: Masaki Muranaka <[email protected]>
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@tolusha please take a look |
@@ -89,6 +89,9 @@ global: | |||
chePluginRegistryUrlFormat: "plugin-registry-%s.%s" | |||
|
|||
che: | |||
properties: |
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.
Why not ?
properties: {}
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 fix was applied by da9c72d and its comment is:
This is just a example for discussing configMapHelper. Do not apply this patch to the master branch.
That commit is just for explaining the usage. It must be reverted before merged.
I agree properties: {}
will be the best choice.
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.
@monaka
Could you pls fix that? I would like to merge the PR )
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.
@tolusha Thanks for heads up. I found a minor bug in this PR.
I'll fix it within a few days and will re-request review.
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.
@monaka
Can we helm with anything?
Co-Authored-By: Masaki Muranaka <[email protected]>
Issues go stale after Mark the issue as fresh with If this issue is safe to close now please do so. Moderators: Add |
What does this PR do?
Generating the name of the environment variables from
values.yaml
on the deployment by Helm.What issues does this PR fix or reference?
eclipse#15824
Release Notes
Should be written.
Docs PR
Not checked yet. I guess some modification will be required.