ActionClientGetter: allow disabling rollbacks after failures (#348)

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.

Author: joelanford

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
 }

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).ToNot(HaveOccurred())
+						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).ToNot(HaveOccurred())
+						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") }