Skip to content

Conversation

@wadhah101
Copy link

@wadhah101 wadhah101 commented Dec 5, 2024

In certain cases you want to override the pod labels.
example : adding azure.workload.identity/use=true in https://learn.microsoft.com/en-us/azure/aks/workload-identity-overview?tabs=dotnet#pod-labels

this pr adds the option to specify labels

@CLAassistant
Copy link

CLAassistant commented Dec 5, 2024

CLA assistant check
All committers have signed the CLA.

@N-o-Z
Copy link
Member

N-o-Z commented Dec 18, 2024

@wadhah101 thank you for opening this PR!
Can you please add a short description in the PR explaining the change and the reason for the request.

@wadhah101
Copy link
Author

@wadhah101 added description

Copy link
Contributor

@Isan-Rivkin Isan-Rivkin left a comment

Choose a reason for hiding this comment

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

Thanks for drafting this. Please bump Chart.yaml

@wadhah101
Copy link
Author

@Isan-Rivkin Bumped to 1.3.26

Copy link
Contributor

@Isan-Rivkin Isan-Rivkin left a comment

Choose a reason for hiding this comment

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

First, thanks for drafting this PR!

The issue with this change is that K8S does not allow duplicate keys in labels.
Add a duplicate key such as app to podLabels yourself you'll see.
For me it breaks when I add podLabel app=duplicate I run helm install with the error

The Deployment "lakefs" is invalid: spec.template.metadata.labels: Invalid value: map[string]string{"app":"duplicate", "app.kubernetes.io/instance":"lakefs", "app.kubernetes.io/name":"lakefs"}: `selector` does not match template `labels`

kubectl version (on EKS):

Client Version: v1.29.0
Server Version: v1.30.7

So first, can you please better explain your use-case, is it specifically pod labels or Deployment labels? is it specifically override specific label keys? Should this affect the labelSelector?
What is it you would like to achieve.

Second incase of override label keys, I'd go with an override all-or-nothing approach not appending to existing labels.

@N-o-Z
Copy link
Member

N-o-Z commented Feb 13, 2025

Hi @wadhah101, are you still interested in contributing these changes?
If so, please fix or respond to @Isan-Rivkin's comments.
Thanks

@itaiad200
Copy link
Contributor

No activity

@itaiad200 itaiad200 closed this May 6, 2025
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.

5 participants