-
Notifications
You must be signed in to change notification settings - Fork 733
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
feat: helm chart podLabels #1126
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1126 +/- ##
==========================================
+ Coverage 47.84% 48.58% +0.74%
==========================================
Files 62 63 +1
Lines 4264 4339 +75
==========================================
+ Hits 2040 2108 +68
- Misses 1966 1975 +9
+ Partials 258 256 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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 the PR!
This LGTM, @sozercan LGTY?
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, small nit on the description
looks like DCO and CLA needs to be signed
cmd/build/helmify/static/README.md
Outdated
@@ -28,6 +28,7 @@ | |||
| audit.hostNetwork | Enables audit to be deployed on hostNetwork | `false` | | |||
| replicas | The number of Gatekeeper replicas to deploy for the webhook | `1` | | |||
| podAnnotations | The annotations to add to the Gatekeeper pods | `container.seccomp.security.alpha.kubernetes.io/manager: runtime/default` | | |||
| podLabels | The pods to add to the Gatekeeper pods | `{}` | |
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.
| podLabels | The pods to add to the Gatekeeper pods | `{}` | | |
| podLabels | The labels to add to the Gatekeeper pods | `{}` | |
@@ -28,6 +28,7 @@ | |||
| audit.hostNetwork | Enables audit to be deployed on hostNetwork | `false` | | |||
| replicas | The number of Gatekeeper replicas to deploy for the webhook | `1` | | |||
| podAnnotations | The annotations to add to the Gatekeeper pods | `container.seccomp.security.alpha.kubernetes.io/manager: runtime/default` | | |||
| podLabels | The pods to add to the Gatekeeper pods | `{}` | |
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.
| podLabels | The pods to add to the Gatekeeper pods | `{}` | | |
| podLabels | The labels to add to the Gatekeeper pods | `{}` | |
Signed-off-by: Julian Dolce <jdolce@qnx.com>
Signed-off-by: Julian Dolce <jdolce@qnx.com>
I signed it |
@sozercan @maxsmythe Pulled in the latest changes and updated the documentation, but having issues with Any insight into what is going on here? I have ran the test locally against a different cluster and it passed. I couldn't spot anything in the logs that might be the cause. |
What this PR does / why we need it:
Allow Helm chart users to add additional labels to pods. This is important for us so that specific labels are added to the Prometheus metrics, which allows us to see what environment alerts are coming from.
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #
Special notes for your reviewer:
Put this together quickly as an alternative to #1125, which modifies the generated files.
I followed the same pattern as the
podAnnotations
, and deleted thegatekeeper.labels
template as it was not being used. The alternative to this is that we remove thecommonLabels
from the kustomize patches and add the labels to each object either with a template or directly. Not using thecommonLabels
seems like it might be harder to maintain going forward. I am not a super big fan of string replacement in the main.go for all Deployments, but seems like the easiest to maintain.