Copy labels from canary to primary workloads based on prefix rules #709
Conversation
pkg/canary/deployment_controller.go
Outdated
| Labels: map[string]string{ | ||
| label: primaryLabelValue, | ||
| }, | ||
| Labels: labels, |
There was a problem hiding this comment.
This filtering only applies to the deployment labels. Should it apply to the spec.template.metadata.labels as well?
From what I understood this is only to solve a very specific use case so I'm not sure if it should apply everywhere.
| }) | ||
| } | ||
|
|
||
| func TestMakePrimaryLabels(t *testing.T) { |
There was a problem hiding this comment.
Unrelated but I noticed this file had not unit tests so I added this one as well.
charts/flagger/README.md
Outdated
| @@ -125,6 +125,7 @@ Parameter | Description | Default | |||
| `serviceAccount.name` | The name of the service account to create or use. If not set and `serviceAccount.create` is `true`, a name is generated using the Flagger fullname | `""` | |||
| `serviceAccount.annotations` | Annotations for service account | `{}` | |||
| `ingressAnnotationsPrefix` | Annotations prefix for ingresses | `custom.ingress.kubernetes.io` | |||
| `excludedLabelsPrefixes` | List of prefixes of labels that are excluded when creating primary controllers | `"fluxcd,jenkins"` | |||
There was a problem hiding this comment.
should the name be changed to reflect that this applies only to deployment/daemonset labesls?
|
@worldtiki I've been searching around and seems that there are many controllers that are using labels for garbage collection and I don't see how we can maintain an exclusion list. Can you please change it to Thanks and sorry for this change, I should've spot it earlier. |
Of course. Would it make sense to have something like a |
I think so as long it's not the default, the default should be include none e.g. string empty. |
stefanprodan
changed the title
|
Any update on merging this commit? |
|
Did we test this well? It's pretty broken. Nothing is working |
|
Can you expand on that? What's not working? |
|
Configured with:
Annotations and Labels of canary deployment: annotations: labels: What actually got copied over to primary's deployment. No annotations |
|
@DerrickMartinez these changes were not included in the v1.2.0 release. Flagger shouldn't even be able to start with that option (include-label-prefix) since it doesn't exist in that version. |
Copying of Configmaps and Secrets managed through Flagger should now follow the same label prefix filtering rules as for the workloads. Extends: fluxcd#709 Signed-off-by: Aurel Canciu <aurelcanciu@gmail.com>
Copy labels from canary to primary deployment and daemonset but exclude labels by prefix.
Copy annotations too.
Fixes #329.