WIP: allow adoption based on v2beta1 state

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

api/v2beta2/helmrelease_types.go

@@ -908,6 +908,11 @@ type HelmReleaseStatus struct {
 	// +optional
 	LastAttemptedValuesChecksum string `json:"lastAttemptedValuesChecksum,omitempty"`
 
+	// LastReleaseRevision is the revision of the last successful Helm release.
+	// Deprecated: Use History instead.
+	// +optional
+	LastReleaseRevision int `json:"lastReleaseRevision,omitempty"`
+
 	// LastAttemptedConfigDigest is the digest for the config (better known as
 	// "values") of the last reconciliation attempt.
 	// +optional

config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml

@@ -2049,6 +2049,10 @@ spec:
                   reconcile request value, so a change of the annotation value can
                   be detected.
                 type: string
+              lastReleaseRevision:
+                description: 'LastReleaseRevision is the revision of the last successful
+                  Helm release. Deprecated: Use History instead.'
+                type: integer
               observedGeneration:
                 description: ObservedGeneration is the last observed generation.
                 format: int64

docs/api/v2beta2/helm.md

@@ -1407,6 +1407,19 @@ Deprecated: Use LastAttemptedConfigDigest instead.</p>
 </tr>
 <tr>
 <td>
+<code>lastReleaseRevision</code><br>
+<em>
+int
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>LastReleaseRevision is the revision of the last successful Helm release.
+Deprecated: Use History instead.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>lastAttemptedConfigDigest</code><br>
 <em>
 string

internal/controller/helmrelease_controller.go

@@ -60,10 +60,12 @@ import (
 	"github.com/fluxcd/helm-controller/internal/action"
 	"github.com/fluxcd/helm-controller/internal/chartutil"
 	"github.com/fluxcd/helm-controller/internal/digest"
+	"github.com/fluxcd/helm-controller/internal/features"
 	"github.com/fluxcd/helm-controller/internal/kube"
 	"github.com/fluxcd/helm-controller/internal/loader"
 	intpredicates "github.com/fluxcd/helm-controller/internal/predicates"
 	intreconcile "github.com/fluxcd/helm-controller/internal/reconcile"
+	"github.com/fluxcd/helm-controller/internal/release"
 )
 
 // +kubebuilder:rbac:groups=helm.toolkit.fluxcd.io,resources=helmreleases,verbs=get;list;watch;create;update;patch;delete
@@ -288,6 +290,16 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 		return ctrl.Result{}, err
 	}
 
+	// Attempt to adopt "legacy" v2beta1 release state on a best-effort basis.
+	// If this fails, the controller will fall back to performing an upgrade
+	// to settle on the desired state.
+	// TODO(hidde): remove this in a future release.
+	if ok, _ := features.Enabled(features.AdoptLegacyReleases); ok {
+		if err := r.adoptLegacyRelease(ctx, getter, obj); err != nil {
+			log.Error(err, "failed to adopt v2beta1 release state")
+		}
+	}
+
 	// 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.
@@ -315,6 +327,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
 	obj.Status.LastAttemptedRevision = loadedChart.Metadata.Version
 	obj.Status.LastAttemptedConfigDigest = chartutil.DigestValues(digest.Canonical, values).String()
 	obj.Status.LastAttemptedValuesChecksum = ""
+	obj.Status.LastReleaseRevision = 0
 
 	// Construct config factory for any further Helm actions.
 	cfg, err := action.NewConfigFactory(getter,
@@ -508,6 +521,46 @@ func (r *HelmReleaseReconciler) checkDependencies(ctx context.Context, obj *v2.H
 	return nil
 }
 
+// adoptLegacyRelease attempts to adopt a v2beta1 release into a v2beta2
+// release.
+// This is done by retrieving the last successful release from the Helm storage
+// and converting it to a v2beta2 release snapshot.
+// If the v2beta1 release has already been adopted, this function is a no-op.
+func (r *HelmReleaseReconciler) adoptLegacyRelease(ctx context.Context, getter genericclioptions.RESTClientGetter, obj *v2.HelmRelease) error {
+	if obj.Status.LastReleaseRevision < 1 || len(obj.Status.History) > 0 {
+		return nil
+	}
+
+	// Construct config factory for current release.
+	cfg, err := action.NewConfigFactory(getter,
+		action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()),
+		action.WithStorageLog(action.NewDebugLog(ctrl.LoggerFrom(ctx).V(logger.TraceLevel))),
+	)
+
+	// Get the last successful release based on the observation for the v2beta1
+	// object.
+	rls, err := cfg.NewStorage().Get(obj.GetReleaseName(), obj.Status.LastReleaseRevision)
+	if err != nil {
+		return err
+	}
+
+	// Convert it to a v2beta2 release snapshot.
+	snap := release.ObservedToSnapshot(release.ObserveRelease(rls))
+
+	// If tests are enabled, include them as well.
+	if obj.GetTest().Enable {
+		snap.SetTestHooks(release.TestHooksFromRelease(rls))
+	}
+
+	// Adopt it as the current release in the history.
+	obj.Status.History = append(obj.Status.History, snap)
+
+	// Erase the last release revision from the status.
+	obj.Status.LastReleaseRevision = 0
+
+	return nil
+}
+
 func (r *HelmReleaseReconciler) buildRESTClientGetter(ctx context.Context, obj *v2.HelmRelease) (genericclioptions.RESTClientGetter, error) {
 	opts := []kube.Option{
 		kube.WithNamespace(obj.GetReleaseNamespace()),

internal/controller/helmrelease_controller_test.go

@@ -607,7 +607,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
 			Spec: v2.HelmReleaseSpec{
 				// Trigger a failure by setting an invalid storage namespace,
 				// preventing the release from actually being installed.
-				// This allows us to just test the , without
+				// This allows us to just test the values being set, without
 				// having to facilitate a full install.
 				StorageNamespace: "not-exist",
 				Values: &apiextensionsv1.JSON{
@@ -1404,6 +1404,174 @@ func TestHelmReleaseReconciler_checkDependencies(t *testing.T) {
 	}
 }
 
+func TestHelmReleaseReconciler_adoptLegacyRelease(t *testing.T) {
+	tests := []struct {
+		name                      string
+		releases                  func(namespace string) []*helmrelease.Release
+		spec                      func(spec *v2.HelmReleaseSpec)
+		status                    v2.HelmReleaseStatus
+		expectHistory             func(releases []*helmrelease.Release) v2.Snapshots
+		expectLastReleaseRevision int
+		wantErr                   bool
+	}{
+		{
+			name: "adopts last release revision",
+			releases: func(namespace string) []*helmrelease.Release {
+				return []*helmrelease.Release{
+					testutil.BuildRelease(&helmrelease.MockReleaseOptions{
+						Name:      "orphaned",
+						Namespace: namespace,
+						Version:   6,
+						Chart:     testutil.BuildChart(),
+						Status:    helmrelease.StatusDeployed,
+					}, testutil.ReleaseWithTestHook()),
+				}
+			},
+			spec: func(spec *v2.HelmReleaseSpec) {
+				spec.ReleaseName = "orphaned"
+			},
+			status: v2.HelmReleaseStatus{
+				LastReleaseRevision: 6,
+			},
+			expectHistory: func(releases []*helmrelease.Release) v2.Snapshots {
+				return v2.Snapshots{
+					release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
+				}
+			},
+			expectLastReleaseRevision: 0,
+		},
+		{
+			name: "includes test hooks if enabled",
+			releases: func(namespace string) []*helmrelease.Release {
+				return []*helmrelease.Release{
+					testutil.BuildRelease(&helmrelease.MockReleaseOptions{
+						Name:      "orphaned-with-hooks",
+						Namespace: namespace,
+						Version:   3,
+						Chart:     testutil.BuildChart(testutil.ChartWithTestHook()),
+						Status:    helmrelease.StatusDeployed,
+					}, testutil.ReleaseWithTestHook()),
+				}
+			},
+			spec: func(spec *v2.HelmReleaseSpec) {
+				spec.ReleaseName = "orphaned-with-hooks"
+				spec.Test = &v2.Test{
+					Enable: true,
+				}
+			},
+			status: v2.HelmReleaseStatus{
+				LastReleaseRevision: 3,
+			},
+			expectHistory: func(releases []*helmrelease.Release) v2.Snapshots {
+				snap := release.ObservedToSnapshot(release.ObserveRelease(releases[0]))
+				snap.SetTestHooks(release.TestHooksFromRelease(releases[0]))
+
+				return v2.Snapshots{
+					snap,
+				}
+			},
+			expectLastReleaseRevision: 0,
+		},
+		{
+			name: "non-existing release",
+			spec: func(spec *v2.HelmReleaseSpec) {
+				spec.ReleaseName = "non-existing"
+			},
+			status: v2.HelmReleaseStatus{
+				LastReleaseRevision: 2,
+			},
+			expectLastReleaseRevision: 2,
+			wantErr:                   true,
+		},
+		{
+			name: "without last release revision",
+			status: v2.HelmReleaseStatus{
+				LastReleaseRevision: 0,
+			},
+			expectHistory: func(releases []*helmrelease.Release) v2.Snapshots {
+				return nil
+			},
+			expectLastReleaseRevision: 0,
+		},
+		{
+			name: "with existing history",
+			status: v2.HelmReleaseStatus{
+				History: v2.Snapshots{
+					{
+						Name: "something",
+					},
+				},
+				LastReleaseRevision: 5,
+			},
+			expectHistory: func(releases []*helmrelease.Release) v2.Snapshots {
+				return v2.Snapshots{
+					{
+						Name: "something",
+					},
+				}
+			},
+			expectLastReleaseRevision: 5,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			// Create a test namespace for storing the Helm release mock.
+			ns, err := testEnv.CreateNamespace(context.TODO(), "adopt-release")
+			g.Expect(err).ToNot(HaveOccurred())
+			t.Cleanup(func() {
+				_ = testEnv.Delete(context.TODO(), ns)
+			})
+
+			// Mock a HelmRelease object.
+			obj := &v2.HelmRelease{
+				Spec: v2.HelmReleaseSpec{
+					StorageNamespace: ns.Name,
+				},
+				Status: tt.status,
+			}
+			if tt.spec != nil {
+				tt.spec(&obj.Spec)
+			}
+
+			r := &HelmReleaseReconciler{
+				Client:           testEnv.Client,
+				GetClusterConfig: GetTestClusterConfig,
+			}
+
+			// 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.GetStorageNamespace()))
+			g.Expect(err).ToNot(HaveOccurred())
+
+			var releases []*helmrelease.Release
+			if tt.releases != nil {
+				releases = tt.releases(ns.Name)
+			}
+			store := helmstorage.Init(cfg.Driver)
+			for _, rls := range releases {
+				g.Expect(store.Create(rls)).To(Succeed())
+			}
+
+			// Adopt the Helm release mock.
+			err = r.adoptLegacyRelease(context.TODO(), getter, obj)
+			g.Expect(err != nil).To(Equal(tt.wantErr), "unexpected error: %s", err)
+
+			// Verify the Helm release mock has been adopted.
+			var expectHistory v2.Snapshots
+			if tt.expectHistory != nil {
+				expectHistory = tt.expectHistory(releases)
+			}
+			g.Expect(obj.Status.History).To(Equal(expectHistory))
+			g.Expect(obj.Status.LastReleaseRevision).To(Equal(tt.expectLastReleaseRevision))
+		})
+	}
+}
+
 func TestHelmReleaseReconciler_buildRESTClientGetter(t *testing.T) {
 	const (
 		namespace = "some-namespace"

internal/features/features.go

@@ -47,6 +47,13 @@ const (
 	// OOMWatch enables the OOM watcher, which will gracefully shut down the controller
 	// when the memory usage exceeds the configured limit. This is disabled by default.
 	OOMWatch = "OOMWatch"
+
+	// AdoptLegacyReleases enables the adoption of the historical Helm release
+	// based on the status fields from a v2beta1 HelmRelease object.
+	// This is enabled by default to support an upgrade path from v2beta1 to v2beta2
+	// without the need to upgrade the Helm release. But it can be disabled to
+	// avoid potential abuse of the adoption mechanism.
+	AdoptLegacyReleases = "AdoptLegacyReleases"
 )
 
 var features = map[string]bool{
@@ -65,6 +72,9 @@ var features = map[string]bool{
 	// OOMWatch
 	// opt-in from v0.31
 	OOMWatch: false,
+	// AdoptLegacyReleases
+	// opt-out from v0.37
+	AdoptLegacyReleases: true,
 }
 
 // FeatureGates contains a list of all supported feature gates and