Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ActionClientGetter: allow disabling rollbacks after failures #348

Merged
merged 1 commit into from
Jul 8, 2024
Merged

ActionClientGetter: allow disabling rollbacks after failures #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions pkg/client/actionclient.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -133,6 +143,7 @@ type actionClientGetter struct {
defaultUpgradeOpts []UpgradeOption
defaultUninstallOpts []UninstallOption

enableFailureRollbacks bool
installFailureUninstallOpts []UninstallOption
upgradeFailureRollbackOpts []RollbackOption

Expand Down Expand Up @@ -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
Expand All @@ -180,6 +192,7 @@ type actionClient struct {
defaultUpgradeOpts []UpgradeOption
defaultUninstallOpts []UninstallOption

enableFailureRollbacks bool
installFailureUninstallOpts []UninstallOption
upgradeFailureRollbackOpts []RollbackOption
}
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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
}
Expand Down
61 changes: 50 additions & 11 deletions pkg/client/actionclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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") }
Expand Down Expand Up @@ -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") }
Expand Down