-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix k8s pod labels tier in metadata #16480
Conversation
Signed-off-by: chrismark <chrismarkou92@gmail.com>
@@ -49,6 +49,8 @@ func (p *pod) Generate(obj kubernetes.Resource, opts ...FieldOptions) common.Map | |||
} | |||
|
|||
out := p.resource.Generate("pod", obj, opts...) | |||
// TODO: remove this call when moving to 8.0 |
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.
I thinks it's fair to create an issue for removing this TODO ;)
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.
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.
+1, that said. I'm not quite convinced this is the way to go. Let's move this fix forward and discuss future steps in the opened issue
@@ -89,3 +91,14 @@ func (p *pod) GenerateFromName(name string, opts ...FieldOptions) common.MapStr | |||
|
|||
return nil | |||
} | |||
|
|||
func (p *pod) fixLabels(in common.MapStr) common.MapStr { |
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.
Hmm.. "fix" says nothing about the behavior. Could you rename this method to sth less vague? movePodLabels
, extractPodLabel
, etc.
Signed-off-by: chrismark <chrismarkou92@gmail.com>
ea32ee1
to
1f4d866
Compare
LGTM, Let's move this one forward, it seems some tests are failing, we need to update some tests cases |
Test cases were fixed. Also added a changelog. |
(cherry picked from commit 837279a)
(cherry picked from commit 837279a)
Closes #16464
Description of bug investigation: #16464 (comment)
cc: @exekias