Skip to content
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 Label Whitelisting functionality #195

Merged
merged 10 commits into from
Dec 4, 2019
Merged

Add Label Whitelisting functionality #195

merged 10 commits into from
Dec 4, 2019

Conversation

adamhf
Copy link
Contributor

@adamhf adamhf commented Oct 25, 2019

Fixes #179

Changes proposed in the PR:
The PR adds the ability to filter which labels on the CRD/Spec should be propagated to Kubernetes resources created by the operator via a label whitelist that specifies regexs.

By default (with no whitelist) specified the existing functionality is maintained and all labels are copied, however, once a whitelist regex is specified only labels matching it will be propagated.

This can be used to explicitly whitelist all labels via the pattern .* blacklist all labels by specifying an unmatchable pattern, or somewhere in between if you only want some propagated.

This solves our issue of our gitops operator (Flux) modifying labels on the Spec and the operator attempting to update the PDB, which are immutable in k8's < 1.15 and failing as a result.

@adamhf adamhf requested review from ese, jchanam and slok as code owners October 25, 2019 09:04
@chusAlvarez
Copy link
Contributor

Hi, lot of thanks for the contribution :). But, as the problem is the disruption bucket inmutability, Will not be delete and re-create it a more useful approach?

This way the users could change the labels and be sure they are propagated in kubernetes, as intended :)

@chusAlvarez chusAlvarez requested review from chusAlvarez and removed request for slok November 11, 2019 11:21
@primeroz
Copy link

Given that this is a backward compatible change that also require a direct action to be enabled ( set the whitelisting to "something") is it such a big issue though ?

In our specific case , for example, we just don't want all this labels to propagate down since they kind of mess up a bunch of UI tools we built that never expected to have all this labels attached.

Also , still i guess in our specific case, having the "flux-cd GC" label applied to the child resources makes flux think that it owns those resources when in reality it does not ( it only owns the CRD and it should only manage that )

Also, to be honest, i don't like the idea of having any time where the PDB are not set in the cluster for a service like redis cluster , even if during normal operation this time should be very short.
Unfortunately this operation would not be atomic opening up to some edge case failures where the PDB is not set when is needed.

I personally prefer the whitelisting option so that not only we can deal with the current issue ( until 1.15 i believe ? ) but also can deal with labels themself.

@ese
Copy link
Member

ese commented Nov 21, 2019

Hi @adamhf, it's ok for me. Can you resolve the conflicts?

@adamhf
Copy link
Contributor Author

adamhf commented Dec 3, 2019

Done, apologies for the delay.

@ese
Copy link
Member

ese commented Dec 4, 2019

Thank you so much for your contribution @adamhf

@ese ese merged commit d728950 into spotahome:master Dec 4, 2019
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.

Statefulset's labels don't change when RedisFailover's labels changed
4 participants