From 18eb67956e44fc90fb7617815f6136ad17f5dcf6 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 27 Jul 2023 22:49:41 +0200 Subject: [PATCH] reconcile: handle manually uninstalled release This is a better way of dealing with this situation, as the previous logic would result in an `ErrNoStorageUpdate`. Signed-off-by: Hidde Beydals --- internal/reconcile/uninstall.go | 8 +++++- internal/reconcile/uninstall_test.go | 39 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/internal/reconcile/uninstall.go b/internal/reconcile/uninstall.go index bcebc2617..16037e68e 100644 --- a/internal/reconcile/uninstall.go +++ b/internal/reconcile/uninstall.go @@ -94,8 +94,14 @@ func (r *Uninstall) Reconcile(ctx context.Context, req *Request) error { // Run the Helm uninstall action. res, err := action.Uninstall(ctx, cfg, req.Object, cur.Name) + + // When the release is not found, something else has already uninstalled + // the release. As such, we can assume the release is uninstalled while + // taking note that we did not do it. if errors.Is(err, helmdriver.ErrReleaseNotFound) { - err = nil + conditions.MarkFalse(req.Object, v2.ReleasedCondition, v2.UninstallSucceededReason, + "Release %s was not found, assuming it is uninstalled", cur.FullReleaseName()) + return nil } // The Helm uninstall action does always target the latest release. Before diff --git a/internal/reconcile/uninstall_test.go b/internal/reconcile/uninstall_test.go index 20dc050ae..8ec8a68be 100644 --- a/internal/reconcile/uninstall_test.go +++ b/internal/reconcile/uninstall_test.go @@ -291,6 +291,45 @@ func TestUninstall_Reconcile(t *testing.T) { expectFailures: 1, wantErr: ErrReleaseMismatch, }, + { + name: "uninstall already deleted release", + driver: func(driver helmdriver.Driver) helmdriver.Driver { + return &storage.Failing{ + // Explicitly inherit the driver, as we want to rely on the + // Secret storage, as the memory storage does not detach + // objects from the release action. Causing writes post-persist + // to leak to the stored release object. + // xref: https://github.com/helm/helm/issues/11304 + Driver: driver, + QueryErr: helmdriver.ErrReleaseNotFound, + } + }, + 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.StatusDeployed, + }), + } + }, + status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus { + return v2.HelmReleaseStatus{ + Current: release.ObservedToInfo(release.ObserveRelease(releases[0])), + } + }, + expectConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, v2.UninstallSucceededReason, + "assuming it is uninstalled"), + *conditions.FalseCondition(v2.ReleasedCondition, v2.UninstallSucceededReason, + "assuming it is uninstalled"), + }, + expectCurrent: func(releases []*helmrelease.Release) *v2.HelmReleaseInfo { + return release.ObservedToInfo(release.ObserveRelease(releases[0])) + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {