Unify Helm chart upgrades

api/v1alpha1/upgradeplan_types.go

@@ -27,6 +27,9 @@ const (
 	RancherUpgradedCondition    = "RancherUpgraded"
 	LonghornUpgradedCondition   = "LonghornUpgraded"
 
+	// UpgradeError indicates that the upgrade process has encountered a transient error.
+	UpgradeError = "Error"
+
 	// UpgradePending indicates that the upgrade process has not begun.
 	UpgradePending = "Pending"
 
@@ -39,7 +42,7 @@ const (
 	// UpgradeSucceeded indicates that the upgrade process has been successful.
 	UpgradeSucceeded = "Succeeded"
 
-	// UpgradeFailed indicates that an error occurred during the upgrade process.
+	// UpgradeFailed indicates that the upgrade process has failed.
 	UpgradeFailed = "Failed"
 )
 

internal/controller/helm.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"slices"
 
 	lifecyclev1alpha1 "github.com/suse-edge/upgrade-controller/api/v1alpha1"
 	"github.com/suse-edge/upgrade-controller/internal/upgrade"
@@ -16,9 +17,14 @@ import (
 	helmstorage "helm.sh/helm/v3/pkg/storage"
 	helmdriver "helm.sh/helm/v3/pkg/storage/driver"
 
+	batchv1 "k8s.io/api/batch/v1"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 func newHelmClient() (*helmstorage.Storage, error) {
@@ -46,12 +52,13 @@ func retrieveHelmRelease(name string) (*helmrelease.Release, error) {
 
 	helmReleases, err := helmClient.History(name)
 	if err != nil {
-		if errors.Is(err, helmdriver.ErrReleaseNotFound) {
-			return nil, nil
-		}
 		return nil, err
 	}
 
+	if len(helmReleases) == 0 {
+		return nil, helmdriver.ErrReleaseNotFound
+	}
+
 	helmutil.Reverse(helmReleases, helmutil.SortByRevision)
 	helmRelease := helmReleases[0]
 
@@ -116,3 +123,79 @@ func (r *UpgradePlanReconciler) createHelmChart(ctx context.Context, upgradePlan
 
 	return r.Create(ctx, chart)
 }
+
+func (r *UpgradePlanReconciler) upgradeHelmChart(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, releaseChart *release.HelmChart) (upgrade.HelmChartState, error) {
+	helmRelease, err := retrieveHelmRelease(releaseChart.Name)
+	if err != nil {
+		if errors.Is(err, helmdriver.ErrReleaseNotFound) {
+			return upgrade.ChartStateNotInstalled, nil
+		}
+		return upgrade.ChartStateUnknown, fmt.Errorf("retrieving helm release: %w", err)
+	}
+
+	chart := &helmcattlev1.HelmChart{}
+
+	if err = r.Get(ctx, upgrade.ChartNamespacedName(helmRelease.Name), chart); err != nil {
+		if !apierrors.IsNotFound(err) {
+			return upgrade.ChartStateUnknown, err
+		}
+
+		if helmRelease.Chart.Metadata.Version == releaseChart.Version {
+			return upgrade.ChartStateVersionAlreadyInstalled, nil
+		}
+
+		return upgrade.ChartStateInProgress, r.createHelmChart(ctx, upgradePlan, helmRelease, releaseChart)
+	}
+
+	if chart.Spec.Version != releaseChart.Version {
+		return upgrade.ChartStateInProgress, r.updateHelmChart(ctx, upgradePlan, chart, releaseChart)
+	}
+
+	releaseVersion := chart.Annotations[upgrade.ReleaseAnnotation]
+	if releaseVersion != upgradePlan.Spec.ReleaseVersion {
+		return upgrade.ChartStateVersionAlreadyInstalled, nil
+	}
+
+	job := &batchv1.Job{}
+	if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.ChartNamespace}, job); err != nil {
+		return upgrade.ChartStateUnknown, err
+	}
+
+	idx := slices.IndexFunc(job.Status.Conditions, func(condition batchv1.JobCondition) bool {
+		return condition.Status == corev1.ConditionTrue &&
+			(condition.Type == batchv1.JobComplete || condition.Type == batchv1.JobFailed)
+	})
+
+	if idx == -1 {
+		// Upgrade job is still ongoing.
+		return upgrade.ChartStateInProgress, nil
+	}
+
+	condition := job.Status.Conditions[idx]
+	if condition.Type == batchv1.JobComplete {
+		return upgrade.ChartStateSucceeded, nil
+	}
+
+	logger := log.FromContext(ctx)
+	logger.Info("Helm chart upgrade job failed",
+		"helmChart", releaseChart.Name,
+		"job", fmt.Sprintf("%s/%s", job.Namespace, job.Name),
+		"jobStatus", condition.Message)
+
+	return upgrade.ChartStateFailed, nil
+}
+
+func evaluateHelmChartState(state upgrade.HelmChartState) (setCondition setCondition, requeue bool) {
+	switch state {
+	case upgrade.ChartStateNotInstalled, upgrade.ChartStateVersionAlreadyInstalled:
+		return setSkippedCondition, true
+	case upgrade.ChartStateInProgress:
+		return setInProgressCondition, false
+	case upgrade.ChartStateSucceeded:
+		return setSuccessfulCondition, true
+	case upgrade.ChartStateFailed:
+		return setFailedCondition, true
+	default:
+		return setErrorCondition, false
+	}
+}

internal/controller/reconcile_longhorn.go

@@ -2,83 +2,20 @@ package controller
 
 import (
 	"context"
-	"fmt"
-	"slices"
 
-	helmcattlev1 "github.com/k3s-io/helm-controller/pkg/apis/helm.cattle.io/v1"
 	lifecyclev1alpha1 "github.com/suse-edge/upgrade-controller/api/v1alpha1"
-	"github.com/suse-edge/upgrade-controller/internal/upgrade"
 	"github.com/suse-edge/upgrade-controller/pkg/release"
-
-	batchv1 "k8s.io/api/batch/v1"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func (r *UpgradePlanReconciler) reconcileLonghorn(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, longhorn *release.HelmChart) (ctrl.Result, error) {
-	helmRelease, err := retrieveHelmRelease(longhorn.Name)
+	state, err := r.upgradeHelmChart(ctx, upgradePlan, longhorn)
 	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("retrieving helm release: %w", err)
-	}
-
-	if helmRelease == nil {
-		setSkippedCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, "Longhorn installation is not found")
-		return ctrl.Result{Requeue: true}, nil
-	}
-
-	chart := &helmcattlev1.HelmChart{}
-
-	if err = r.Get(ctx, upgrade.ChartNamespacedName(helmRelease.Name), chart); err != nil {
-		if !errors.IsNotFound(err) {
-			return ctrl.Result{}, err
-		}
-
-		if helmRelease.Chart.Metadata.Version == longhorn.Version {
-			setSkippedCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, versionAlreadyInstalledMessage(upgradePlan))
-			return ctrl.Result{Requeue: true}, nil
-		}
-
-		setInProgressCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, "Longhorn is being upgraded")
-		return ctrl.Result{}, r.createHelmChart(ctx, upgradePlan, helmRelease, longhorn)
-	}
-
-	if chart.Spec.Version != longhorn.Version {
-		setInProgressCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, "Longhorn is being upgraded")
-		return ctrl.Result{}, r.updateHelmChart(ctx, upgradePlan, chart, longhorn)
+		return ctrl.Result{}, err
 	}
 
-	releaseVersion := chart.Annotations[upgrade.ReleaseAnnotation]
-	if releaseVersion != upgradePlan.Spec.ReleaseVersion {
-		setSkippedCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, versionAlreadyInstalledMessage(upgradePlan))
-		return ctrl.Result{Requeue: true}, nil
-	}
-
-	job := &batchv1.Job{}
-	if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.ChartNamespace}, job); err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
-
-	idx := slices.IndexFunc(job.Status.Conditions, func(condition batchv1.JobCondition) bool {
-		return condition.Status == corev1.ConditionTrue &&
-			(condition.Type == batchv1.JobComplete || condition.Type == batchv1.JobFailed)
-	})
-
-	if idx == -1 {
-		// Upgrade job is still ongoing.
-		return ctrl.Result{}, nil
-	}
-
-	condition := job.Status.Conditions[idx]
-
-	switch condition.Type {
-	case batchv1.JobComplete:
-		setSuccessfulCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, "Longhorn is upgraded")
-	case batchv1.JobFailed:
-		setFailedCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, fmt.Sprintf("Error occurred: %s", condition.Message))
-	}
+	setCondition, requeue := evaluateHelmChartState(state)
+	setCondition(upgradePlan, lifecyclev1alpha1.LonghornUpgradedCondition, state.Message())
 
-	return ctrl.Result{Requeue: true}, nil
+	return ctrl.Result{Requeue: requeue}, nil
 }

internal/controller/reconcile_rancher.go

@@ -2,83 +2,20 @@ package controller
 
 import (
 	"context"
-	"fmt"
-	"slices"
 
-	helmcattlev1 "github.com/k3s-io/helm-controller/pkg/apis/helm.cattle.io/v1"
 	lifecyclev1alpha1 "github.com/suse-edge/upgrade-controller/api/v1alpha1"
-	"github.com/suse-edge/upgrade-controller/internal/upgrade"
 	"github.com/suse-edge/upgrade-controller/pkg/release"
-
-	batchv1 "k8s.io/api/batch/v1"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func (r *UpgradePlanReconciler) reconcileRancher(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, rancher *release.HelmChart) (ctrl.Result, error) {
-	helmRelease, err := retrieveHelmRelease(rancher.Name)
+	state, err := r.upgradeHelmChart(ctx, upgradePlan, rancher)
 	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("retrieving helm release: %w", err)
-	}
-
-	if helmRelease == nil {
-		setSkippedCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, "Rancher installation is not found")
-		return ctrl.Result{Requeue: true}, nil
-	}
-
-	chart := &helmcattlev1.HelmChart{}
-
-	if err = r.Get(ctx, upgrade.ChartNamespacedName(helmRelease.Name), chart); err != nil {
-		if !errors.IsNotFound(err) {
-			return ctrl.Result{}, err
-		}
-
-		if helmRelease.Chart.Metadata.Version == rancher.Version {
-			setSkippedCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, versionAlreadyInstalledMessage(upgradePlan))
-			return ctrl.Result{Requeue: true}, nil
-		}
-
-		setInProgressCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, "Rancher is being upgraded")
-		return ctrl.Result{}, r.createHelmChart(ctx, upgradePlan, helmRelease, rancher)
-	}
-
-	if chart.Spec.Version != rancher.Version {
-		setInProgressCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, "Rancher is being upgraded")
-		return ctrl.Result{}, r.updateHelmChart(ctx, upgradePlan, chart, rancher)
+		return ctrl.Result{}, err
 	}
 
-	releaseVersion := chart.Annotations[upgrade.ReleaseAnnotation]
-	if releaseVersion != upgradePlan.Spec.ReleaseVersion {
-		setSkippedCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, versionAlreadyInstalledMessage(upgradePlan))
-		return ctrl.Result{Requeue: true}, nil
-	}
-
-	job := &batchv1.Job{}
-	if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.ChartNamespace}, job); err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
-
-	idx := slices.IndexFunc(job.Status.Conditions, func(condition batchv1.JobCondition) bool {
-		return condition.Status == corev1.ConditionTrue &&
-			(condition.Type == batchv1.JobComplete || condition.Type == batchv1.JobFailed)
-	})
-
-	if idx == -1 {
-		// Upgrade job is still ongoing.
-		return ctrl.Result{}, nil
-	}
-
-	condition := job.Status.Conditions[idx]
-
-	switch condition.Type {
-	case batchv1.JobComplete:
-		setSuccessfulCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, "Rancher is upgraded")
-	case batchv1.JobFailed:
-		setFailedCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, fmt.Sprintf("Error occurred: %s", condition.Message))
-	}
+	setCondition, requeue := evaluateHelmChartState(state)
+	setCondition(upgradePlan, lifecyclev1alpha1.RancherUpgradedCondition, state.Message())
 
-	return ctrl.Result{Requeue: true}, nil
+	return ctrl.Result{Requeue: requeue}, nil
 }

internal/controller/upgradeplan_controller.go

@@ -183,11 +183,18 @@ func parseDrainOptions(plan *lifecyclev1alpha1.UpgradePlan) (drainControlPlane b
 	return drainControlPlane, drainWorker
 }
 
+type setCondition func(plan *lifecyclev1alpha1.UpgradePlan, conditionType string, message string)
+
 func setPendingCondition(plan *lifecyclev1alpha1.UpgradePlan, conditionType, message string) {
 	condition := metav1.Condition{Type: conditionType, Status: metav1.ConditionUnknown, Reason: lifecyclev1alpha1.UpgradePending, Message: message}
 	meta.SetStatusCondition(&plan.Status.Conditions, condition)
 }
 
+func setErrorCondition(plan *lifecyclev1alpha1.UpgradePlan, conditionType, message string) {
+	condition := metav1.Condition{Type: conditionType, Status: metav1.ConditionUnknown, Reason: lifecyclev1alpha1.UpgradeError, Message: message}
+	meta.SetStatusCondition(&plan.Status.Conditions, condition)
+}
+
 func setInProgressCondition(plan *lifecyclev1alpha1.UpgradePlan, conditionType, message string) {
 	condition := metav1.Condition{Type: conditionType, Status: metav1.ConditionFalse, Reason: lifecyclev1alpha1.UpgradeInProgress, Message: message}
 	meta.SetStatusCondition(&plan.Status.Conditions, condition)
@@ -208,10 +215,6 @@ func setSkippedCondition(plan *lifecyclev1alpha1.UpgradePlan, conditionType, mes
 	meta.SetStatusCondition(&plan.Status.Conditions, condition)
 }
 
-func versionAlreadyInstalledMessage(plan *lifecyclev1alpha1.UpgradePlan) string {
-	return fmt.Sprintf("Component version is the same in release %s", plan.Spec.ReleaseVersion)
-}
-
 func (r *UpgradePlanReconciler) findUpgradePlanFromJob(ctx context.Context, job client.Object) []reconcile.Request {
 	jobLabels := job.GetLabels()
 	chartName, ok := jobLabels[chart.Label]

internal/upgrade/helm.go

@@ -12,3 +12,33 @@ func ChartNamespacedName(chart string) types.NamespacedName {
 		Namespace: ChartNamespace,
 	}
 }
+
+type HelmChartState int
+
+const (
+	ChartStateUnknown HelmChartState = iota
+	ChartStateNotInstalled
+	ChartStateVersionAlreadyInstalled
+	ChartStateInProgress
+	ChartStateFailed
+	ChartStateSucceeded
+)
+
+func (s HelmChartState) Message() string {
+	switch s {
+	case ChartStateUnknown:
+		return "Chart state is unknown"
+	case ChartStateNotInstalled:
+		return "Chart is not installed"
+	case ChartStateVersionAlreadyInstalled:
+		return "Chart version is already installed"
+	case ChartStateInProgress:
+		return "Chart upgrade is in progress"
+	case ChartStateFailed:
+		return "Chart upgrade failed"
+	case ChartStateSucceeded:
+		return "Chart upgrade succeeded"
+	default:
+		return ""
+	}
+}