Allow marking releases stuck in a pending state as failed

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 {

pkg/reconciler/internal/fake/actionclient.go

@@ -48,17 +48,19 @@ func (hcg *fakeActionClientGetter) ActionClientFor(_ crclient.Object) (client.Ac
 }
 
 type ActionClient struct {
- Gets []GetCall
- Installs []InstallCall
- Upgrades []UpgradeCall
- Uninstalls []UninstallCall
- Reconciles []ReconcileCall
-
- HandleGet func() (*release.Release, error)
- HandleInstall func() (*release.Release, error)
- HandleUpgrade func() (*release.Release, error)
- HandleUninstall func() (*release.UninstallReleaseResponse, error)
- HandleReconcile func() error
+ 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)
+ HandleMarkFailed func() error
+ HandleUninstall func() (*release.UninstallReleaseResponse, error)
+ HandleReconcile func() error
 }
 
 func NewActionClient() ActionClient {
@@ -72,17 +74,19 @@ func NewActionClient() ActionClient {
  return func() error { return err }
  }
  return ActionClient{
- Gets: make([]GetCall, 0),
- Installs: make([]InstallCall, 0),
- Upgrades: make([]UpgradeCall, 0),
- Uninstalls: make([]UninstallCall, 0),
- Reconciles: make([]ReconcileCall, 0),
-
- HandleGet: relFunc(errors.New("get not implemented")),
- HandleInstall: relFunc(errors.New("install not implemented")),
- HandleUpgrade: relFunc(errors.New("upgrade not implemented")),
- HandleUninstall: uninstFunc(errors.New("uninstall not implemented")),
- HandleReconcile: recFunc(errors.New("reconcile not implemented")),
+ Gets: make([]GetCall, 0),
+ Installs: make([]InstallCall, 0),
+ Upgrades: make([]UpgradeCall, 0),
+ Uninstalls: make([]UninstallCall, 0),
+ Reconciles: make([]ReconcileCall, 0),
+ MarkFaileds: make([]MarkFailedCall, 0),
+
+ HandleGet: relFunc(errors.New("get not implemented")),
+ HandleInstall: relFunc(errors.New("install not implemented")),
+ HandleUpgrade: relFunc(errors.New("upgrade not implemented")),
+ HandleUninstall: uninstFunc(errors.New("uninstall not implemented")),
+ HandleReconcile: recFunc(errors.New("reconcile not implemented")),
+ HandleMarkFailed: recFunc(errors.New("mark failed not implemented")),
  }
 }
 
@@ -109,6 +113,11 @@ type UpgradeCall struct {
  Opts []client.UpgradeOption
 }
 
+type MarkFailedCall struct {
+ Release *release.Release
+ Reason string
+}
+
 type UninstallCall struct {
  Name string
  Opts []client.UninstallOption

pkg/reconciler/reconciler.go

@@ -61,13 +61,14 @@ const uninstallFinalizer = "uninstall-helm-release"
 
 // Reconciler reconciles a Helm object
 type Reconciler struct {
- client client.Client
- actionClientGetter helmclient.ActionClientGetter
- valueTranslator values.Translator
- valueMapper values.Mapper // nolint:staticcheck
- eventRecorder record.EventRecorder
- preHooks []hook.PreHook
- postHooks []hook.PostHook
+ client client.Client
+ actionClientGetter helmclient.ActionClientGetter
+ actionConfigGetter helmclient.ActionConfigGetter
+ valueTranslator values.Translator
+ valueMapper values.Mapper // nolint:staticcheck
+ eventRecorder record.EventRecorder
+ preHooks []hook.PreHook
+ postHooks []hook.PostHook
 
  log logr.Logger
  gvk *schema.GroupVersionKind
@@ -77,6 +78,7 @@ type Reconciler struct {
  skipDependentWatches bool
  maxConcurrentReconciles int
  reconcilePeriod time.Duration
+ markFailedAfter time.Duration
  maxHistory int
 
  annotSetupOnce sync.Once
@@ -175,6 +177,16 @@ func WithActionClientGetter(actionClientGetter helmclient.ActionClientGetter) Op
  }
 }
 
+// WithActionConfigGetter is an Option that configures a Reconciler's ActionConfigGetter
+//
+// A default ActionConfigGetter is used if this option is not configured.
+func WithActionConfigGetter(actionConfigGetter helmclient.ActionConfigGetter) Option {
+ return func(r *Reconciler) error {
+ r.actionConfigGetter = actionConfigGetter
+ return nil
+ }
+}
+
 // WithEventRecorder is an Option that configures a Reconciler's EventRecorder.
 //
 // By default, manager.GetEventRecorderFor() is used if this option is not
@@ -297,6 +309,18 @@ func WithMaxReleaseHistory(maxHistory int) 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.markFailedAfter = 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.
@@ -531,6 +555,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.
  )
  return ctrl.Result{}, err
  }
+ if state == statePending {
+ return r.handlePending(obj, rel, &u, log)
+ }
+
  u.UpdateStatus(updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", "")))
 
  for _, h := range r.preHooks {
@@ -597,6 +625,7 @@ const (
  stateNeedsInstall helmReleaseState = "needs install"
  stateNeedsUpgrade helmReleaseState = "needs upgrade"
  stateUnchanged helmReleaseState = "unchanged"
+ statePending helmReleaseState = "pending"
  stateError helmReleaseState = "error"
 )
 
@@ -645,6 +674,10 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta
  return nil, stateNeedsInstall, nil
  }
 
+ if currentRelease.Info != nil && currentRelease.Info.Status.IsPending() {
+ return currentRelease, statePending, nil
+ }
+
  var opts []helmclient.UpgradeOption
  if r.maxHistory > 0 {
  opts = append(opts, func(u *action.Upgrade) error {
@@ -722,6 +755,48 @@ func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updat
  return rel, nil
 }
 
+func (r *Reconciler) handlePending(obj *unstructured.Unstructured, rel *release.Release, u *updater.Updater, log logr.Logger) (ctrl.Result, error) {
+ err := r.doHandlePending(obj, 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(obj *unstructured.Unstructured, rel *release.Release, log logr.Logger) error {
+ 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.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)
+ }
+
+ log.Info("Marking release as failed", "releaseName", rel.Name)
+ err := r.markReleaseFailed(obj, 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("marked release %s as failed to allow upgrade to succeed in next reconcile attempt", rel.Name)
+}
+
+func (r *Reconciler) markReleaseFailed(obj *unstructured.Unstructured, rel *release.Release, reason string) error {
+ infoCopy := *rel.Info
+ releaseCopy := *rel
+ releaseCopy.Info = &infoCopy
+ releaseCopy.SetStatus(release.StatusFailed, reason)
+
+ actionConfig, err := r.actionConfigGetter.ActionConfigFor(obj)
+ if err != nil {
+ return err
+ }
+ return actionConfig.Releases.Update(&releaseCopy)
+}
+
 func (r *Reconciler) reportOverrideEvents(obj runtime.Object) {
  for k, v := range r.overrideValues {
  r.eventRecorder.Eventf(obj, "Warning", "ValueOverridden",
@@ -796,8 +871,8 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) {
  r.log = ctrl.Log.WithName("controllers").WithName("Helm")
  }
  if r.actionClientGetter == nil {
- actionConfigGetter := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), r.log)
- r.actionClientGetter = helmclient.NewActionClientGetter(actionConfigGetter)
+ r.actionConfigGetter = helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), r.log)
+ r.actionClientGetter = helmclient.NewActionClientGetter(r.actionConfigGetter)
  }
  if r.eventRecorder == nil {
  r.eventRecorder = mgr.GetEventRecorderFor(controllerName)

pkg/reconciler/reconciler_test.go

@@ -58,6 +58,7 @@ import (
  helmfake "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/fake"
  "github.com/operator-framework/helm-operator-plugins/pkg/sdk/controllerutil"
  "github.com/operator-framework/helm-operator-plugins/pkg/values"
+ helmTime "helm.sh/helm/v3/pkg/time"
 )
 
 var _ = Describe("Reconciler", func() {
@@ -382,6 +383,12 @@ var _ = Describe("Reconciler", func() {
  Expect(r.valueTranslator.Translate(context.Background(), &unstructured.Unstructured{})).To(Equal(chartutil.Values{"translated": true}))
  })
  })
+ var _ = Describe("WithMarkedFailAfter", func() {
+ It("should set the reconciler mark failed after duration", func() {
+ Expect(WithMarkFailedAfter(1 * time.Minute)(r)).To(Succeed())
+ Expect(r.markFailedAfter).To(Equal(1 * time.Minute))
+ })
+ })
  })
 
  var _ = Describe("Reconcile", func() {
@@ -474,6 +481,63 @@ var _ = Describe("Reconciler", func() {
  Expect(mgr.GetClient().Create(ctx, obj)).To(Succeed())
  })
 
+ When("release is in pending state", func() {
+ fakeClient := helmfake.NewActionClient()
+ deployTime := helmTime.Now().Add(-5 * time.Minute)
+ exampleRelease := &release.Release{
+ Name: "example-release",
+ Info: &release.Info{
+ Status: release.StatusPendingUpgrade,
+ LastDeployed: deployTime,
+ FirstDeployed: deployTime,
+ },
+ }
+
+ BeforeEach(func() {
+ r.actionClientGetter = helmclient.ActionClientGetterFunc(func(object client.Object) (helmclient.ActionInterface, error) {
+ fakeClient.HandleGet = func() (*release.Release, error) {
+ return exampleRelease, nil
+ }
+ return &fakeClient, nil
+ })
+ //TODO: add actionConfigGetter fake
+ //r.actionConfigGetter =
+ })
+ AfterEach(func() {
+ r.actionClientGetter = nil
+ })
+
+ When("time elapsed since last deployment exceeds markFailedAfter duration", func() {
+ It("should be marked as failed", func() {
+ r.markFailedAfter = 1 * time.Minute
+ res, err := r.Reconcile(ctx, req)
+ Expect(res).To(Equal(reconcile.Result{}))
+ Expect(err).ToNot(BeNil())
+ Expect(err).To(MatchError("marked release example-release as failed to allow upgrade to succeed in next reconcile attempt"))
+ })
+ })
+
+ When("markFailedAfter is disabled", func() {
+ It("should require user intervention", func() {
+ r.markFailedAfter = 0
+ res, err := r.Reconcile(ctx, req)
+ Expect(res).To(Equal(reconcile.Result{}))
+ Expect(err).ToNot(BeNil())
+ Expect(err).To(MatchError("Release is in a pending (locked) state and cannot be modified. User intervention is required."))
+ })
+ })
+
+ When("time since last deployment is higher than markFiledAfter duration", func() {
+ It("should return duration until the release will be marked as failed", func() {
+ r.markFailedAfter = 10 * time.Minute
+ res, err := r.Reconcile(ctx, req)
+ Expect(res).To(Equal(reconcile.Result{}))
+ Expect(err).ToNot(BeNil())
+ Expect(err.Error()).Should(ContainSubstring("Release is in a pending (locked) state and cannot currently be modified. Release will be marked failed to allow a roll-forward in"))
+ })
+ })
+ })
+
  When("requested CR release is not present", func() {
  When("action client getter is not working", func() {
  It("returns an error getting the action client", func() {