From a563385584429726f89221f1dc94b8c6042552b2 Mon Sep 17 00:00:00 2001 From: MichaelMorris Date: Thu, 19 Mar 2026 14:14:21 +0000 Subject: [PATCH] Support helm release upgrade on helm chart name change This re-introduces support that was lost going from 2.1.x -> 2.2.x but controls the behaviour through a feature gate so either the previous or current behaviour (default) can be achieved Signed-off-by: MichaelMorris --- internal/action/verify.go | 4 +- internal/action/verify_test.go | 2 +- internal/controller/helmrelease_controller.go | 3 +- .../controller/helmrelease_controller_test.go | 103 ++++++++++++++++++ internal/features/features.go | 8 ++ main.go | 7 ++ 6 files changed, 123 insertions(+), 4 deletions(-) diff --git a/internal/action/verify.go b/internal/action/verify.go index 3407db143..69944dc9a 100644 --- a/internal/action/verify.go +++ b/internal/action/verify.go @@ -46,7 +46,7 @@ const ( targetStorageNamespace = "storage namespace" targetReleaseNamespace = "release namespace" targetReleaseName = "release name" - targetChartName = "chart name" + TargetChartName = "chart name" ) // ReleaseTargetChanged returns a reason and true if the given release and/or @@ -68,7 +68,7 @@ func ReleaseTargetChanged(obj *v2.HelmRelease, chartName string) (string, bool) case release.ShortenName(obj.GetReleaseName()) != cur.Name: return targetReleaseName, true case chartName != cur.ChartName: - return targetChartName, true + return TargetChartName, true default: return "", false } diff --git a/internal/action/verify_test.go b/internal/action/verify_test.go index c26b2b57f..ea90ee518 100644 --- a/internal/action/verify_test.go +++ b/internal/action/verify_test.go @@ -167,7 +167,7 @@ func TestReleaseTargetChanged(t *testing.T) { }, StorageNamespace: defaultNamespace, }, - wantReason: targetChartName, + wantReason: TargetChartName, want: true, }, { diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index d772ee6a2..123c4e9bd 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -112,6 +112,7 @@ type HelmReleaseReconciler struct { DefaultToRetryOnFailure bool DirectSourceFetch bool DisableChartDigestTracking bool + UninstallOnChartNameChange bool } const terminalErrorMessage = "Reconciliation failed terminally due to configuration error" @@ -371,7 +372,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, // If the release target configuration has changed, we need to uninstall the // previous release target first. If we did not do this, the installation would // fail due to resources already existing. - if reason, changed := action.ReleaseTargetChanged(obj, loadedChart.Name()); changed { + if reason, changed := action.ReleaseTargetChanged(obj, loadedChart.Name()); changed && (reason != action.TargetChartName || r.UninstallOnChartNameChange) { log.Info(fmt.Sprintf("release target configuration changed (%s): running uninstall for current release", reason)) if err = r.reconcileUninstall(ctx, getter, obj); err != nil && !errors.Is(err, intreconcile.ErrNoLatest) { return ctrl.Result{}, err diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index 9e48fda8c..dda2822e3 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -486,6 +486,109 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { g.Expect(obj.Status.StorageNamespace).To(BeEmpty()) }) + t.Run("Upgrades HelmRelease if chart name changed with feature gate set", func(t *testing.T) { + g := NewWithT(t) + + // Create HelmChart mock. + chartMock := testutil.BuildChart() + chartArtifact, err := testutil.SaveChartAsArtifact(chartMock, digest.SHA256, testServer.URL(), testServer.Root()) + g.Expect(err).ToNot(HaveOccurred()) + + ns, err := testEnv.CreateNamespace(context.TODO(), "mock") + g.Expect(err).ToNot(HaveOccurred()) + t.Cleanup(func() { + _ = testEnv.Delete(context.TODO(), ns) + }) + + hc := &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: chartMock.Name(), + Namespace: ns.Name, + Generation: 1, + }, + Spec: sourcev1.HelmChartSpec{ + Chart: chartMock.Name(), + Version: chartMock.Metadata.Version, + }, + Status: sourcev1.HelmChartStatus{ + ObservedGeneration: 1, + Artifact: chartArtifact, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + // Create a test Helm release storage mock. + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: "release", + Namespace: ns.Name, + Version: 1, + Chart: chartMock, + Status: helmreleasecommon.StatusDeployed, + }) + + snapshot := release.ObservedToSnapshot(release.ObserveRelease(rls)) + snapshot.ChartName = "oldChartName" + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: ns.Name, + }, + Spec: v2.HelmReleaseSpec{ + ChartRef: &v2.CrossNamespaceSourceReference{ + Kind: sourcev1.HelmChartKind, + Name: hc.Name, + }, + }, + Status: v2.HelmReleaseStatus{ + StorageNamespace: ns.Name, + History: v2.Snapshots{ + snapshot, + }, + HelmChart: hc.Namespace + "/" + hc.Name, + }, + } + + c := fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(hc, obj). + Build() + + r := &HelmReleaseReconciler{ + Client: c, + GetClusterConfig: GetTestClusterConfig, + EventRecorder: record.NewFakeRecorder(32), + UninstallOnChartNameChange: false, + } + + //Store the Helm release mock in the test namespace. + getter, err := r.buildRESTClientGetter(context.TODO(), obj) + g.Expect(err).ToNot(HaveOccurred()) + + cfg, err := action.NewConfigFactory(getter, action.WithStorage(helmdriver.SecretsDriverName, obj.Status.StorageNamespace)) + g.Expect(err).ToNot(HaveOccurred()) + + store := helmstorage.Init(cfg.Driver) + g.Expect(store.Create(rls)).To(Succeed()) + + _, err = r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj, nil) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded for release %s with chart %s", + fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(), + chartMock.Metadata.Version)), + *conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded for release %s with chart %s", + fmt.Sprintf("%s/%s.v%d", rls.Namespace, rls.Name, rls.Version+1), fmt.Sprintf("%s@%s", chartMock.Name(), + chartMock.Metadata.Version)), + })) + }) + t.Run("resets failure counts on configuration change", func(t *testing.T) { g := NewWithT(t) diff --git a/internal/features/features.go b/internal/features/features.go index e385bc731..88275fff4 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -87,6 +87,11 @@ const ( // auto-clears failures on success, providing better UX especially // when CancelHealthCheckOnNewRevision is enabled. DefaultToRetryOnFailure = "DefaultToRetryOnFailure" + + // UninstallOnChartNameChange controls if a release is uninstalled when the chart + // name changes resulting in a new install of the release. This is enabled by + // default. When disabled, an upgrade will be attempted + UninstallOnChartNameChange = "UninstallOnChartNameChange" ) var features = map[string]bool{ @@ -132,6 +137,9 @@ var features = map[string]bool{ // DefaultToRetryOnFailure // opt-in from v1.5.2 DefaultToRetryOnFailure: false, + // UninstallOnChartNameChange + // opt-in from v1.5.4 + UninstallOnChartNameChange: true, } func init() { diff --git a/main.go b/main.go index 6f5668327..922f690a1 100644 --- a/main.go +++ b/main.go @@ -254,6 +254,12 @@ func main() { "adoption of HelmRelease resources in legacy API versions is no longer supported") } + uninstallOnChartNameChange, err := features.Enabled((features.UninstallOnChartNameChange)) + if err != nil { + setupLog.Error(err, "unable to check feature gate "+features.UninstallOnChartNameChange) + os.Exit(1) + } + // Set the managedFields owner for resources reconciled from Helm charts. kube.ManagedFieldsManager = controllerName @@ -394,6 +400,7 @@ func main() { ArtifactFetchRetries: httpRetry, AllowExternalArtifact: allowExternalArtifact, DisallowedFieldManagers: disallowedFieldManagers, + UninstallOnChartNameChange: uninstallOnChartNameChange, }).SetupWithManager(ctx, mgr, controller.HelmReleaseReconcilerOptions{ RateLimiter: helper.GetRateLimiter(rateLimiterOptions), WatchConfigs: watchConfigs,