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

Helm Operator for A/B deployment #4949

Closed
jfgosselin opened this issue May 26, 2021 · 13 comments
Closed

Helm Operator for A/B deployment #4949

jfgosselin opened this issue May 26, 2021 · 13 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. language/helm Issue is related to a Helm operator project needs discussion
Milestone

Comments

@jfgosselin
Copy link

Feature Request

Describe the problem you need a feature to resolve.

I’m using the Helm Operator and I want to use it for an A/B deployment (or blue green/deployment) in the same namespace (it must be in the same namespace).

Obviously I can’t deploy two Helm Operators with the same watches.yaml but different Helm chart versions (they will step on each other).

I also don’t want to bump the CRD version for each new Helm chart release.

Describe the solution you'd like.

Each Helm Operator could watch the CR with a specific label defined in their watches.yaml e.g: release: A, release B.

Sorry, I think this feature was requested before but I can’t find the answer.

Thanks

JF

@estroz estroz added language/helm Issue is related to a Helm operator project needs discussion labels Jun 7, 2021
@estroz estroz added this to the Backlog milestone Jun 7, 2021
@estroz estroz added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 7, 2021
@estroz
Copy link
Member

estroz commented Jun 7, 2021

/cc @fabianvf

@joelanford
Copy link
Member

Just putting this out there as something to consider. Do we want to expose this functionality in the watches.yaml, which would apply to all helm-based operators? Or do we think this more of a niche feature that would make more sense to be implemented as part of a hybrid helm/go operator?

I don't personally have a strong opinion, but one thing that comes to mind is the possibility of this being a footgun. I can foresee lots of issues, especially from users of an operator in the ilk of "why isn't my CR being reconciled?" and one of the first troubleshooting steps will be to look at the watches.yaml and the CR labels. The watches.yaml file is not something users will typically see, so any operator that makes use of this feature will need very clear guidance about how to create CRs.

@jfgosselin
Copy link
Author

What about via an environment variable ? Similar to what you have for the namespace WATCH_NAMESPACE.

@VenkatRamaraju
Copy link
Contributor

@joelanford wrt my PR (here), If no selector is specified in watches.yaml, the functionality would be the same as it is currently. A CR will not be reconciled if a selector is specified and the labels of the CR don't match what's specified in the selector (in watches.yaml).

So if users don't specify any selector, which is the default, then everything remains the same. Do you still foresee a lot of issues? Correct me if I'm wrong anywhere.

@estroz
Copy link
Member

estroz commented Jun 22, 2021

@VenkatRamaraju I think what @joelanford is saying is that watches.yaml is not publicly exposed to the user persona who creates CRs but does not administer the operator itself, since that file is only read at startup time. There would be no easy way to troubleshoot issues stemming from a lack of some label required by the operator. Hopefully I captured this potential issue accurately. I guess the solution is to define this value somewhere surfaced by the operator to users, but outside of a CSV I don't have a good idea of where that is. An environment variable has the same problem as watches.yaml unless a user can kubectl describe the operator's deployment.

@estroz estroz modified the milestones: v1.10.0, Backlog Jun 22, 2021
@estroz
Copy link
Member

estroz commented Jun 22, 2021

I moved this to the backlog since it has needs-discussion.

@VenkatRamaraju
Copy link
Contributor

@estroz Thanks for clearing that up.

@fabianvf
Copy link
Member

@estroz @joelanford I will say this currently is implemented in the Ansible's watches.yaml and we've not seen issues with it, I think it's primarily being used by teams that are both developing and deploying/managing the operator. I guess there is the potential for footgunning, but I also feel like it's a not uncommon pattern to scope reconciliation by label (I'm thinking specifically of ingress controllers here), and to rely on documentation of the controller to know what labels will lead to reconciliation.

@estroz
Copy link
Member

estroz commented Jul 2, 2021

If this is already something ansible supports, then helm-operator can similarly adopt it imo. There seems to be extra parsing logic implemented for ansible's watches.yaml; @fabianvf is there a reason metav1.LabelSelector can't be used directly?

@jfgosselin
Copy link
Author

First thanks a lot for looking into this feature request, it will simplify my deployments.

Is there an early release I could try or the implementation is still under discussion.

Thanks !

@VenkatRamaraju
Copy link
Contributor

VenkatRamaraju commented Aug 2, 2021

Hi @jfgosselin,

The feature has now been approved and merged onto master (#4997). It will be included in the next release of operator-sdk (1.11).

It can also be used currently with the helm-operator binary built from the master, or the master tag for the helm-operator base image.

@VenkatRamaraju
Copy link
Contributor

VenkatRamaraju commented Aug 2, 2021

@estroz Created a new PR for ansible that removes all the extra parsing you referenced in your comment. It mirrors #4997.
Link to PR: #5086

@jmrodri
Copy link
Member

jmrodri commented Sep 3, 2021

Fixed by PR #5086

@jmrodri jmrodri closed this as completed Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. language/helm Issue is related to a Helm operator project needs discussion
Projects
None yet
Development

No branches or pull requests

7 participants