Helm operator: in some rare cases, helm operator is incorrectly determining a release is not installed

[mikeshng]

What did you do?

I am looking at the helm operator manager sync code where it determines if a release is installed or not: https://github.com/operator-framework/operator-sdk/blob/v1.2.0/internal/helm/release/manager.go#L113-#L122
Its relying on an installed helm release to always have a deployed storage available.
But looking at the helm code it seems like at least in one case that might not be always true.
During the helm upgrade, it seems to put the release in another state before creating the new deployed state which creates a very small timing gap that helm operator might incorrectly determine the release is not installed:
https://github.com/helm/helm/blob/v3.4.1/pkg/action/upgrade.go#L345-#L346
https://github.com/helm/helm/blob/v3.4.1/pkg/action/upgrade.go#L137

The consequences seems to be when the helm operator thinks the release is not installed, it tries to perform an install which might fail and perform a uninstall as rollback, causing potential data lost even if the install is recovered afterwards in the next reconcile:

https://github.com/operator-framework/operator-sdk/blob/v1.2.0/internal/helm/controller/reconcile.go#L174-#L179
https://github.com/operator-framework/operator-sdk/blob/v1.2.0/internal/helm/release/manager.go#L170-#L175

What did you expect to see?

That when the helm release is installed, helm operator correctly determines it is installed or double check before performing the install->uninstall rollback

What did you see instead?

In some extremely rare cases, helm operator incorrectly determines an already installed release is not installed.

Environment

Operator type:
/language helm

Kubernetes cluster type:
Kind

$ operator-sdk version
Master branch

Possible Solution

Can we double check against the CR status and see if the deployedRelease is populated to verify that indeed the release is not installed?

[mikeshng]

A more likely case that helm operator might run into this problem is if a helm upgrade failed in a bad spot such as:

• because the updating of helm release status of new and old records are not done in transactional/atomic way
• the pre upgrade release is already set to superseded status
• the post upgraded release failed to set to deployed status then upgrade will return with err
• helm operator triggers the rollback and it failed as well
• now all the release records in the storage are marked as something other than “deployed”
• then helm operator will remove all the records in the sync function:
  https://github.com/operator-framework/operator-sdk/blob/v1.2.0/internal/helm/release/manager.go#L105-L106

now the helm operator will perform the install path since there are no more release left to indicate that the release is deployed.

In this case, if the install fails, is it still valid that helm operator should perform an uninstall as the recovery procedure? If one of the resource deleted is a crd then all the instances associated with the crd are deleted as well. Please advice. Fyi @camilamacedo86

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[openshift-bot]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

[openshift-bot]

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

[openshift-ci]

@openshift-bot: Closing this issue.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.