feat: separate podlabels in controller-manager and audit deployment #3378
Conversation
bobertrublik
changed the title
bobertrublik
force-pushed
the
separate-deployment-labels
branch
from
0b23d4f
to
23d1737
Compare
bobertrublik
force-pushed
the
separate-deployment-labels
branch
from
23d1737
to
3b6e6b1
Compare
| obj = strings.Replace(obj, " priorityClassName: system-cluster-critical", " {{- if .Values.audit.priorityClassName }}\n priorityClassName: {{ .Values.audit.priorityClassName }}\n {{- end }}", 1) | ||
| obj = strings.Replace(obj, " - emptyDir: {}", " {{- if .Values.audit.writeToRAMDisk }}\n - emptyDir:\n medium: Memory\n {{ else }}\n - emptyDir: {}\n {{- end }}", 1) | ||
| } | ||
|
|
||
| if kind == DeploymentKind { |
There was a problem hiding this comment.
Thanks for the PR @bobertrublik! We need to ensure backward compatibilities as to not break existing things users might be using and reduce developer toil. can you pls bring back all the existing podLabels and add support for new audit and controller manager podLabels?
There was a problem hiding this comment.
Of course, I'm just a little unsure how to approach this. Are you saying that I should readd the .Values.podLabels field to ensure backward compatibility? Are the labels set there added to all pods and we can additionally set the .Values.controllerManager.podLabels and .Values.audit.podLabels to add specific ones?
There was a problem hiding this comment.
yes, Pod can have many labels. Please restore the existing values.podLabels field to ensure backward compatibility. Then add the new labels.
There was a problem hiding this comment.
I restored .values.podLabels and kept .Values.controllerManager.podLabels and .Values.audit.podLabels. 🙂
bobertrublik
force-pushed
the
separate-deployment-labels
branch
4 times, most recently
from
dacf5ee
to
5be0b6a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3378 +/- ##
==========================================
- Coverage 54.49% 48.23% -6.27%
==========================================
Files 134 219 +85
Lines 12329 14989 +2660
==========================================
+ Hits 6719 7230 +511
- Misses 5116 6946 +1830
- Partials 494 813 +319
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
bobertrublik
force-pushed
the
separate-deployment-labels
branch
from
5be0b6a
to
f6b92d8
Compare
|
Hello @ritazh, I rebased the branch onto main and wanted to ask you for a short review and approval. 🙂 |
|
@bobertrublik looks like there are a few merge conflicts, could you ptal when you get a chance? thanks! |
bobertrublik
force-pushed
the
separate-deployment-labels
branch
from
2f06c10
to
344cc11
Compare
|
Thanks for approving, I fixed the conflicts. 🙂 |
Signed-off-by: Robert Bublik <mail@robertbublik.de>
bobertrublik
force-pushed
the
separate-deployment-labels
branch
from
344cc11
to
f8bb2b6
Compare
What this PR does / why we need it:
We want to introduce new labels across our infrastructure and validate them using the
K8sRequiredLabelspolicy. Depending on its workload each pod will receive a differentworkloadlabel which is then used to enrich metrics collected from it. The Gatekeeper Helm chart doesn't allow setting separate labels for the controller manager pod and audit pod which will break our idea.Special notes for your reviewer:
I was following the PR below to apply the changes here.
I had to remove
{{- include "gatekeeper.podLabels" . | nindent 8 }}across all jobs since it's removed from the helpers.tpl file.