-
Notifications
You must be signed in to change notification settings - Fork 489
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 support to set additional pod labels #3733
base: main
Are you sure you want to change the base?
Conversation
@@ -15,14 +15,24 @@ import ( | |||
// Deployment builds the deployment for the given instance. | |||
func Deployment(params Params) (*appsv1.Deployment, error) { | |||
name := naming.TargetAllocator(params.TargetAllocator.Name) | |||
labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) | |||
labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, manifestutils.WithFilterLabels(params.Config.LabelsFilter())) |
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 believe we need to support label filter here.
ef474b3
to
50b525a
Compare
I'm not sure if this PR just covers #2884 or more things. My feeling is that we should filter some of the values in the created resources, not adding a new field (that can be interesting but I don't feel is what the issue is about). |
@iblancasa Thank you for your feedback! Now that I’ve reread the issue, I realize I completely misunderstood it—this PR actually does propagate annotations and labels to child resources 😂 Let me make sure I understand correctly. Are you suggesting that instead of adding new fields to the CRD or flags to the controllers, we should create a list of annotations and labels that should not be propagated to child resources? |
Yes. |
Let me move our discussion to the original issue 🙏 |
Description:
I've added support to set additional pod labels like pod annotations. Thank you for your review!
Link to tracking Issue(s):
Testing:
Documentation: