[Bug/unintended behavior] Upgrade is run after a failed install who's uninstall history is kept

[fherenius]

Hi all,

Given a HelmRelease with the following install, uninstall, upgrade behavior:

apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
  name: chart
  namespace: chart
spec:
  interval: 5m0s
  chart:
    spec:
      chart: chart
      sourceRef:
        name: chart-repo
        kind: HelmRepository
        namespace: chart
  install:
    remediation:
      remediateLastFailure: true
    replace: true
  upgrade:
    cleanupOnFail: true
    remediation:
      strategy: "rollback"
      remediateLastFailure: true

Expected behavior

When the install of the chart fails for whatever reason, a secret with the name sh.helm.release.v1.chart.v1 is created with labels.status: uninstalled.

When an event causes the HelmRelease to be reconciled again (new chart version, new images via ImageUpdateAutomation) the helm-controller will try to install the chart again.

It'll keep doing this on events until the chart is succesfully installed, after which it will upgrade instead.

Actual behavior

When the install of the chart fails for whatever reason, a secret with the name sh.helm.release.v1.chart.v1 is created with labels.status: uninstalled.

When the HelmRelease is reconciled again, the helm-controller will run upgrade causing the Helm error Upgrade Failed: chart has no deployed releases.

This means the HelmRelease is now stuck and can only be fixed by manually deleting the secret sh.helm.release.v1.chart.v1.

Possible solution

I think the issue comes from the following code;

First, the last observed release is retrieved using helm history. In the above example, this will be the v1 with labels.status: uninstalled.

helm-controller/internal/controller/helmrelease_controller.go

Line 303 in 472244b

rel, observeLastReleaseErr := run.ObserveLastRelease(hr)

To decide on an install of upgrade, we only check if an last observed release exists. It's not checked if the last release is uninstalled for instance. Since, in the case above, the HelmRelease has a release, the helm-controller decides to run upgrade.

helm-controller/internal/controller/helmrelease_controller.go

Lines 396 to 408 in 472244b

	if rel == nil {
	r.event(ctx, hr, revision, eventv1.EventSeverityInfo, "Helm install has started")
	deployAction = hr.Spec.GetInstall()
	rel, err = run.Install(ctx, hr, chart, values)
	err = r.handleHelmActionResult(ctx, &hr, revision, err, deployAction.GetDescription(),
	v2.ReleasedCondition, v2.InstallSucceededReason, v2.InstallFailedReason)
	} else {
	r.event(ctx, hr, revision, eventv1.EventSeverityInfo, "Helm upgrade has started")
	deployAction = hr.Spec.GetUpgrade()
	rel, err = run.Upgrade(ctx, hr, chart, values)
	err = r.handleHelmActionResult(ctx, &hr, revision, err, deployAction.GetDescription(),
	v2.ReleasedCondition, v2.UpgradeSucceededReason, v2.UpgradeFailedReason)
	}

I think this can either be solved by adding the --install flag by default to the upgrade command or checking if the latest observed released has the uninstalled status.