ROX-12219: Add support for pause-reconcile annotation (#29)

Co-authored-by: Marcin Owsiany <porridge@redhat.com>

pkg/reconciler/internal/conditions/conditions.go

@@ -29,10 +29,12 @@ const (
 	TypeDeployed       = "Deployed"
 	TypeReleaseFailed  = "ReleaseFailed"
 	TypeIrreconcilable = "Irreconcilable"
+	TypePaused         = "Paused"
 
-	ReasonInstallSuccessful   = status.ConditionReason("InstallSuccessful")
-	ReasonUpgradeSuccessful   = status.ConditionReason("UpgradeSuccessful")
-	ReasonUninstallSuccessful = status.ConditionReason("UninstallSuccessful")
+	ReasonInstallSuccessful            = status.ConditionReason("InstallSuccessful")
+	ReasonUpgradeSuccessful            = status.ConditionReason("UpgradeSuccessful")
+	ReasonUninstallSuccessful          = status.ConditionReason("UninstallSuccessful")
+	ReasonPauseReconcileAnnotationTrue = status.ConditionReason("PauseReconcileAnnotationTrue")
 
 	ReasonErrorGettingClient       = status.ConditionReason("ErrorGettingClient")
 	ReasonErrorGettingValues       = status.ConditionReason("ErrorGettingValues")
@@ -60,6 +62,10 @@ func Irreconcilable(stat corev1.ConditionStatus, reason status.ConditionReason,
 	return newCondition(TypeIrreconcilable, stat, reason, message)
 }
 
+func Paused(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition {
+	return newCondition(TypePaused, stat, reason, message)
+}
+
 func newCondition(t status.ConditionType, s corev1.ConditionStatus, r status.ConditionReason, m interface{}) status.Condition {
 	message := fmt.Sprintf("%s", m)
 	return status.Condition{

pkg/reconciler/internal/updater/updater.go

@@ -142,6 +142,12 @@ func EnsureConditionUnknown(t status.ConditionType) UpdateStatusFunc {
 	}
 }
 
+func EnsureConditionAbsent(t status.ConditionType) UpdateStatusFunc {
+	return func(status *helmAppStatus) bool {
+		return status.Conditions.RemoveCondition(t)
+	}
+}
+
 func EnsureDeployedRelease(rel *release.Release) UpdateStatusFunc {
 	return func(status *helmAppStatus) bool {
 		newRel := helmAppReleaseFor(rel)

pkg/reconciler/reconciler.go

@@ -83,17 +83,18 @@ type Reconciler struct {
 	extraWatches                     []watchDescription
 	maxConcurrentReconciles          int
 	reconcilePeriod                  time.Duration
-	markFailedAfter                  time.Duration
+	markFailedAfter         		 time.Duration
 	maxHistory                       int
 	skipPrimaryGVKSchemeRegistration bool
 
 	stripManifestFromStatus bool
 
-	annotSetupOnce       sync.Once
-	annotations          map[string]struct{}
-	installAnnotations   map[string]annotation.Install
-	upgradeAnnotations   map[string]annotation.Upgrade
-	uninstallAnnotations map[string]annotation.Uninstall
+	annotSetupOnce           sync.Once
+	annotations              map[string]struct{}
+	installAnnotations       map[string]annotation.Install
+	upgradeAnnotations       map[string]annotation.Upgrade
+	uninstallAnnotations     map[string]annotation.Uninstall
+	pauseReconcileAnnotation string
 }
 
 type watchDescription struct {
@@ -450,6 +451,18 @@ func WithUninstallAnnotations(as ...annotation.Uninstall) Option {
 	}
 }
 
+// WithPauseReconcileAnnotation is an Option that sets
+// a PauseReconcile annotation. If the Custom Resource watched by this
+// reconciler has the given annotation, and its value is set to `true`,
+// then reconciliation for this CR will not be performed until this annotation
+// is removed.
+func WithPauseReconcileAnnotation(annotationName string) Option {
+	return func(r *Reconciler) error {
+		r.pauseReconcileAnnotation = annotationName
+		return nil
+	}
+}
+
 // WithPreHook is an Option that configures the reconciler to run the given
 // PreHook just before performing any actions (e.g. install, upgrade, uninstall,
 // or reconciliation).
@@ -604,6 +617,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.
 		return ctrl.Result{}, err
 	}
 
+	// The finalizer must be present on the CR before we can do anything. Otherwise, if the reconciliation fails,
+	// there might be resources  created by the chart that will not be garbage-collected
+	// (cluster-scoped resources or resources in other namespaces, which are not bound by an owner reference).
+	// This is a safety measure to ensure that the chart is fully uninstalled before the CR is deleted.
+	if obj.GetDeletionTimestamp() == nil && !controllerutil.ContainsFinalizer(obj, uninstallFinalizer) {
+		log.V(1).Info("Adding uninstall finalizer")
+		obj.SetFinalizers(append(obj.GetFinalizers(), uninstallFinalizer))
+		if err := r.client.Update(ctx, obj); err != nil {
+			log.Error(err, "Failed to add uninstall finalizer")
+			return ctrl.Result{}, err
+		}
+	}
+
 	u := updater.New(r.client)
 	defer func() {
 		applyErr := u.Apply(ctx, obj)
@@ -612,6 +638,31 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.
 		}
 	}()
 
+	if r.pauseReconcileAnnotation != "" {
+		if v, ok := obj.GetAnnotations()[r.pauseReconcileAnnotation]; ok {
+			if v == "true" {
+				log.Info(fmt.Sprintf("Resource has '%s' annotation set to 'true', reconcile paused.", r.pauseReconcileAnnotation))
+				u.UpdateStatus(
+					updater.EnsureCondition(conditions.Paused(corev1.ConditionTrue, conditions.ReasonPauseReconcileAnnotationTrue, "")),
+					updater.EnsureConditionUnknown(conditions.TypeIrreconcilable),
+					updater.EnsureConditionUnknown(conditions.TypeDeployed),
+					updater.EnsureConditionUnknown(conditions.TypeInitialized),
+					updater.EnsureConditionUnknown(conditions.TypeReleaseFailed),
+					updater.EnsureDeployedRelease(nil),
+				)
+				return ctrl.Result{}, nil
+			}
+		}
+	}
+
+	u.UpdateStatus(
+		// TODO(ROX-12637): change to updater.EnsureCondition(conditions.Paused(corev1.ConditionFalse, "", "")))
+		// once stackrox operator with pause support is released.
+		// At that time also add `Paused` to the list of conditions expected in stackrox operator e2e tests.
+		// Otherwise, the number of conditions in the `status.conditions` list will vary depending on the version
+		// of used operator, which is cumbersome due to https://github.com/kudobuilder/kuttl/issues/76
+		updater.EnsureConditionAbsent(conditions.TypePaused))
+
 	actionClient, err := r.actionClientGetter.ActionClientFor(obj)
 	if err != nil {
 		u.UpdateStatus(
@@ -621,8 +672,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.
 			updater.EnsureConditionUnknown(conditions.TypeReleaseFailed),
 			updater.EnsureDeployedRelease(nil),
 		)
-		// NOTE: If obj has the uninstall finalizer, that means a release WAS deployed at some point
-		//   in the past, but we don't know if it still is because we don't have an actionClient to check.
+		// NOTE: If obj has the uninstall finalizer, that doesn't mean a release was deployed at some point in the past.
+		//   This just means the CR was accepted by the reconcile method. The presence of the finalizer is required
+		//   to deploy any resources.
 		//   So the question is, what do we do with the finalizer? We could:
 		//      - Leave it in place. This would make the CR impossible to delete without either resolving this error, or
 		//        manually uninstalling the release, deleting the finalizer, and deleting the CR.
@@ -1117,8 +1169,6 @@ func (r *Reconciler) ensureDeployedRelease(u *updater.Updater, rel *release.Rele
 		relCopy.Manifest = ""
 		rel = &relCopy
 	}
-
-	u.Update(updater.EnsureFinalizer(uninstallFinalizer))
 	u.UpdateStatus(
 		updater.EnsureCondition(conditions.Deployed(corev1.ConditionTrue, reason, message)),
 		updater.EnsureDeployedRelease(rel),

pkg/reconciler/reconciler_test.go

@@ -382,6 +382,13 @@ var _ = Describe("Reconciler", func() {
 				}))
 			})
 		})
+		var _ = Describe("WithPauseReconcileAnnotation", func() {
+			It("should set the pauseReconcileAnnotation field to the annotation name", func() {
+				a := "my.domain/pause-reconcile"
+				Expect(WithPauseReconcileAnnotation(a)(r)).To(Succeed())
+				Expect(r.pauseReconcileAnnotation).To(Equal(a))
+			})
+		})
 		var _ = Describe("WithPreHook", func() {
 			It("should set a reconciler prehook", func() {
 				called := false
@@ -524,6 +531,7 @@ var _ = Describe("Reconciler", func() {
 						WithInstallAnnotations(annotation.InstallDescription{}),
 						WithUpgradeAnnotations(annotation.UpgradeDescription{}),
 						WithUninstallAnnotations(annotation.UninstallDescription{}),
+						WithPauseReconcileAnnotation("my.domain/pause-reconcile"),
 						WithOverrideValues(map[string]string{
 							"image.repository": "custom-nginx",
 						}),
@@ -538,6 +546,7 @@ var _ = Describe("Reconciler", func() {
 						WithInstallAnnotations(annotation.InstallDescription{}),
 						WithUpgradeAnnotations(annotation.UpgradeDescription{}),
 						WithUninstallAnnotations(annotation.UninstallDescription{}),
+						WithPauseReconcileAnnotation("my.domain/pause-reconcile"),
 						WithOverrideValues(map[string]string{
 							"image.repository": "custom-nginx",
 						}),
@@ -614,8 +623,8 @@ var _ = Describe("Reconciler", func() {
 								Expect(c.Message).To(Equal(acgErr.Error()))
 							})
 
-							By("verifying the uninstall finalizer is not present on the CR", func() {
-								Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeFalse())
+							By("verifying the uninstall finalizer is present on the CR", func() {
+								Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeTrue())
 							})
 						})
 						It("returns an error getting the release", func() {
@@ -651,8 +660,8 @@ var _ = Describe("Reconciler", func() {
 								Expect(c.Message).To(Equal("get not implemented"))
 							})
 
-							By("verifying the uninstall finalizer is not present on the CR", func() {
-								Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeFalse())
+							By("verifying the uninstall finalizer is present on the CR", func() {
+								Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeTrue())
 							})
 						})
 					})
@@ -687,8 +696,8 @@ var _ = Describe("Reconciler", func() {
 								Expect(c.Message).To(ContainSubstring("error parsing index"))
 							})
 
-							By("verifying the uninstall finalizer is not present on the CR", func() {
-								Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeFalse())
+							By("verifying the uninstall finalizer is present on the CR", func() {
+								Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeTrue())
 							})
 						})
 					})
@@ -754,8 +763,8 @@ var _ = Describe("Reconciler", func() {
 									Expect(c.Message).To(ContainSubstring("install failed: foobar"))
 								})
 
-								By("ensuring the uninstall finalizer is not present on the CR", func() {
-									Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeFalse())
+								By("ensuring the uninstall finalizer is present on the CR", func() {
+									Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeTrue())
 								})
 							})
 						})
@@ -1316,6 +1325,64 @@ var _ = Describe("Reconciler", func() {
 									verifyNoRelease(ctx, mgr.GetClient(), obj.GetNamespace(), obj.GetName(), currentRelease)
 								})
 
+								By("ensuring the finalizer is removed and the CR is deleted", func() {
+									err := mgr.GetAPIReader().Get(ctx, objKey, obj)
+									Expect(apierrors.IsNotFound(err)).To(BeTrue())
+								})
+							})
+						})
+						When("pause-reconcile annotation is present", func() {
+							It("pauses reconciliation", func() {
+								By("adding the pause-reconcile annotation to the CR", func() {
+									Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed())
+									obj.SetAnnotations(map[string]string{"my.domain/pause-reconcile": "true"})
+									obj.Object["spec"] = map[string]interface{}{"replicaCount": "666"}
+									Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed())
+								})
+
+								By("deleting the CR", func() {
+									Expect(mgr.GetClient().Delete(ctx, obj)).To(Succeed())
+								})
+
+								By("successfully reconciling a request when paused", func() {
+									res, err := r.Reconcile(ctx, req)
+									Expect(res).To(Equal(reconcile.Result{}))
+									Expect(err).To(BeNil())
+								})
+
+								By("getting the CR", func() {
+									Expect(mgr.GetAPIReader().Get(ctx, objKey, obj)).To(Succeed())
+								})
+
+								By("verifying the CR status is Paused", func() {
+									objStat := &objStatus{}
+									Expect(runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, objStat)).To(Succeed())
+									Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypePaused)).To(BeTrue())
+								})
+
+								By("verifying the release has not changed", func() {
+									rel, err := ac.Get(obj.GetName())
+									Expect(err).To(BeNil())
+									Expect(rel).NotTo(BeNil())
+									Expect(*rel).To(Equal(*currentRelease))
+								})
+
+								By("removing the pause-reconcile annotation from the CR", func() {
+									Expect(mgr.GetClient().Get(ctx, objKey, obj)).To(Succeed())
+									obj.SetAnnotations(nil)
+									Expect(mgr.GetClient().Update(ctx, obj)).To(Succeed())
+								})
+
+								By("successfully reconciling a request", func() {
+									res, err := r.Reconcile(ctx, req)
+									Expect(res).To(Equal(reconcile.Result{}))
+									Expect(err).To(BeNil())
+								})
+
+								By("verifying the release is uninstalled", func() {
+									verifyNoRelease(ctx, mgr.GetClient(), obj.GetNamespace(), obj.GetName(), currentRelease)
+								})
+
 								By("ensuring the finalizer is removed and the CR is deleted", func() {
 									err := mgr.GetAPIReader().Get(ctx, objKey, obj)
 									Expect(apierrors.IsNotFound(err)).To(BeTrue())