From 1c07dc0b2c80d3e271c2fe41dd2808400cacd620 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 24 Feb 2025 19:02:31 +0100 Subject: [PATCH 1/6] ROX-28223: Pause-reconcile annotation Co-authored-by: Marcin Owsiany --- .../internal/conditions/conditions.go | 12 +++- pkg/reconciler/internal/updater/updater.go | 6 ++ pkg/reconciler/reconciler.go | 48 +++++++++++-- pkg/reconciler/reconciler_test.go | 67 +++++++++++++++++++ 4 files changed, 125 insertions(+), 8 deletions(-) diff --git a/pkg/reconciler/internal/conditions/conditions.go b/pkg/reconciler/internal/conditions/conditions.go index 55e2c656..45bde93d 100644 --- a/pkg/reconciler/internal/conditions/conditions.go +++ b/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") @@ -59,6 +61,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{ diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 1508c32a..b149422b 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -146,6 +146,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) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 2b6f0706..22640d41 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -84,11 +84,12 @@ type Reconciler struct { skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc - 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 } // New creates a new Reconciler that reconciles custom resources that define a @@ -439,6 +440,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). @@ -591,6 +604,31 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re } }() + 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(ctx, obj) if err != nil { u.UpdateStatus( diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 09dde645..66e4f634 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -394,6 +394,13 @@ var _ = Describe("Reconciler", func() { })) }) }) + _ = 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)) + }) + }) _ = Describe("WithPreHook", func() { It("should set a reconciler prehook", func() { called := false @@ -535,6 +542,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", }), @@ -549,6 +557,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", }), @@ -1419,6 +1428,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).ToNot(HaveOccurred()) + }) + + 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).ToNot(HaveOccurred()) + 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).ToNot(HaveOccurred()) + }) + + 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()) From e236de3773c2559e4cfe3b953269463a60be52f3 Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Tue, 25 Feb 2025 11:27:09 +0100 Subject: [PATCH 2/6] Remove Stackrox specific workaround --- pkg/reconciler/internal/updater/updater.go | 6 ------ pkg/reconciler/reconciler.go | 8 +------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index b149422b..1508c32a 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -146,12 +146,6 @@ 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) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 22640d41..1be96c91 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -621,13 +621,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re } } - 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)) + u.UpdateStatus(updater.EnsureCondition(conditions.Paused(corev1.ConditionFalse, "", ""))) actionClient, err := r.actionClientGetter.ActionClientFor(ctx, obj) if err != nil { From 6f6421a17cd53b1175d6a8aa09c5f6dbb185712b Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 3 Mar 2025 17:44:29 +0100 Subject: [PATCH 3/6] Pause reconcile handler --- pkg/reconciler/reconciler.go | 74 +++++++++++++++++++------------ pkg/reconciler/reconciler_test.go | 11 +---- 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 1be96c91..b401c15f 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -83,13 +83,13 @@ type Reconciler struct { maxReleaseHistory *int skipPrimaryGVKSchemeRegistration bool controllerSetupFuncs []ControllerSetupFunc + pauseHandler PauseReconcileHandlerFunc - annotSetupOnce sync.Once - annotations map[string]struct{} - installAnnotations map[string]annotation.Install - upgradeAnnotations map[string]annotation.Upgrade - uninstallAnnotations map[string]annotation.Uninstall - pauseReconcileAnnotation string + annotSetupOnce sync.Once + annotations map[string]struct{} + installAnnotations map[string]annotation.Install + upgradeAnnotations map[string]annotation.Upgrade + uninstallAnnotations map[string]annotation.Uninstall } // New creates a new Reconciler that reconciles custom resources that define a @@ -440,18 +440,35 @@ 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 { +// PauseReconcileHandlerFunc defines a function type that determines whether reconciliation should be paused +// for a given custom resource +type PauseReconcileHandlerFunc func(ctx context.Context, obj *unstructured.Unstructured) (bool, error) + +// WithPauseReconcileHandler is an Option that sets a PauseReconcile handler, which is a function that +// that determines whether reconciliation should be paused for the custom resource watched by this reconciler. +// +// Example usage: WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")) +func WithPauseReconcileHandler(handler PauseReconcileHandlerFunc) Option { return func(r *Reconciler) error { - r.pauseReconcileAnnotation = annotationName + r.pauseHandler = handler return nil } } +// PauseReconcileIfAnnotationTrue returns a PauseReconcileHandlerFunc that pauses reconciliation if the given +// annotation is present and set to "true" +func PauseReconcileIfAnnotationTrue(annotationName string) PauseReconcileHandlerFunc { + return func(ctx context.Context, obj *unstructured.Unstructured) (bool, error) { + if v, ok := obj.GetAnnotations()[annotationName]; ok { + if v == "true" { + return true, nil + } + } + + return false, 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,21 +621,22 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re } }() - 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 - } - } + paused, err := r.pauseHandler(ctx, obj) + if err != nil { + log.Error(err, "pause reconcile handler failed") + } + + if paused { + log.Info("Reconcile is paused for this resource.") + 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(updater.EnsureCondition(conditions.Paused(corev1.ConditionFalse, "", ""))) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 66e4f634..ab9da8c1 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -394,13 +394,6 @@ var _ = Describe("Reconciler", func() { })) }) }) - _ = 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)) - }) - }) _ = Describe("WithPreHook", func() { It("should set a reconciler prehook", func() { called := false @@ -542,7 +535,7 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), - WithPauseReconcileAnnotation("my.domain/pause-reconcile"), + WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -557,7 +550,7 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), - WithPauseReconcileAnnotation("my.domain/pause-reconcile"), + WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), From a2d43d5c05b42a4b458b19ea095aaac24f70ed1a Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 3 Mar 2025 18:34:46 +0100 Subject: [PATCH 4/6] Fix panic & tests --- pkg/reconciler/reconciler.go | 32 ++++++++++++++++--------------- pkg/reconciler/reconciler_test.go | 7 +++++-- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index b401c15f..9ce4056a 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -621,22 +621,24 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re } }() - paused, err := r.pauseHandler(ctx, obj) - if err != nil { - log.Error(err, "pause reconcile handler failed") - } + if r.pauseHandler != nil { + paused, err := r.pauseHandler(ctx, obj) + if err != nil { + log.Error(err, "pause reconcile handler failed") + } - if paused { - log.Info("Reconcile is paused for this resource.") - 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 + if paused { + log.Info("Reconcile is paused for this resource.") + 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(updater.EnsureCondition(conditions.Paused(corev1.ConditionFalse, "", ""))) diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index ab9da8c1..cc8b315d 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -535,7 +535,6 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), - WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -550,7 +549,6 @@ var _ = Describe("Reconciler", func() { WithInstallAnnotations(annotation.InstallDescription{}), WithUpgradeAnnotations(annotation.UpgradeDescription{}), WithUninstallAnnotations(annotation.UninstallDescription{}), - WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")), WithOverrideValues(map[string]string{ "image.repository": "custom-nginx", }), @@ -1429,6 +1427,11 @@ var _ = Describe("Reconciler", func() { }) When("pause-reconcile annotation is present", func() { It("pauses reconciliation", func() { + By("adding a pause-reconcile handler to the Reconciler", func() { + pauseHandler := WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")) + pauseHandler(r) + }) + 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"}) From d88b1c387e2f50300014aaadb22b6f87867eb15b Mon Sep 17 00:00:00 2001 From: Vlad Bologa Date: Mon, 3 Mar 2025 20:02:20 +0100 Subject: [PATCH 5/6] Code review suggestions --- pkg/reconciler/reconciler.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 9ce4056a..54631abf 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -445,7 +445,7 @@ func WithUninstallAnnotations(as ...annotation.Uninstall) Option { type PauseReconcileHandlerFunc func(ctx context.Context, obj *unstructured.Unstructured) (bool, error) // WithPauseReconcileHandler is an Option that sets a PauseReconcile handler, which is a function that -// that determines whether reconciliation should be paused for the custom resource watched by this reconciler. +// determines whether reconciliation should be paused for the custom resource watched by this reconciler. // // Example usage: WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")) func WithPauseReconcileHandler(handler PauseReconcileHandlerFunc) Option { @@ -459,10 +459,8 @@ func WithPauseReconcileHandler(handler PauseReconcileHandlerFunc) Option { // annotation is present and set to "true" func PauseReconcileIfAnnotationTrue(annotationName string) PauseReconcileHandlerFunc { return func(ctx context.Context, obj *unstructured.Unstructured) (bool, error) { - if v, ok := obj.GetAnnotations()[annotationName]; ok { - if v == "true" { - return true, nil - } + if v, ok := obj.GetAnnotations()[annotationName]; ok && v == "true" { + return true, nil } return false, nil From 195e0cbd5fae3fa646b3cf0cb61da82c99b3198a Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 13 Mar 2025 08:46:31 +0100 Subject: [PATCH 6/6] Fix lint issues Signed-off-by: Per Goncalves da Silva --- pkg/reconciler/reconciler.go | 2 +- pkg/reconciler/reconciler_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 54631abf..56a9711d 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -458,7 +458,7 @@ func WithPauseReconcileHandler(handler PauseReconcileHandlerFunc) Option { // PauseReconcileIfAnnotationTrue returns a PauseReconcileHandlerFunc that pauses reconciliation if the given // annotation is present and set to "true" func PauseReconcileIfAnnotationTrue(annotationName string) PauseReconcileHandlerFunc { - return func(ctx context.Context, obj *unstructured.Unstructured) (bool, error) { + return func(_ context.Context, obj *unstructured.Unstructured) (bool, error) { if v, ok := obj.GetAnnotations()[annotationName]; ok && v == "true" { return true, nil } diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index cc8b315d..86d00123 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -1429,7 +1429,7 @@ var _ = Describe("Reconciler", func() { It("pauses reconciliation", func() { By("adding a pause-reconcile handler to the Reconciler", func() { pauseHandler := WithPauseReconcileHandler(PauseReconcileIfAnnotationTrue("my.domain/pause-reconcile")) - pauseHandler(r) + Expect(pauseHandler(r)).To(Succeed()) }) By("adding the pause-reconcile annotation to the CR", func() {