feat(helm): All recommended Kubernetes labels #1538
Conversation
nlamirault
requested review from
NissesSenap,
weisdd,
ishanjainn,
theSuess and
pb82
as code owners
There was a problem hiding this comment.
Think it's a good idea to move Values.additionalLabels to _helpers.tpl.
But there is no need to separately define app.kubernetes.io/component for each object, see my comment below.
I'm also happy that you don't update the selectorLabels, since it was a pain to update last time.
There is no need to bump the helm chart, we do that in CI.
| app.kubernetes.io/part-of: grafana-operator | ||
| {{- with .Values.additionalLabels }} | ||
| {{ toYaml . }} | ||
| {{- end }} |
There was a problem hiding this comment.
This I think is a good idea
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| {{- include "grafana-operator.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: config |
There was a problem hiding this comment.
I don't agree with your interpretation of how app.kubernetes.io/component should be used.
The component is related to the whole object, in this case grafana-operator. It doesn't differ between object like rbac or network.
There was a problem hiding this comment.
@NissesSenap so what do you like for this label ? just grafana-operator or operator ?
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
feat(helm): All recommended Kubernetes labels
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
| @@ -21,9 +19,7 @@ spec: | |||
| {{- end }} | |||
| labels: | |||
| {{- include "grafana-operator.selectorLabels" . | nindent 8 }} | |||
There was a problem hiding this comment.
grafana-operator.selectorLabels don't include .Values.additionalLabels, so removing the with block is a breaking change; since this change landed, it's not possible to set additional pod labels.
I believe this can be easily fixed by including grafana-operator.labels instead:
| {{- include "grafana-operator.selectorLabels" . | nindent 8 }} | |
| {{- include "grafana-operator.labels" . | nindent 8 }} |
Add missing labels and refactoring additional labels.
I don't change Helm chart version, because it is
v0.1.4but in documentation, it isv5.9.0: