diff --git a/internal/reconcile/uninstall.go b/internal/reconcile/uninstall.go index 3d0a10e46..c434c3d76 100644 --- a/internal/reconcile/uninstall.go +++ b/internal/reconcile/uninstall.go @@ -104,6 +104,18 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { return nil } + // When the release is already uninstalled and the user requested to keep + // the history, we can assume the release is uninstalled while taking note + // that we did not do it. + // This can happen when the release was uninstalled as part of a + // remediation, with a subsequent uninstall request due to the object + // being deleted. + if err != nil && req.Object.GetUninstall().KeepHistory && strings.Contains(err.Error(), "is already deleted") { + conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallSucceededReason, + "Release %s was already uninstalled", cur.FullReleaseName()) + return nil + } + // The Helm uninstall action does always target the latest release. Before // accepting results, we need to confirm this is actually the release we // have recorded as Current. @@ -115,7 +127,12 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { // The Helm uninstall action may return without an error while the update // to the storage failed. Detect this and return an error. if err == nil && cur.Digest == req.Object.GetCurrent().Digest { - err = fmt.Errorf("uninstall completed with error: %w", ErrNoStorageUpdate) + // An exception is made for the case where the release was already marked + // as uninstalled, which would only result in the release object getting + // removed from the storage. + if s := helmrelease.Status(cur.Status); s != helmrelease.StatusUninstalled { + err = fmt.Errorf("uninstall completed with error: %w", ErrNoStorageUpdate) + } } // Handle any error. diff --git a/internal/reconcile/uninstall_test.go b/internal/reconcile/uninstall_test.go index 2308b91de..8a9036957 100644 --- a/internal/reconcile/uninstall_test.go +++ b/internal/reconcile/uninstall_test.go @@ -66,7 +66,7 @@ func TestUninstall_Reconcile(t *testing.T) { expectConditions []metav1.Condition // expectCurrent is the expected Current release information in the // HelmRelease after uninstall. - expectCurrent func(releases []*helmrelease.Release) *v2.Snapshot + expectCurrent func(namespace string, releases []*helmrelease.Release) *v2.Snapshot // expectPrevious returns the expected Previous release information of // the HelmRelease after uninstall. expectPrevious func(releases []*helmrelease.Release) *v2.Snapshot @@ -110,7 +110,7 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "succeeded"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) }, }, @@ -145,7 +145,7 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, "uninstallation completed with 1 error(s): 1 error occurred:\n\t* timed out waiting for the condition"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) }, expectFailures: 1, @@ -192,7 +192,7 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, ErrNoStorageUpdate.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) }, expectFailures: 1, @@ -235,7 +235,7 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, "delete error"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) }, expectFailures: 1, @@ -294,7 +294,7 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallFailedReason, ErrReleaseMismatch.Error()), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) }, expectFailures: 1, @@ -337,7 +337,79 @@ func TestUninstall_Reconcile(t *testing.T) { *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, "assuming it is uninstalled"), }, - expectCurrent: func(releases []*helmrelease.Release) *v2.Snapshot { + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { + return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) + }, + }, + { + name: "already uninstalled without keep history", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusUninstalled, + }), + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.ReleaseHistory{ + Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallSucceededReason, + "succeeded"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, + "succeeded"), + }, + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { + rls := testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusUninstalled, + }) + return release.ObservedToSnapshot(release.ObserveRelease(rls)) + }, + }, + { + name: "already uninstalled with keep history", + releases: func(namespace string) []*helmrelease.Release { + return []*helmrelease.Release{ + testutil.BuildRelease(&helmrelease.MockReleaseOptions{ + Name: mockReleaseName, + Namespace: namespace, + Version: 1, + Chart: testutil.BuildChart(testutil.ChartWithTestHook()), + Status: helmrelease.StatusUninstalled, + }), + } + }, + spec: func(spec *v2.HelmReleaseSpec) { + spec.Uninstall = &v2.Uninstall{ + KeepHistory: true, + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + History: v2.ReleaseHistory{ + Current: release.ObservedToSnapshot(release.ObserveRelease(releases[0])), + }, + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallSucceededReason, + "was already uninstalled"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, + "was already uninstalled"), + }, + expectCurrent: func(namespace string, releases []*helmrelease.Release) *v2.Snapshot { return release.ObservedToSnapshot(release.ObserveRelease(releases[0])) }, }, @@ -407,7 +479,7 @@ func TestUninstall_Reconcile(t *testing.T) { releaseutil.SortByRevision(releases) if tt.expectCurrent != nil { - g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releases))) + g.Expect(obj.GetCurrent()).To(testutil.Equal(tt.expectCurrent(releaseNamespace, releases))) } else { g.Expect(obj.GetCurrent()).To(BeNil(), "expected current to be nil") }