chore: refactor sync() methods in controllers

This commit aligns all controllers to follow the same pattern when
dealing with non-existing and paused objects. It also ensures consistent
logging.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

pkg/alertmanager/operator.go

@@ -594,15 +594,16 @@ func (c *Operator) sync(ctx context.Context, key string) error {
 		return nil
 	}
 
+	logger := c.logger.With("key", key)
+	logger.Info("sync alertmanager")
+
 	if am.Spec.Paused {
+		logger.Info("no action taken (the resource is paused)")
 		return nil
 	}
 
-	logger := c.logger.With("key", key)
 	c.recordDeprecatedFields(key, logger, am)
 
-	logger.Info("sync alertmanager")
-
 	if err := operator.CheckStorageClass(ctx, c.canReadStorageClass, c.kclient, am.Spec.Storage); err != nil {
 		return err
 	}

pkg/operator/informers.go

@@ -81,16 +81,21 @@ func HasReferenceFunc(
 	}
 }
 
-// GetObjectFromKey retrieves an object from the informer cache using the provided key. It returns a nil value and no error if the object is not found.
-// The function will panic if the caller provides an informer which doesn't reference objects of type T.
+// GetObjectFromKey retrieves an object from the informer cache using the
+// provided key. It returns a nil value and no error if the object is not
+// found.
+//
+// The function will panic if the caller provides an informer which doesn't
+// reference objects of type T.
 func GetObjectFromKey[T runtime.Object](infs *informers.ForResource, key string) (T, error) {
-	obj, err := infs.Get(key)
 	var zero T
 
+	obj, err := infs.Get(key)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			return zero, nil
 		}
+
 		return zero, fmt.Errorf("failed to retrieve object from informer: %w", err)
 	}
 
@@ -98,5 +103,6 @@ func GetObjectFromKey[T runtime.Object](infs *informers.ForResource, key string)
 	if err = k8sutil.AddTypeInformationToObject(obj); err != nil {
 		return zero, err
 	}
+
 	return obj.(T), nil
 }

pkg/prometheus/agent/operator.go

@@ -619,7 +619,6 @@ func (c *Operator) Sync(ctx context.Context, key string) error {
 
 func (c *Operator) sync(ctx context.Context, key string) error {
 	p, err := operator.GetObjectFromKey[*monitoringv1alpha1.PrometheusAgent](c.promInfs, key)
-
 	if err != nil {
 		return err
 	}
@@ -631,6 +630,7 @@ func (c *Operator) sync(ctx context.Context, key string) error {
 	}
 
 	logger := c.logger.With("key", key)
+	logger.Info("sync prometheusagent")
 
 	finalizersAdded, err := c.finalizerSyncer.Sync(ctx, p, c.rr.DeletionInProgress(p), func() error { return nil })
 	if err != nil {
@@ -650,12 +650,10 @@ func (c *Operator) sync(ctx context.Context, key string) error {
 	}
 
 	if p.Spec.Paused {
-		logger.Info("the resource is paused, not reconciling")
+		logger.Info("no action taken (the resource is paused)")
 		return nil
 	}
 
-	logger.Info("sync prometheusagent")
-
 	if ptr.Deref(p.Spec.Mode, "") == monitoringv1alpha1.DaemonSetPrometheusAgentMode && !c.daemonSetFeatureGateEnabled {
 		return fmt.Errorf("feature gate for Prometheus Agent's DaemonSet mode is not enabled")
 	}

pkg/prometheus/server/operator.go

@@ -822,7 +822,6 @@ func (c *Operator) Sync(ctx context.Context, key string) error {
 
 func (c *Operator) sync(ctx context.Context, key string) error {
 	p, err := operator.GetObjectFromKey[*monitoringv1.Prometheus](c.promInfs, key)
-
 	if err != nil {
 		return err
 	}
@@ -834,19 +833,17 @@ func (c *Operator) sync(ctx context.Context, key string) error {
 	}
 
 	logger := c.logger.With("key", key)
-	c.recordDeprecatedFields(key, logger, p)
+	logger.Info("sync prometheus")
 
-	statusCleanup := func() error {
+	finalizerAdded, err := c.finalizerSyncer.Sync(ctx, p, c.rr.DeletionInProgress(p), func() error {
 		return c.configResStatusCleanup(ctx, p)
-	}
-
-	finalizerAdded, err := c.finalizerSyncer.Sync(ctx, p, c.rr.DeletionInProgress(p), statusCleanup)
+	})
 	if err != nil {
 		return err
 	}
 
 	if finalizerAdded {
-		// Since the object has been updated, let's trigger another sync.
+		// Since the finalizer has been added to the object, let's trigger another sync.
 		c.rr.EnqueueForReconciliation(p)
 		return nil
 	}
@@ -856,28 +853,20 @@ func (c *Operator) sync(ctx context.Context, key string) error {
 		return nil
 	}
 
+	if p.Spec.Paused {
+		logger.Info("no action taken (the resource is paused)")
+		return nil
+	}
+
+	c.recordDeprecatedFields(key, logger, p)
+
 	if err := operator.CheckStorageClass(ctx, c.canReadStorageClass, c.kclient, p.Spec.Storage); err != nil {
 		return err
 	}
 
-	if p.Spec.Paused {
-		logger.Info("the resource is paused, not reconciling")
-		return nil
-	}
-
-	logger.Info("sync prometheus")
-
 	assetStore := assets.NewStoreBuilder(c.kclient.CoreV1(), c.kclient.CoreV1())
 
-	opts := []prompkg.ConfigGeneratorOption{}
-	if c.endpointSliceSupported {
-		opts = append(opts, prompkg.WithEndpointSliceSupport())
-	}
-	cg, err := prompkg.NewConfigGenerator(logger, p, opts...)
-	if err != nil {
-		return err
-	}
-
+	// Select configuration resources.
 	resources, err := c.getSelectedConfigResources(ctx, logger, p, assetStore)
 	if err != nil {
 		return err
@@ -892,6 +881,15 @@ func (c *Operator) sync(ctx context.Context, key string) error {
 		return err
 	}
 
+	opts := []prompkg.ConfigGeneratorOption{}
+	if c.endpointSliceSupported {
+		opts = append(opts, prompkg.WithEndpointSliceSupport())
+	}
+	cg, err := prompkg.NewConfigGenerator(logger, p, opts...)
+	if err != nil {
+		return err
+	}
+
 	if err := c.createOrUpdateConfigurationSecret(ctx, logger, p, cg, ruleConfigMapNames, assetStore, resources); err != nil {
 		return fmt.Errorf("creating config failed: %w", err)
 	}

pkg/thanos/operator.go

@@ -473,7 +473,6 @@ func (o *Operator) Sync(ctx context.Context, key string) error {
 
 func (o *Operator) sync(ctx context.Context, key string) error {
 	tr, err := operator.GetObjectFromKey[*monitoringv1.ThanosRuler](o.thanosRulerInfs, key)
-
 	if err != nil {
 		return err
 	}
@@ -484,23 +483,18 @@ func (o *Operator) sync(ctx context.Context, key string) error {
 		return nil
 	}
 
-	if tr.Spec.Paused {
-		return nil
-	}
-
 	logger := o.logger.With("key", key)
+	logger.Info("sync thanos-ruler")
 
-	statusCleanup := func() error {
+	finalizerAdded, err := o.finalizerSyncer.Sync(ctx, tr, o.rr.DeletionInProgress(tr), func() error {
 		return o.configResStatusCleanup(ctx, tr)
-	}
-
-	finalizerAdded, err := o.finalizerSyncer.Sync(ctx, tr, o.rr.DeletionInProgress(tr), statusCleanup)
+	})
 	if err != nil {
 		return err
 	}
 
 	if finalizerAdded {
-		// Since the object has been updated, let's trigger another sync.
+		// Since the finalizer has been added to the object, let's trigger another sync.
 		o.rr.EnqueueForReconciliation(tr)
 		return nil
 	}
@@ -511,7 +505,10 @@ func (o *Operator) sync(ctx context.Context, key string) error {
 		return nil
 	}
 
-	logger.Info("sync thanos-ruler")
+	if tr.Spec.Paused {
+		logger.Info("no action taken (the resource is paused)")
+		return nil
+	}
 
 	o.recordDeprecatedFields(key, logger, tr)