From f436c1e71fb6a22b3d92a16bd8a31b4430e41a09 Mon Sep 17 00:00:00 2001 From: Simon Baeumer Date: Wed, 16 Jun 2021 10:49:32 +0200 Subject: [PATCH 1/4] X-Smart-Branch-Parent: main From aece28d206b9e97ba07dc480144227090dcadeaf Mon Sep 17 00:00:00 2001 From: Simon Baeumer Date: Wed, 16 Jun 2021 11:42:50 +0200 Subject: [PATCH 2/4] Unlock pending releases --- pkg/client/actionclient.go | 12 ++++++ .../internal/conditions/conditions.go | 11 +++++ pkg/reconciler/reconciler.go | 42 ++++++++++++++++--- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index e6315fe..c598554 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -63,12 +63,14 @@ type ActionInterface interface { 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) Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) + Rollback(name string, opts ...RollbackOption) error Reconcile(rel *release.Release) error } 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 { @@ -179,6 +181,16 @@ 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) 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..ecc431a 100644 --- a/pkg/reconciler/internal/conditions/conditions.go +++ b/pkg/reconciler/internal/conditions/conditions.go @@ -28,7 +28,9 @@ const ( TypeInitialized = "Initialized" TypeDeployed = "Deployed" TypeReleaseFailed = "ReleaseFailed" + TypeRollbackFailed = "TypeRollbackFailed" TypeIrreconcilable = "Irreconcilable" + TypeReleasePending = "ReleasePending" ReasonInstallSuccessful = status.ConditionReason("InstallSuccessful") ReasonUpgradeSuccessful = status.ConditionReason("UpgradeSuccessful") @@ -41,6 +43,7 @@ const ( ReasonUpgradeError = status.ConditionReason("UpgradeError") ReasonReconcileError = status.ConditionReason("ReconcileError") ReasonUninstallError = status.ConditionReason("UninstallError") + ReasonReleasePending = status.ConditionReason("ReleasePending") ) func Initialized(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { @@ -55,6 +58,14 @@ func ReleaseFailed(stat corev1.ConditionStatus, reason status.ConditionReason, m return newCondition(TypeReleaseFailed, stat, reason, message) } +func ReleasePending(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { + return newCondition(TypeReleasePending, stat, reason, message) +} + +func RollbackFailed(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { + return newCondition(TypeRollbackFailed, stat, reason, message) +} + func Irreconcilable(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { return newCondition(TypeIrreconcilable, stat, reason, message) } diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index aada8a3..1e7df6e 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "math/rand" "strings" "sync" "time" @@ -561,6 +562,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. return ctrl.Result{}, err } + case statedNeedsRollback: { + if err := r.doRollback(actionClient, &u, obj); err != nil { + return ctrl.Result{}, err + } + } + case stateUnchanged: if err := r.doReconcile(actionClient, &u, rel, log); err != nil { return ctrl.Result{}, err @@ -614,10 +621,11 @@ func (r *Reconciler) getValues(ctx context.Context, obj *unstructured.Unstructur type helmReleaseState string const ( - stateNeedsInstall helmReleaseState = "needs install" - stateNeedsUpgrade helmReleaseState = "needs upgrade" - stateUnchanged helmReleaseState = "unchanged" - stateError helmReleaseState = "error" + stateNeedsInstall helmReleaseState = "needs install" + stateNeedsUpgrade helmReleaseState = "needs upgrade" + stateUnchanged helmReleaseState = "unchanged" + stateError helmReleaseState = "error" + statedNeedsRollback helmReleaseState = "needs rollback" ) func (r *Reconciler) handleDeletion(ctx context.Context, actionClient helmclient.ActionInterface, obj *unstructured.Unstructured, log logr.Logger) error { @@ -665,6 +673,14 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta return nil, stateNeedsInstall, nil } + // If the release is pending it is likely that it happened because the helm install/upgrade/rollback action + // were cancelled unexpectedly. + //TODO(do-not-merge): as pending releases are now rolled back it is necessary to ensure that an in-progress helm release is not cancelled + // and this code is only executed to ensure aborted releases are rolled back. + if currentRelease.Info.Status.IsPending() { + return nil, statedNeedsRollback, nil + } + var opts []helmclient.UpgradeOption for name, annot := range r.upgradeAnnotations { if v, ok := obj.GetAnnotations()[name]; ok { @@ -679,7 +695,7 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta if err != nil { return currentRelease, stateError, err } - if specRelease.Manifest != currentRelease.Manifest || + if specRelease.Manifest != fmt.Sprintf("%d%s", rand.Int31(), currentRelease.Manifest) || currentRelease.Info.Status == release.StatusFailed || currentRelease.Info.Status == release.StatusSuperseded { return currentRelease, stateNeedsUpgrade, nil @@ -737,6 +753,22 @@ func (r *Reconciler) reportOverrideEvents(obj runtime.Object) { } } +func (r *Reconciler) doRollback(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured) error { + u.UpdateStatus( + //TODO(do-not-merge): Add better error message indicating that release was maybe aborted + updater.EnsureCondition(conditions.ReleasePending(corev1.ConditionFalse, "", "")), + ) + + if err := actionClient.Rollback(obj.GetName()); err != nil { + u.UpdateStatus( + //TODO(do-not-merge): Add better error message indicating that release was maybe aborted with manual steps + updater.EnsureCondition(conditions.RollbackFailed(corev1.ConditionFalse, "", err)), + ) + return err + } + return nil +} + func (r *Reconciler) doReconcile(actionClient helmclient.ActionInterface, u *updater.Updater, rel *release.Release, log logr.Logger) error { // If a change is made to the CR spec that causes a release failure, a // ConditionReleaseFailed is added to the status conditions. If that change From b8a76402a834d10b072f863a3d20650a6f92bf83 Mon Sep 17 00:00:00 2001 From: Simon Baeumer Date: Wed, 16 Jun 2021 14:10:41 +0200 Subject: [PATCH 3/4] Add reconcile requeue on pending state --- pkg/client/actionclient.go | 2 +- pkg/reconciler/internal/fake/actionclient.go | 12 +++++++++ pkg/reconciler/reconciler.go | 27 ++++++++++++++++---- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index c598554..9d6f944 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -183,7 +183,7 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m func (c *actionClient) Rollback(name string, opts ...RollbackOption) error { rollback := action.NewRollback(c.conf) - for _, o := range opts { + for _, o := range opts { if err := o(rollback); err != nil { return err } diff --git a/pkg/reconciler/internal/fake/actionclient.go b/pkg/reconciler/internal/fake/actionclient.go index 410c4de..1a1c6b8 100644 --- a/pkg/reconciler/internal/fake/actionclient.go +++ b/pkg/reconciler/internal/fake/actionclient.go @@ -53,12 +53,14 @@ type ActionClient struct { Upgrades []UpgradeCall Uninstalls []UninstallCall Reconciles []ReconcileCall + Rollbacks []RollbackCall HandleGet func() (*release.Release, error) HandleInstall func() (*release.Release, error) HandleUpgrade func() (*release.Release, error) HandleUninstall func() (*release.UninstallReleaseResponse, error) HandleReconcile func() error + HandleRollback func() error } func NewActionClient() ActionClient { @@ -118,6 +120,11 @@ type ReconcileCall struct { Release *release.Release } +type RollbackCall struct { + Name string + Opts []client.RollbackOption +} + func (c *ActionClient) Get(name string, opts ...client.GetOption) (*release.Release, error) { c.Gets = append(c.Gets, GetCall{name, opts}) return c.HandleGet() @@ -142,3 +149,8 @@ func (c *ActionClient) Reconcile(rel *release.Release) error { c.Reconciles = append(c.Reconciles, ReconcileCall{rel}) return c.HandleReconcile() } + +func (c *ActionClient) Rollback(name string, opts ...client.RollbackOption) error { + c.Rollbacks = append(c.Rollbacks, RollbackCall{name, opts}) + return c.HandleRollback() +} diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 1e7df6e..1e09816 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -57,7 +57,13 @@ import ( "github.com/joelanford/helm-operator/pkg/values" ) -const uninstallFinalizer = "uninstall-helm-release" +const ( + uninstallFinalizer = "uninstall-helm-release" + // pendingReleaseTimeout defines the time frame after which a rollback is performed on a given release + pendingReleaseTimeout = time.Second * 30 + // pendingRetryInterval defines the time waiting until the current reconciliation finished to avoid spamming helm actions + pendingRetryInterval = time.Second * 5 +) // Reconciler reconciles a Helm object type Reconciler struct { @@ -562,16 +568,21 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. return ctrl.Result{}, err } - case statedNeedsRollback: { - if err := r.doRollback(actionClient, &u, obj); err != nil { - return ctrl.Result{}, err + case statedNeedsRollback: + { + if err := r.doRollback(actionClient, &u, obj); err != nil { + return ctrl.Result{}, err + } } - } case stateUnchanged: if err := r.doReconcile(actionClient, &u, rel, log); err != nil { return ctrl.Result{}, err } + + case stateNeedsSkip: + return ctrl.Result{Requeue: true, RequeueAfter: pendingRetryInterval}, nil + default: return ctrl.Result{}, fmt.Errorf("unexpected release state: %s", state) } @@ -626,6 +637,7 @@ const ( stateUnchanged helmReleaseState = "unchanged" stateError helmReleaseState = "error" statedNeedsRollback helmReleaseState = "needs rollback" + stateNeedsSkip helmReleaseState = "needs skip" ) func (r *Reconciler) handleDeletion(ctx context.Context, actionClient helmclient.ActionInterface, obj *unstructured.Unstructured, log logr.Logger) error { @@ -678,6 +690,11 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta //TODO(do-not-merge): as pending releases are now rolled back it is necessary to ensure that an in-progress helm release is not cancelled // and this code is only executed to ensure aborted releases are rolled back. if currentRelease.Info.Status.IsPending() { + t := time.Now().Sub(currentRelease.Info.LastDeployed.Time) + if t <= pendingReleaseTimeout { + r.log.Info("Release pending, skipped", "name", currentRelease.Name, "version", currentRelease.Version, "retry_in", pendingRetryInterval.String()) + return nil, stateNeedsSkip, nil + } return nil, statedNeedsRollback, nil } From e3d12dbe146260a0333b8e85e7d32c814b185412 Mon Sep 17 00:00:00 2001 From: Simon Baeumer Date: Wed, 16 Jun 2021 14:21:25 +0200 Subject: [PATCH 4/4] style --- pkg/reconciler/reconciler.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 1e09816..4b30c49 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -58,7 +58,7 @@ import ( ) const ( - uninstallFinalizer = "uninstall-helm-release" + uninstallFinalizer = "uninstall-helm-release" // pendingReleaseTimeout defines the time frame after which a rollback is performed on a given release pendingReleaseTimeout = time.Second * 30 // pendingRetryInterval defines the time waiting until the current reconciliation finished to avoid spamming helm actions @@ -581,6 +581,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. } case stateNeedsSkip: + // TODO(do-not-merge): set status custom resource status return ctrl.Result{Requeue: true, RequeueAfter: pendingRetryInterval}, nil default: