Skip to content
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

Add podLabels variable for all daemonsets. #690

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sidewinder12s
Copy link

This will let you add pod labels to the daemonset pods without messing with the selector labels.

Closes #280

This will let you add pod labels to the daemonset pods without messing with the selector Labels.

Signed-off-by: Geoff Webster <gwebster@aurora.tech>
@ArangoGutierrez
Copy link
Collaborator

Thanks a lot @sidewinder12s !

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thoughts @elezar ?

@sidewinder12s
Copy link
Author

Any update @ArangoGutierrez?

@elezar elezar requested a review from klueska May 29, 2024 09:18
@elezar elezar self-assigned this May 29, 2024
@@ -38,6 +38,7 @@ spec:
metadata:
labels:
{{- include "nvidia-device-plugin.templateLabels" . | nindent 8 }}
{{- toYaml .Values.podLabels | nindent 8 }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general question. What is the behaviour if these conflict with any labels generated from templateLabels?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should merge and keep the last stated label I believe.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if someone passed in a label the templateLabels variable sets, they will override it.

@sidewinder12s
Copy link
Author

Anything else required?

@sidewinder12s
Copy link
Author

@elezar Can this be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add podLabels/customizable labels to Helm chart
3 participants