Recover from failures not related to install/upgrade

Previously, we re-attempted release reconciliation on existing
releases when upgradeFailures > 0. This misses other types
of failures, such as k8s API call failures. Also currently
upgradeFailures > 0 doesn't mean there wasn't a subsequent success
after those failures, as the failure counts don't get reset until
a new desired state is seen, since this allows users to be aware
that those failures occurred.

This moves to checking for a non-ready condition to determine
if any error was seen on the last reconciliation attempt, and if so
performing the release reconciliation.

api/v2alpha1/helmrelease_types.go

@@ -625,6 +625,20 @@ func HelmReleaseProgressing(hr HelmRelease) HelmRelease {
 	return hr
 }
 
+// GetHelmReleaseConditionStatus returns the status of the given condition with the given type of the given HelmRelease.
+func GetHelmReleaseConditionStatus(hr HelmRelease, conditionType string) corev1.ConditionStatus {
+	var condition *Condition
+	for _, c := range hr.Status.Conditions {
+		if c.Type == conditionType {
+			condition = &c
+		}
+	}
+	if condition != nil {
+		return condition.Status
+	}
+	return corev1.ConditionUnknown
+}
+
 // SetHelmReleaseCondition sets the given condition with the given status, reason and message
 // on the HelmRelease.
 func SetHelmReleaseCondition(hr *HelmRelease, condition string, status corev1.ConditionStatus, reason, message string) {

controllers/helmrelease_controller.go

@@ -211,7 +211,7 @@ func (r *HelmReleaseReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 		return ctrl.Result{}, nil
 	}
 
-	reconciledHr, reconcileErr := r.release(log, *hr.DeepCopy(), hc, values, hasNewGeneration)
+	reconciledHr, reconcileErr := r.release(log, *hr.DeepCopy(), hc, values)
 	if reconcileErr != nil {
 		r.event(hr, hc.GetArtifact().Revision, recorder.EventSeverityError, fmt.Sprintf("reconciliation failed: %s", reconcileErr.Error()))
 	}
@@ -295,7 +295,7 @@ func (r *HelmReleaseReconciler) reconcileChart(ctx context.Context, hr *v2.HelmR
 	return &helmChart, true, nil
 }
 
-func (r *HelmReleaseReconciler) release(log logr.Logger, hr v2.HelmRelease, source sourcev1.Source, values chartutil.Values, hasNewGeneration bool) (v2.HelmRelease, error) {
+func (r *HelmReleaseReconciler) release(log logr.Logger, hr v2.HelmRelease, source sourcev1.Source, values chartutil.Values) (v2.HelmRelease, error) {
 	// Acquire lock
 	unlock, err := lock(fmt.Sprintf("%s-%s", hr.GetName(), hr.GetNamespace()))
 	if err != nil {
@@ -350,10 +350,10 @@ func (r *HelmReleaseReconciler) release(log logr.Logger, hr v2.HelmRelease, sour
 	// Install if there is none.
 	case rel == nil:
 		deployAction = hr.Spec.GetInstall()
-	// Upgrade if there is a new generation, new state, or this is an upgrade retry.
-	case hasNewGeneration || hasNewState || hr.Spec.GetUpgrade().GetRemediation().GetFailureCount(hr) > 0:
+	// Upgrade if the release is not in a ready state from a previous attempt.
+	case v2.GetHelmReleaseConditionStatus(hr, v2.ReadyCondition) != corev1.ConditionTrue:
 		deployAction = hr.Spec.GetUpgrade()
-	// Otherwise no action needed.
+	// Otherwise, no action needed.
 	default:
 		return hr, nil
 	}