From cdd373d2710db0db85da252104162c028ecc6258 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 3 Jul 2024 17:31:18 -0400 Subject: [PATCH] ActionClientGetter: allow disabling rollbacks after failures When an install fails, we uninstall the failed release. When an upgrade fails, we rollback to the previous release. This commit makes it possible to disable that behavior, which results in the latest release being left in a Failed state for the caller to decide how to act. --- pkg/client/actionclient.go | 23 ++++++++++--- pkg/client/actionclient_test.go | 61 +++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index 4b6c6de0..5cba3dd5 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -115,8 +115,18 @@ func AppendPostRenderers(postRendererFns ...PostRendererProvider) ActionClientGe } } +func WithFailureRollbacks(enableFailureRollbacks bool) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.enableFailureRollbacks = enableFailureRollbacks + return nil + } +} + func NewActionClientGetter(acg ActionConfigGetter, opts ...ActionClientGetterOption) (ActionClientGetter, error) { - actionClientGetter := &actionClientGetter{acg: acg} + actionClientGetter := &actionClientGetter{ + acg: acg, + enableFailureRollbacks: true, + } for _, opt := range opts { if err := opt(actionClientGetter); err != nil { return nil, err @@ -133,6 +143,7 @@ type actionClientGetter struct { defaultUpgradeOpts []UpgradeOption defaultUninstallOpts []UninstallOption + enableFailureRollbacks bool installFailureUninstallOpts []UninstallOption upgradeFailureRollbackOpts []RollbackOption @@ -167,6 +178,7 @@ func (hcg *actionClientGetter) ActionClientFor(ctx context.Context, obj client.O defaultUpgradeOpts: append([]UpgradeOption{WithUpgradePostRenderer(cpr)}, hcg.defaultUpgradeOpts...), defaultUninstallOpts: hcg.defaultUninstallOpts, + enableFailureRollbacks: hcg.enableFailureRollbacks, installFailureUninstallOpts: hcg.installFailureUninstallOpts, upgradeFailureRollbackOpts: hcg.upgradeFailureRollbackOpts, }, nil @@ -180,6 +192,7 @@ type actionClient struct { defaultUpgradeOpts []UpgradeOption defaultUninstallOpts []UninstallOption + enableFailureRollbacks bool installFailureUninstallOpts []UninstallOption upgradeFailureRollbackOpts []RollbackOption } @@ -209,7 +222,7 @@ func (c *actionClient) Install(name, namespace string, chrt *chart.Chart, vals m rel, err := install.Run(chrt, vals) if err != nil { c.conf.Log("Install failed") - if rel != nil { + if c.enableFailureRollbacks && rel != nil { // Uninstall the failed release installation so that we can retry // the installation again during the next reconciliation. In many // cases, the issue is unresolvable without a change to the CR, but @@ -228,7 +241,7 @@ func (c *actionClient) Install(name, namespace string, chrt *chart.Chart, vals m return nil, fmt.Errorf("uninstall failed: %v: original install error: %w", uninstallErr, err) } } - return nil, err + return rel, err } return rel, nil } @@ -243,7 +256,7 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m upgrade.Namespace = namespace rel, err := upgrade.Run(name, chrt, vals) if err != nil { - if rel != nil { + if c.enableFailureRollbacks && rel != nil { rollbackOpts := append([]RollbackOption{func(rollback *action.Rollback) error { rollback.Force = true rollback.MaxHistory = upgrade.MaxHistory @@ -260,7 +273,7 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return nil, fmt.Errorf("rollback failed: %v: original upgrade error: %w", rollbackErr, err) } } - return nil, err + return rel, err } return rel, nil } diff --git a/pkg/client/actionclient_test.go b/pkg/client/actionclient_test.go index f1dd3a2f..608b9554 100644 --- a/pkg/client/actionclient_test.go +++ b/pkg/client/actionclient_test.go @@ -322,17 +322,19 @@ var _ = Describe("ActionClient", func() { var _ = Describe("ActionClient methods", func() { var ( - obj client.Object - cl client.Client - ac ActionInterface - vals = chartutil.Values{"service": map[string]interface{}{"type": "NodePort"}} + obj client.Object + cl client.Client + actionCfgGetter ActionConfigGetter + ac ActionInterface + vals = chartutil.Values{"service": map[string]interface{}{"type": "NodePort"}} ) BeforeEach(func() { obj = testutil.BuildTestCR(gvk) - actionConfigGetter, err := NewActionConfigGetter(cfg, rm) + var err error + actionCfgGetter, err = NewActionConfigGetter(cfg, rm) Expect(err).ShouldNot(HaveOccurred()) - acg, err := NewActionClientGetter(actionConfigGetter) + acg, err := NewActionClientGetter(actionCfgGetter) Expect(err).ToNot(HaveOccurred()) ac, err = acg.ActionClientFor(context.Background(), obj) Expect(err).ToNot(HaveOccurred()) @@ -373,14 +375,32 @@ var _ = Describe("ActionClient", func() { }) It("should uninstall a failed install", func() { By("failing to install the release", func() { - chrt := testutil.MustLoadChart("../../pkg/internal/testdata/test-chart-1.2.0.tgz") - chrt.Templates[2].Data = append(chrt.Templates[2].Data, []byte("\ngibberish")...) + vals := chartutil.Values{"service": map[string]interface{}{"type": "FooBar"}} r, err := ac.Install(obj.GetName(), obj.GetNamespace(), &chrt, vals) Expect(err).To(HaveOccurred()) - Expect(r).To(BeNil()) + Expect(r).NotTo(BeNil()) }) verifyNoRelease(cl, obj.GetNamespace(), obj.GetName(), nil) }) + When("failure uninstall is disabled", func() { + BeforeEach(func() { + acg, err := NewActionClientGetter(actionCfgGetter, WithFailureRollbacks(false)) + Expect(err).ToNot(HaveOccurred()) + ac, err = acg.ActionClientFor(context.Background(), obj) + Expect(err).ToNot(HaveOccurred()) + }) + It("should not uninstall a failed install", func() { + vals := chartutil.Values{"service": map[string]interface{}{"type": "FooBar"}} + returnedRelease, err := ac.Install(obj.GetName(), obj.GetNamespace(), &chrt, vals) + Expect(err).To(HaveOccurred()) + Expect(returnedRelease).ToNot(BeNil()) + Expect(returnedRelease.Info.Status).To(Equal(release.StatusFailed)) + latestRelease, err := ac.Get(obj.GetName()) + Expect(err).To(BeNil()) + Expect(latestRelease).ToNot(BeNil()) + Expect(latestRelease.Version).To(Equal(returnedRelease.Version)) + }) + }) When("using an option function that returns an error", func() { It("should fail", func() { opt := func(*action.Install) error { return errors.New("expect this error") } @@ -484,17 +504,36 @@ var _ = Describe("ActionClient", func() { verifyRelease(cl, obj, rel) }) It("should rollback a failed upgrade", func() { - By("failing to install the release", func() { + By("failing to upgrade the release", func() { vals := chartutil.Values{"service": map[string]interface{}{"type": "FooBar"}} r, err := ac.Upgrade(obj.GetName(), obj.GetNamespace(), &chrt, vals) Expect(err).To(HaveOccurred()) - Expect(r).To(BeNil()) + Expect(r).ToNot(BeNil()) }) tmp := *installedRelease rollbackRelease := &tmp rollbackRelease.Version = installedRelease.Version + 2 verifyRelease(cl, obj, rollbackRelease) }) + When("failure rollback is disabled", func() { + BeforeEach(func() { + acg, err := NewActionClientGetter(actionCfgGetter, WithFailureRollbacks(false)) + Expect(err).ToNot(HaveOccurred()) + ac, err = acg.ActionClientFor(context.Background(), obj) + Expect(err).ToNot(HaveOccurred()) + }) + It("should not rollback a failed upgrade", func() { + vals := chartutil.Values{"service": map[string]interface{}{"type": "FooBar"}} + returnedRelease, err := ac.Upgrade(obj.GetName(), obj.GetNamespace(), &chrt, vals) + Expect(err).To(HaveOccurred()) + Expect(returnedRelease).ToNot(BeNil()) + Expect(returnedRelease.Info.Status).To(Equal(release.StatusFailed)) + latestRelease, err := ac.Get(obj.GetName()) + Expect(err).To(BeNil()) + Expect(latestRelease).ToNot(BeNil()) + Expect(latestRelease.Version).To(Equal(returnedRelease.Version)) + }) + }) When("using an option function that returns an error", func() { It("should fail", func() { opt := func(*action.Upgrade) error { return errors.New("expect this error") }