From fbb977a574b51b0d70abb89967a5e11038bb23eb Mon Sep 17 00:00:00 2001
From: Mathieu Cesbron <mathieu.cesbron@protonmail.com>
Date: Sun, 25 Feb 2024 11:40:35 +0100
Subject: [PATCH] chore(controller): Make controller logic consistent

Signed-off-by: Mathieu Cesbron <mathieu.cesbron@protonmail.com>
---
 .../certificate_lifecycle_controller.go       | 29 ++++++++++---------
 controllers/datastore_controller.go           | 14 ++++-----
 controllers/tenantcontrolplane_controller.go  | 17 +++++------
 3 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/controllers/certificate_lifecycle_controller.go b/controllers/certificate_lifecycle_controller.go
index 9e18c9cd..807227e5 100644
--- a/controllers/certificate_lifecycle_controller.go
+++ b/controllers/certificate_lifecycle_controller.go
@@ -35,28 +35,29 @@ type CertificateLifecycle struct {
 }
 
 func (s *CertificateLifecycle) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
-	logger := log.FromContext(ctx)
+	log := log.FromContext(ctx)
 
-	logger.Info("starting CertificateLifecycle handling")
+	log.Info("starting CertificateLifecycle handling")
 
 	secret := corev1.Secret{}
-	if err := s.client.Get(ctx, request.NamespacedName, &secret); err != nil {
-		if k8serrors.IsNotFound(err) {
-			logger.Info("resource may have been deleted, skipping")
-
-			return reconcile.Result{}, nil
-		}
+	err := s.client.Get(ctx, request.NamespacedName, &secret)
+	if k8serrors.IsNotFound(err) {
+		log.Info("resource have been deleted, skipping")
+		return reconcile.Result{}, nil
+	}
+	if err != nil {
+		log.Error(err, "cannot retrieve the required resource")
+		return reconcile.Result{}, err
 	}
 
 	checkType, ok := secret.GetLabels()[constants.ControllerLabelResource]
 	if !ok {
-		logger.Info("missing controller label, shouldn't happen")
+		log.Info("missing controller label, shouldn't happen")
 
 		return reconcile.Result{}, nil
 	}
 
 	var crt *x509.Certificate
-	var err error
 
 	switch checkType {
 	case "x509":
@@ -68,7 +69,7 @@ func (s *CertificateLifecycle) Reconcile(ctx context.Context, request reconcile.
 	}
 
 	if err != nil {
-		logger.Error(err, "skipping reconciliation")
+		log.Error(err, "skipping reconciliation")
 
 		return reconcile.Result{}, nil
 	}
@@ -76,7 +77,7 @@ func (s *CertificateLifecycle) Reconcile(ctx context.Context, request reconcile.
 	deadline := time.Now().AddDate(0, 0, 1)
 
 	if deadline.After(crt.NotAfter) {
-		logger.Info("certificate near expiration, must be rotated")
+		log.Info("certificate near expiration, must be rotated")
 
 		s.Channel <- event.GenericEvent{Object: &kamajiv1alpha1.TenantControlPlane{
 			ObjectMeta: metav1.ObjectMeta{
@@ -85,14 +86,14 @@ func (s *CertificateLifecycle) Reconcile(ctx context.Context, request reconcile.
 			},
 		}}
 
-		logger.Info("certificate rotation triggered")
+		log.Info("certificate rotation triggered")
 
 		return reconcile.Result{}, nil
 	}
 
 	after := crt.NotAfter.Sub(deadline)
 
-	logger.Info("certificate is still valid, enqueuing back", "after", after.String())
+	log.Info("certificate is still valid, enqueuing back", "after", after.String())
 
 	return reconcile.Result{Requeue: true, RequeueAfter: after}, nil
 }
diff --git a/controllers/datastore_controller.go b/controllers/datastore_controller.go
index 13fa47fa..6a57836f 100644
--- a/controllers/datastore_controller.go
+++ b/controllers/datastore_controller.go
@@ -39,13 +39,13 @@ func (r *DataStore) Reconcile(ctx context.Context, request reconcile.Request) (r
 	log := log.FromContext(ctx)
 
 	ds := &kamajiv1alpha1.DataStore{}
-	if err := r.Client.Get(ctx, request.NamespacedName, ds); err != nil {
-		if k8serrors.IsNotFound(err) {
-			return reconcile.Result{}, nil
-		}
-
-		log.Error(err, "unable to retrieve the request")
-
+	err := r.Client.Get(ctx, request.NamespacedName, ds)
+	if k8serrors.IsNotFound(err) {
+		log.Info("resource have been deleted, skipping")
+		return reconcile.Result{}, nil
+	}
+	if err != nil {
+		log.Error(err, "cannot retrieve the required resource")
 		return reconcile.Result{}, err
 	}
 
diff --git a/controllers/tenantcontrolplane_controller.go b/controllers/tenantcontrolplane_controller.go
index dd39d4e9..99b2cdea 100644
--- a/controllers/tenantcontrolplane_controller.go
+++ b/controllers/tenantcontrolplane_controller.go
@@ -15,7 +15,7 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
-	apimachineryerrors "k8s.io/apimachinery/pkg/api/errors"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/utils/clock"
@@ -84,16 +84,13 @@ func (r *TenantControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
 	defer cancelFn()
 
 	tenantControlPlane, err := r.getTenantControlPlane(ctx, req.NamespacedName)()
+	if k8serrors.IsNotFound(err) {
+		log.Info("resource have been deleted, skipping")
+		return reconcile.Result{}, nil
+	}
 	if err != nil {
-		if apimachineryerrors.IsNotFound(err) {
-			log.Info("resource may have been deleted, skipping")
-
-			return ctrl.Result{}, nil
-		}
-
-		log.Error(err, "cannot retrieve the required instance")
-
-		return ctrl.Result{}, err
+		log.Error(err, "cannot retrieve the required resource")
+		return reconcile.Result{}, err
 	}
 
 	releaser, err := mutex.Acquire(r.mutexSpec(tenantControlPlane))