From a40503ed050ff1d7e55548b695f92a3f537f00f4 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Sat, 26 Jun 2021 14:53:42 +0200 Subject: [PATCH 1/8] X-Smart-Branch-Parent: main From 1758074ad2d99ac287a6bf4ea9bf50daf2b6df62 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Sat, 26 Jun 2021 15:49:16 +0200 Subject: [PATCH 2/8] handle pending resources --- pkg/client/actionclient.go | 13 ++++ .../internal/conditions/conditions.go | 1 + pkg/reconciler/internal/fake/actionclient.go | 12 ++++ pkg/reconciler/reconciler.go | 68 ++++++++++++++++++- 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index 71fd45a..985d956 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -62,6 +62,7 @@ type ActionInterface interface { Get(name string, opts ...GetOption) (*release.Release, error) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...InstallOption) (*release.Release, error) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...UpgradeOption) (*release.Release, error) + Rollback(name string, opts ...RollbackOption) error Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) Reconcile(rel *release.Release) error } @@ -69,6 +70,7 @@ type ActionInterface interface { type GetOption func(*action.Get) error type InstallOption func(*action.Install) error type UpgradeOption func(*action.Upgrade) error +type RollbackOption func(*action.Rollback) error type UninstallOption func(*action.Uninstall) error func NewActionClientGetter(acg ActionConfigGetter) ActionClientGetter { @@ -180,6 +182,17 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return rel, nil } +func (c *actionClient) Rollback(name string, opts ...RollbackOption) error { + rollback := action.NewRollback(c.conf) + for _, o := range opts { + if err := o(rollback); err != nil { + return err + } + } + rollback.Force = true + return rollback.Run(name) +} + func (c *actionClient) Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) { uninstall := action.NewUninstall(c.conf) for _, o := range opts { diff --git a/pkg/reconciler/internal/conditions/conditions.go b/pkg/reconciler/internal/conditions/conditions.go index 892aee3..a37735b 100644 --- a/pkg/reconciler/internal/conditions/conditions.go +++ b/pkg/reconciler/internal/conditions/conditions.go @@ -41,6 +41,7 @@ const ( ReasonUpgradeError = status.ConditionReason("UpgradeError") ReasonReconcileError = status.ConditionReason("ReconcileError") ReasonUninstallError = status.ConditionReason("UninstallError") + ReasonPendingError = status.ConditionReason("PendingError") ) func Initialized(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { diff --git a/pkg/reconciler/internal/fake/actionclient.go b/pkg/reconciler/internal/fake/actionclient.go index 410c4de..8d39dea 100644 --- a/pkg/reconciler/internal/fake/actionclient.go +++ b/pkg/reconciler/internal/fake/actionclient.go @@ -51,12 +51,14 @@ type ActionClient struct { Gets []GetCall Installs []InstallCall Upgrades []UpgradeCall + Rollbacks []RollbackCall Uninstalls []UninstallCall Reconciles []ReconcileCall HandleGet func() (*release.Release, error) HandleInstall func() (*release.Release, error) HandleUpgrade func() (*release.Release, error) + HandleRollback func() error HandleUninstall func() (*release.UninstallReleaseResponse, error) HandleReconcile func() error } @@ -109,6 +111,11 @@ type UpgradeCall struct { Opts []client.UpgradeOption } +type RollbackCall struct { + Name string + Opts []client.RollbackOption +} + type UninstallCall struct { Name string Opts []client.UninstallOption @@ -133,6 +140,11 @@ func (c *ActionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return c.HandleUpgrade() } +func (c *ActionClient) Rollback(name string, opts ...client.RollbackOption) error { + c.Rollbacks = append(c.Rollbacks, RollbackCall{name, opts}) + return c.HandleRollback() +} + func (c *ActionClient) Uninstall(name string, opts ...client.UninstallOption) (*release.UninstallReleaseResponse, error) { c.Uninstalls = append(c.Uninstalls, UninstallCall{name, opts}) return c.HandleUninstall() diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 7ee0e64..3b6c0eb 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -78,6 +78,7 @@ type Reconciler struct { skipDependentWatches bool maxConcurrentReconciles int reconcilePeriod time.Duration + autoRollbackAfter time.Duration maxHistory int annotSetupOnce sync.Once @@ -304,6 +305,18 @@ func WithMaxReleaseHistory(maxHistory int) Option { } } +// WithAutoRollbackAfter specifies the duration after which the reconciler will attempt to automatically +// roll back a release that is in a pending (locked) state. +func WithAutoRollbackAfter(duration time.Duration) Option { + return func(r *Reconciler) error { + if duration < 0 { + return errors.New("auto-rollback after duration must not be negative") + } + r.autoRollbackAfter = duration + return nil + } +} + // WithInstallAnnotations is an Option that configures Install annotations // to enable custom action.Install fields to be set based on the value of // annotations found in the custom resource watched by this reconciler. @@ -512,7 +525,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. // We also make sure not to return any errors we encounter so // we can still attempt an uninstall if the CR is being deleted. rel, err := actionClient.Get(obj.GetName()) - if errors.Is(err, driver.ErrReleaseNotFound) { + if errors.Is(err, driver.ErrReleaseNotFound) || (rel != nil && rel.Info != nil && rel.Info.Status == release.StatusUninstalled) { u.UpdateStatus(updater.EnsureCondition(conditions.Deployed(corev1.ConditionFalse, "", ""))) } else if err == nil { ensureDeployedRelease(&u, rel) @@ -553,6 +566,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. ) return ctrl.Result{}, err } + if state == statePending { + return r.handlePending(actionClient, rel, &u, log) + } + u.UpdateStatus(updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", ""))) for _, h := range r.preHooks { @@ -630,6 +647,7 @@ const ( stateNeedsInstall helmReleaseState = "needs install" stateNeedsUpgrade helmReleaseState = "needs upgrade" stateUnchanged helmReleaseState = "unchanged" + statePending helmReleaseState = "pending" stateError helmReleaseState = "error" ) @@ -674,10 +692,14 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta return nil, stateError, err } - if errors.Is(err, driver.ErrReleaseNotFound) { + if errors.Is(err, driver.ErrReleaseNotFound) || (currentRelease != nil && currentRelease.Info != nil && currentRelease.Info.Status == release.StatusUninstalled) { return nil, stateNeedsInstall, nil } + if currentRelease.Info != nil && currentRelease.Info.Status.IsPending() { + return nil, statePending, nil + } + var opts []helmclient.UpgradeOption if r.maxHistory > 0 { opts = append(opts, func(u *action.Upgrade) error { @@ -755,6 +777,48 @@ func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updat return rel, nil } +func (r *Reconciler) handlePending(actionClient helmclient.ActionInterface, rel *release.Release, u *updater.Updater, log logr.Logger) (ctrl.Result, error) { + err := r.doHandlePending(actionClient, rel, log) + if err == nil { + err = errors.New("unknown error handling pending release") + } + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonPendingError, err))) + return ctrl.Result{}, err +} + +func (r *Reconciler) doHandlePending(actionClient helmclient.ActionInterface, rel *release.Release, log logr.Logger) error { + if r.autoRollbackAfter <= 0 { + return errors.New("Release is in a pending (locked) state and cannot be modified. User intervention is required.") + } + if rel.Info == nil || rel.Info.LastDeployed.IsZero() { + return errors.New("Release is in a pending (locked) state and lacks 'last deployed' timestamp. User intervention is required.") + } + if pendingSince := time.Since(rel.Info.LastDeployed.Time); pendingSince < r.autoRollbackAfter { + return fmt.Errorf("Release is in a pending (locked) state and cannot currently be modified. Rollback will be attempted in %v.", r.autoRollbackAfter-pendingSince) + } + + var fixAction string + var fixErr error + if rel.Info.Status == release.StatusPendingInstall { + fixAction = "uninstall" + _, fixErr = actionClient.Uninstall(rel.Name, func(u *action.Uninstall) error { + u.KeepHistory = true + return nil + }) + } else { + fixAction = "rollback" + fixErr = actionClient.Rollback(rel.Name, func(r *action.Rollback) error { + r.Force = true + return nil + }) + } + if fixErr != nil { + return fmt.Errorf("Release is in a pending (locked) state. An attempted %s failed: %w", fixAction, fixErr) + } + return fmt.Errorf("Release was in a pending (locked) state. A %s was performed to allow the next reconciliation to succeed.", fixAction) +} + func (r *Reconciler) reportOverrideEvents(obj runtime.Object) { for k, v := range r.overrideValues { r.eventRecorder.Eventf(obj, "Warning", "ValueOverridden", From 43c9d2928e7e7077f99d72d6020b38bbe30d7c76 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Sat, 26 Jun 2021 15:51:24 +0200 Subject: [PATCH 3/8] small fix --- pkg/client/actionclient.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index 985d956..bab9832 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -189,7 +189,6 @@ func (c *actionClient) Rollback(name string, opts ...RollbackOption) error { return err } } - rollback.Force = true return rollback.Run(name) } From 6c6491c4e933c35a2d5c12a35881e578b925509d Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Mon, 28 Jun 2021 22:09:00 +0200 Subject: [PATCH 4/8] return current release if pending --- pkg/reconciler/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 3b6c0eb..b05746a 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -697,7 +697,7 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta } if currentRelease.Info != nil && currentRelease.Info.Status.IsPending() { - return nil, statePending, nil + return currentRelease, statePending, nil } var opts []helmclient.UpgradeOption From ede5514a815f964e7e7e7b9d08c68ea60b0d6b14 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Mon, 28 Jun 2021 22:10:26 +0200 Subject: [PATCH 5/8] log --- pkg/reconciler/reconciler.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index b05746a..7fa556a 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -801,12 +801,14 @@ func (r *Reconciler) doHandlePending(actionClient helmclient.ActionInterface, re var fixAction string var fixErr error if rel.Info.Status == release.StatusPendingInstall { + log.Info("Attempting uninstall for locked release", "releaseName", rel.Name) fixAction = "uninstall" _, fixErr = actionClient.Uninstall(rel.Name, func(u *action.Uninstall) error { u.KeepHistory = true return nil }) } else { + log.Info("Attempting rollback for locked release", "releaseName", rel.Name) fixAction = "rollback" fixErr = actionClient.Rollback(rel.Name, func(r *action.Rollback) error { r.Force = true From 45344b5a7bd0daadb9590c03464589d559feee65 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Mon, 28 Jun 2021 22:34:11 +0200 Subject: [PATCH 6/8] mark failed instead of rollback --- pkg/client/actionclient.go | 10 ++++- pkg/reconciler/internal/fake/actionclient.go | 36 ++++++++--------- pkg/reconciler/reconciler.go | 41 +++++++------------- 3 files changed, 40 insertions(+), 47 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index bab9832..9ac19e5 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -62,7 +62,7 @@ type ActionInterface interface { Get(name string, opts ...GetOption) (*release.Release, error) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...InstallOption) (*release.Release, error) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...UpgradeOption) (*release.Release, error) - Rollback(name string, opts ...RollbackOption) error + MarkFailed(release *release.Release, reason string) error Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) Reconcile(rel *release.Release) error } @@ -192,6 +192,14 @@ func (c *actionClient) Rollback(name string, opts ...RollbackOption) error { return rollback.Run(name) } +func (c *actionClient) MarkFailed(rel *release.Release, reason string) error { + infoCopy := *rel.Info + releaseCopy := *rel + releaseCopy.Info = &infoCopy + releaseCopy.SetStatus(release.StatusFailed, reason) + return c.conf.Releases.Update(&releaseCopy) +} + func (c *actionClient) Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) { uninstall := action.NewUninstall(c.conf) for _, o := range opts { diff --git a/pkg/reconciler/internal/fake/actionclient.go b/pkg/reconciler/internal/fake/actionclient.go index 8d39dea..2bd6022 100644 --- a/pkg/reconciler/internal/fake/actionclient.go +++ b/pkg/reconciler/internal/fake/actionclient.go @@ -48,19 +48,19 @@ func (hcg *fakeActionClientGetter) ActionClientFor(_ crclient.Object) (client.Ac } type ActionClient struct { - Gets []GetCall - Installs []InstallCall - Upgrades []UpgradeCall - Rollbacks []RollbackCall - Uninstalls []UninstallCall - Reconciles []ReconcileCall + Gets []GetCall + Installs []InstallCall + Upgrades []UpgradeCall + MarkFaileds []MarkFailedCall + Uninstalls []UninstallCall + Reconciles []ReconcileCall - HandleGet func() (*release.Release, error) - HandleInstall func() (*release.Release, error) - HandleUpgrade func() (*release.Release, error) - HandleRollback func() error - HandleUninstall func() (*release.UninstallReleaseResponse, error) - HandleReconcile func() error + HandleGet func() (*release.Release, error) + HandleInstall func() (*release.Release, error) + HandleUpgrade func() (*release.Release, error) + HandleMarkFailed func() error + HandleUninstall func() (*release.UninstallReleaseResponse, error) + HandleReconcile func() error } func NewActionClient() ActionClient { @@ -111,9 +111,9 @@ type UpgradeCall struct { Opts []client.UpgradeOption } -type RollbackCall struct { - Name string - Opts []client.RollbackOption +type MarkFailedCall struct { + Release *release.Release + Reason string } type UninstallCall struct { @@ -140,9 +140,9 @@ func (c *ActionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return c.HandleUpgrade() } -func (c *ActionClient) Rollback(name string, opts ...client.RollbackOption) error { - c.Rollbacks = append(c.Rollbacks, RollbackCall{name, opts}) - return c.HandleRollback() +func (c *ActionClient) MarkFailed(rel *release.Release, reason string) error { + c.MarkFaileds = append(c.MarkFaileds, MarkFailedCall{rel, reason}) + return c.HandleMarkFailed() } func (c *ActionClient) Uninstall(name string, opts ...client.UninstallOption) (*release.UninstallReleaseResponse, error) { diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 7fa556a..038d431 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -78,7 +78,7 @@ type Reconciler struct { skipDependentWatches bool maxConcurrentReconciles int reconcilePeriod time.Duration - autoRollbackAfter time.Duration + markFailedAfter time.Duration maxHistory int annotSetupOnce sync.Once @@ -305,14 +305,14 @@ func WithMaxReleaseHistory(maxHistory int) Option { } } -// WithAutoRollbackAfter specifies the duration after which the reconciler will attempt to automatically -// roll back a release that is in a pending (locked) state. -func WithAutoRollbackAfter(duration time.Duration) Option { +// WithMarkFailedAfter specifies the duration after which the reconciler will mark a release in a pending (locked) +// state as false in order to allow rolling forward. +func WithMarkFailedAfter(duration time.Duration) Option { return func(r *Reconciler) error { if duration < 0 { return errors.New("auto-rollback after duration must not be negative") } - r.autoRollbackAfter = duration + r.markFailedAfter = duration return nil } } @@ -788,37 +788,22 @@ func (r *Reconciler) handlePending(actionClient helmclient.ActionInterface, rel } func (r *Reconciler) doHandlePending(actionClient helmclient.ActionInterface, rel *release.Release, log logr.Logger) error { - if r.autoRollbackAfter <= 0 { + if r.markFailedAfter <= 0 { return errors.New("Release is in a pending (locked) state and cannot be modified. User intervention is required.") } if rel.Info == nil || rel.Info.LastDeployed.IsZero() { return errors.New("Release is in a pending (locked) state and lacks 'last deployed' timestamp. User intervention is required.") } - if pendingSince := time.Since(rel.Info.LastDeployed.Time); pendingSince < r.autoRollbackAfter { - return fmt.Errorf("Release is in a pending (locked) state and cannot currently be modified. Rollback will be attempted in %v.", r.autoRollbackAfter-pendingSince) + if pendingSince := time.Since(rel.Info.LastDeployed.Time); pendingSince < r.markFailedAfter { + return fmt.Errorf("Release is in a pending (locked) state and cannot currently be modified. Release will be marked failed to allow a roll-forward in %v.", r.markFailedAfter-pendingSince) } - var fixAction string - var fixErr error - if rel.Info.Status == release.StatusPendingInstall { - log.Info("Attempting uninstall for locked release", "releaseName", rel.Name) - fixAction = "uninstall" - _, fixErr = actionClient.Uninstall(rel.Name, func(u *action.Uninstall) error { - u.KeepHistory = true - return nil - }) - } else { - log.Info("Attempting rollback for locked release", "releaseName", rel.Name) - fixAction = "rollback" - fixErr = actionClient.Rollback(rel.Name, func(r *action.Rollback) error { - r.Force = true - return nil - }) - } - if fixErr != nil { - return fmt.Errorf("Release is in a pending (locked) state. An attempted %s failed: %w", fixAction, fixErr) + log.Info("Marking release as failed", "releaseName", rel.Name) + err := actionClient.MarkFailed(rel, fmt.Sprintf("operator marked pending (locked) release as failed after state did not change for %v", r.markFailedAfter)) + if err != nil { + return fmt.Errorf("Failed to mark pending (locked) release as failed: %w", err) } - return fmt.Errorf("Release was in a pending (locked) state. A %s was performed to allow the next reconciliation to succeed.", fixAction) + return nil } func (r *Reconciler) reportOverrideEvents(obj runtime.Object) { From 4674a18709d1f9f65c31baa9031e6a67a02f0744 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Mon, 28 Jun 2021 22:40:58 +0200 Subject: [PATCH 7/8] always return error --- pkg/reconciler/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 038d431..a8fb6df 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -803,7 +803,7 @@ func (r *Reconciler) doHandlePending(actionClient helmclient.ActionInterface, re if err != nil { return fmt.Errorf("Failed to mark pending (locked) release as failed: %w", err) } - return nil + return fmt.Errorf("marked release %s as failed to allow upgrade to succeed in next reconcile attempt", rel.Name) } func (r *Reconciler) reportOverrideEvents(obj runtime.Object) { From 6addcb0b327320aeaec6ed1177f875cad532a545 Mon Sep 17 00:00:00 2001 From: Malte Isberner Date: Mon, 28 Jun 2021 22:46:38 +0200 Subject: [PATCH 8/8] reconciler --- pkg/client/actionclient.go | 11 ----------- pkg/reconciler/reconciler.go | 4 ++-- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index 9ac19e5..c7a5d9a 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -70,7 +70,6 @@ type ActionInterface interface { type GetOption func(*action.Get) error type InstallOption func(*action.Install) error type UpgradeOption func(*action.Upgrade) error -type RollbackOption func(*action.Rollback) error type UninstallOption func(*action.Uninstall) error func NewActionClientGetter(acg ActionConfigGetter) ActionClientGetter { @@ -182,16 +181,6 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return rel, nil } -func (c *actionClient) Rollback(name string, opts ...RollbackOption) error { - rollback := action.NewRollback(c.conf) - for _, o := range opts { - if err := o(rollback); err != nil { - return err - } - } - return rollback.Run(name) -} - func (c *actionClient) MarkFailed(rel *release.Release, reason string) error { infoCopy := *rel.Info releaseCopy := *rel diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index a8fb6df..bedde99 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -525,7 +525,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. // We also make sure not to return any errors we encounter so // we can still attempt an uninstall if the CR is being deleted. rel, err := actionClient.Get(obj.GetName()) - if errors.Is(err, driver.ErrReleaseNotFound) || (rel != nil && rel.Info != nil && rel.Info.Status == release.StatusUninstalled) { + if errors.Is(err, driver.ErrReleaseNotFound) { u.UpdateStatus(updater.EnsureCondition(conditions.Deployed(corev1.ConditionFalse, "", ""))) } else if err == nil { ensureDeployedRelease(&u, rel) @@ -692,7 +692,7 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta return nil, stateError, err } - if errors.Is(err, driver.ErrReleaseNotFound) || (currentRelease != nil && currentRelease.Info != nil && currentRelease.Info.Status == release.StatusUninstalled) { + if errors.Is(err, driver.ErrReleaseNotFound) { return nil, stateNeedsInstall, nil }