helm chart: Add extra labels and annotations to pods and k8s resources / Allow SSL mode to be defined. #655
Conversation
jescarri
force-pushed
the
add/custom-labels
branch
from
40f9e09
to
3c1bc77
Compare
jescarri
changed the title
|
Sorry for the review delay, we'll try to assign someone in tomorrow's planning call flatcar/Flatcar#1103 |
|
Ah! sweet let me bump the version. Thanks! |
Allow users to specify extra labels and annotations to pods and other k8s resources.
jescarri
force-pushed
the
add/custom-labels
branch
from
3c1bc77
to
d001572
Compare
|
@pothos bumped the patch version. Thanks for taking a look at this PR. |
|
At my company we would really like this change as well. |
|
The CI linter complains, can someone have a look? I used the GitHub UI to bump to version 1.1.1 but it created a merge commit, maybe squash that, too. |
| @@ -24,6 +32,7 @@ spec: | |||
| {{- end }} | |||
| labels: | |||
| {{- include "nebraska.selectorLabels" . | nindent 8 }} | |||
| {{ toYaml .Values.podLabels | nindent 8 }} | |||
There was a problem hiding this comment.
It looks like adding extra pod labels in this way does not work.
| {{ toYaml .Values.podLabels | nindent 8 }} | |
| {{- with .Values.podLabels }} | |
| {{ toYaml . | nindent 8 }} | |
| {{ - end }} |
should fix the linter issue.
| @@ -19,7 +19,7 @@ sources: | |||
| maintainers: | |||
| - name: kinvolk | |||
| url: https://kinvolk.io/ | |||
| version: 1.1.0 | |||
| version: 1.1.1 | |||
There was a problem hiding this comment.
Please bump at least the minor version as this PR adds additional features and not bugfixes.
There was a problem hiding this comment.
Thank you for the review, I'm not a Helm user myself and don't know how the versioning is used in practice… So rather semver-like, it seems.
| {{- with .Values.ingress.annotations }} | ||
| annotations: | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| {{- with.Values.extraAnnotations }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} |
There was a problem hiding this comment.
This block needs more attention.
If ingress.annotations is not set and you set extraAnnotations, the manifest is no longer valid as the annotations: line is not rendered then.
| {{- with .Values.extraLabels }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| annotations: |
There was a problem hiding this comment.
Almost same here, please surround the annotations: line with an if conditional or use a sprig merge function to merge both anntations slices together.
| {{- with .Values.extraLabels }} | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| annotations: |
| {{- with .Values.serviceAccount.annotations }} | ||
| annotations: | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| {{- with.Values.extraAnnotations }} |
| {{- with .Values.ingress.update.annotations }} | ||
| annotations: | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- end }} | ||
| {{- with.Values.extraAnnotations }} |
|
Cool I will add the fixes today. Thanks for the review. |
|
hi @jescarri, friendly ping here, Did you get to look through the suggestions mentioned in the review? |
Allow users to specify extra labels and annotations to pods and other k8s resources.
This change allows users to set extra labels and annotations to pods and k8s resources.
How to use
In the values file:
The above will label all k8s resources with foo: bar and annotate all k8s resources with foo:bar
In addition pods will be labelled with extraLabels + another: test and annotated with extraAnnotations + test: test
To test:
Deploy nebraska using the helm chart + add the above values to your values file.
Testing done
Nebraska has been deployed in our clusters using the modified helm chart, we are able to add extra labels and annotations.
changelog/directory (user-facing change, bug fix, security fix, update)/bootand/usrsize, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.