fluxcd
/
helm-controller
mirror of https://github.com/fluxcd/helm-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

reconcile: improve uninstall w/o purging history 

This improves the reconciliation of an uninstall when the release has
already been uninstalled while `KeepHistory` has been set, by detecting
the (sadly non-typed) error Helm produces as desired state.

Avoiding certain edge-cases where for example a deleted HelmRelease
would end up in an irrecoverable loop of uninstall attempts, after
being remediated (using an uninstall) before the deletion request.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
This commit is contained in:
Hidde Beydals 2023-11-01 13:14:59 +01:00
parent 191bebfafd
commit d0c4c14056
No known key found for this signature in database
GPG Key ID: 979F380FC2341744
2 changed files with 98 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
internal/reconcile/uninstall.go
View File

@ -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.

88
internal/reconcile/uninstall_test.go
View File

@ -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")
}