-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Adding annotations support to helm chart configmaps #5178
Conversation
Signed-off-by: Greg Sidelinger <[email protected]>
Signed-off-by: Greg Sidelinger <[email protected]>
Welcome @gregsidelinger! |
Hi @gregsidelinger. Thanks for your PR. I'm waiting for a kubernetes 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/test-infra repository. |
/ok-to-test |
ping @ChiefAlexander |
Codecov Report
@@ Coverage Diff @@
## master #5178 +/- ##
==========================================
+ Coverage 58.61% 58.64% +0.02%
==========================================
Files 88 88
Lines 6872 6872
==========================================
+ Hits 4028 4030 +2
+ Misses 2402 2401 -1
+ Partials 442 441 -1
Continue to review full report at Codecov.
|
@gregsidelinger thank you for the PR. We are still in the process of migrating the helm chart into the repository. For that reason, approval/merging of this PR could take some days. |
@aledbf I'm going to assume all of the tests that failed are from the in flight move based on what I saw and can be re-ran once they are fixed. Let me know if there is anything needed from me or if the new settings should be moved to other locations in the values file. |
That's correct. Once I fixed the CI jobs I'll let you know if a change is required. |
/retest |
3 similar comments
/retest |
/retest |
/retest |
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 looks good to me. Thanks for doing the first PR to the chart here!
/lgtm |
@gregsidelinger thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, gregsidelinger The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
Signed-off-by: Greg Sidelinger [email protected]
What this PR does / why we need it:
We are using harness.io for deployments and they actually rename configmaps by default to allow for multiple versions of the app to be deployed at a time. In order to disable this feature I need to be able to add an annotation on the configmap as documented here. This PR adds support for annotations on the custom config, tcp and ucp configmaps as their names are passed in via command line switches.
Types of changes
Which issue/s this PR fixes
How Has This Been Tested?
I tested the chart updates locally.
Checklist: