-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 labels field to kustomization #3743
Conversation
/retest |
if l := labelFromCommonLabels(k.CommonLabels); l != nil { | ||
k.Labels = append(k.Labels, *l) | ||
k.CommonLabels = nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider returning an error if labels collide.
Also, should probably add a note as to why this isn't done in PostUnmarshalling (because the edit
commands only work with commonLabels
, not labels`).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
thanks!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: monopole, Shell32-Natsu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This seems to be buggy with Kustomize 4.1.2, or I do not understand this correctly. It seems like it makes no difference weather I specify This is part of my kustomization:
My service.yaml, which doesn't specify any selectors at all...:
... still receives both labels as selectors after building with Kustomize:
|
@ChristianCiach Please file an issue for it or a PR to add a test case. |
I am hesitant to file an issue, because I am not sure if...
If you confirm that this does indeed look like an issue (and it is supposed to work in 4.1.2), then I will file an issue. |
This looks like a bug, because with a minimal example it works like expected. Give me some time to check out what part of my kustomization makes this issue appear, then I will file a bug :) |
Yes it looks like a bug. A good approach (and it's what we recommend) is creating a PR and add a test case for this situation in |
Thanks for the pointer. Unfortunately, creating a PR is not so easy for me, because I am a professional Java programmer with almost no experience in Go :) Please accept my issue posted as YAML examples. I am working on it. Thanks again for your quick reply! |
So, turns out this is (probably) not an issue, just a little pitfall. In my base-kustomization I have this:
In my overlay I have this:
The result is as shown above:
So, This is probably working as expected, so I will not file an issue. |
close #1009
ALLOW_MODULE_SPAN