How should the Helm operator handle existing Helm installations?

[SimonBaeumer]

If a Helm operator is installed into a namespace with an already existing Helm release how should the operator handle this situation.

A) automatically reconcile and try to match the CR's state
B) failing because the operator tries to reconcile a resource which is not owned by the operator (in this case the Helm release secret)

Suggestion

• By default fail on releases which were not owned by the operator, thus preventing users accidentally messing up current installations
• Add an option to opt-in to let the operator reconcile an existing Helm installation.

Opt-in options

1. Label on the latest Helm release secret operators.operatorframework.io.helm-operator-plugins.adopt-release: true

   • Explicitly mark a release to be adopted by the operator. This only has an effect if the secret is not operator owned.
     o Manually created Helm releases disable the reconciler for that resource (positive or negative depending on the use-case)
   • Looking up the latest Helm secret in the reconciliation

2. CR field on the helm-operator CR (i.e. adoptExistingRelease)

   • Explicitly and obvious option on the CR
   • No need to lookup the Helm release ownership nor labels
   • A CR field for a special case which imho does not reflect cluster state
   • Needs to be manually implemented in a hybrid setting

3. Annotation on the helm-operator CR

• No need to lookup the Helm release ownership nor labels
  - Shares the same negatives as the CR field but even more a special case on the CR

Considering
A) adopting an existing Helm release is a task done once at setup
B) running Helm upgrades manually in operator managed environments is bad practice

I prefer option 1. With that users of the library don't need to implemented the feature by themselves and can offer smooth transitions to operator managed charts.

See discussion: #116 (comment)
cc: @joelanford

[joelanford]

My first thought was actually option 3.

A) adopting an existing Helm release is a task done once at setup

Generally yeah. But it's theoretically possible for someone to run helm uninstall && helm install really quickly between operator reconciliations (maybe the operator lost its connection to the apiserver temporarily), so I could see this annotation being more in the vane of "Always ensure the release secret is owned by me", which implies that the operator should adopt the release secret if it is not already owned by it.

I'm also curious what the current behavior is (I should know, I wrote it. Unfortunately, this was not really considered or tested, so I'm not sure). Since helm-operator is 1.0.0, it seems like the choice of opt-in vs opt-out is already made for us because we can't change the default functionality.

Needs to be manually implemented in a hybrid setting

Why is this true? Could we not either hardcode this into the reconciler (perhaps with a configurable name) or make it something that gets enabled via a reconciler option?

[varshaprasad96]

Based on the discussion during the community meeting, Joe mentioned these 2 points:

1. If we decide on Option3, it should be configurable. Annotations can have custom domain name prefixes, like memcached.xx.operatorframework.io.
2. Option 2 maybe not be desirable, since in previous implementations - the spec field of CR and the fields in values.yaml had 1 to 1 mapping, but it maynot be the exact same case now since hybrid libraries provide an option to modify that.

[SimonBaeumer]

Why is this true? Could we not either hardcode this into the reconciler (perhaps with a configurable name) or make it something that gets enabled via a reconciler option?

For a CR use-case this is true because the CR needs to be implemented by us manually. To be honest, thinking twice it does not make sense than to be implemented in the helm-operator-plugins itself.

Generally yeah. But it's theoretically possible for someone to run helm uninstall && helm install really quickly between operator reconciliations (maybe the operator lost its connection to the apiserver temporarily), so I could see this annotation being more in the vane of "Always ensure the release secret is owned by me", which implies that the operator should adopt the release secret if it is not already owned by it.

Yes, but we can only be optimistic in catching user-errors. If a user asks the operator to take over releases the user is expected to not touch it anymore or need to stop the operator.

I'm also curious what the current behavior is (I should know, I wrote it. Unfortunately, this was not really considered or tested, so I'm not sure). Since helm-operator is 1.0.0, it seems like the choice of opt-in vs opt-out is already made for us because we can't change the default functionality.

I don't think so, the behaviours can be different for Helm-only reconciles and Hybrid approaches.
The Helm operator scaffolding could get a --legacy flag which ensures compatibility to v1, otherwise the new implementation is chosen.
For hybrid operators it should be up to the developers, only providing reasonable defaults.

If we decide on Option3, it should be configurable. Annotations can have custom domain name prefixes, like memcached.xx.operatorframework.io.

@varshaprasad96 Do you mean the annotation used is configurable?
If yes, I am also fine with giving a good default, I don't see much value in having this configurable (except hiding that helm-operator is used under the hood).

Option 2 maybe not be desirable, since in previous implementations - the spec field of CR and the fields in values.yaml had 1 to 1 mapping, but it maynot be the exact same case now since hybrid libraries provide an option to modify that.

Yes, I agree. 👍