Driver Controller: Enhance logging

Signed-off-by: nb-ohad <[email protected]>

internal/controller/driver_controller.go

@@ -190,19 +190,26 @@ func (r *DriverReconciler) SetupWithManager(mgr ctrl.Manager) error {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
 func (r *DriverReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	log := ctrllog.FromContext(ctx)
+	log.Info("Starting reconcile iteration for Ceph CSI driver", "req", req)
+
 	reconcileHandler := driverReconcile{}
 	reconcileHandler.DriverReconciler = *r
 	reconcileHandler.ctx = ctx
-	reconcileHandler.log = ctrllog.FromContext(ctx)
+	reconcileHandler.log = log
 	reconcileHandler.driver.Name = req.Name
 	reconcileHandler.driver.Namespace = req.Namespace
 
-	return reconcileHandler.reconcile()
+	result, err := reconcileHandler.reconcile()
+	if err != nil {
+		log.Error(err, "CSI Driver reconciliation failed")
+	} else {
+		log.Info("CSI Driver reconciliation completed successfully")
+	}
+	return result, err
 }
 
 func (r *driverReconcile) reconcile() (ctrl.Result, error) {
-	r.log.Info("Enter Reconcile", "req", client.ObjectKeyFromObject(&r.driver))
-
 	// Load the driver desired state based on driver resource, operator config resource and default values.
 	if err := r.LoadAndValidateDesiredState(); err != nil {
 		return ctrl.Result{}, err
@@ -221,7 +228,6 @@ func (r *driverReconcile) reconcile() (ctrl.Result, error) {
 	// of the reconciliation steps.
 	errList := utils.ChannelToSlice(errChan)
 	if err := errors.Join(errList...); err != nil {
-		r.log.Error(err, "Reconciliation failed")
 		return ctrl.Result{}, err
 	}
 
@@ -271,7 +277,7 @@ func (r *driverReconcile) LoadAndValidateDesiredState() error {
 
 	// Load the current desired state in the form of a ceph csi driver resource
 	if err := r.Get(r.ctx, client.ObjectKeyFromObject(&r.driver), &r.driver); err != nil {
-		r.log.Error(err, "Unable to load driver.csi.ceph.io object", "name", client.ObjectKeyFromObject(&r.driver))
+		r.log.Error(err, "Unable to load driver.csi.ceph.io", "name", client.ObjectKeyFromObject(&r.driver))
 		return err
 	}
 
@@ -384,9 +390,9 @@ func (r *driverReconcile) reconcileControllerPluginDeployment() error {
 	deploy.Namespace = r.driver.Namespace
 
 	log := r.log.WithValues("deploymentName", deploy.Name)
-	log.Info("Reconciling controller plugin deployment resource")
+	log.Info("Reconciling controller plugin deployment")
 
-	_, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, deploy, func() error {
+	opResult, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, deploy, func() error {
 		if err := ctrlutil.SetOwnerReference(&r.driver, deploy, r.Scheme); err != nil {
 			log.Error(err, "Failed setting an owner reference on deployment")
 			return err
@@ -708,10 +714,8 @@ func (r *driverReconcile) reconcileControllerPluginDeployment() error {
 		return nil
 	})
 
-	if err != nil {
-		r.log.Error(err, "")
-	}
-	return nil
+	logCreateOrUpdateResult(log, "controller plugin deployment", deploy, opResult, err)
+	return err
 }
 
 func (r *driverReconcile) reconcileNodePluginDeamonSet() error {
@@ -722,7 +726,7 @@ func (r *driverReconcile) reconcileNodePluginDeamonSet() error {
 	log := r.log.WithValues("daemonSetName", daemonSet.Name)
 	log.Info("Reconciling node plugin deployment")
 
-	_, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, daemonSet, func() error {
+	opResult, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, daemonSet, func() error {
 		if err := ctrlutil.SetOwnerReference(&r.driver, daemonSet, r.Scheme); err != nil {
 			log.Error(err, "Failed setting an owner reference on deployment")
 			return err
@@ -977,27 +981,25 @@ func (r *driverReconcile) reconcileNodePluginDeamonSet() error {
 		return nil
 	})
 
-	if err != nil {
-		r.log.Error(err, "")
-	}
-	return nil
+	logCreateOrUpdateResult(log, "node plugin daemonset", daemonSet, opResult, err)
+	return err
 }
 
 func (r *driverReconcile) reconcileLivnessService() error {
 	if r.driver.Spec.Liveness == nil {
 		return nil
 	}
 
-	service := corev1.Service{}
+	service := &corev1.Service{}
 	service.Namespace = r.driver.Namespace
 	service.Name = r.generateName("livness")
 
 	log := r.log.WithValues("service", service.Name)
-	log.Info("Reconciling livness service resource")
+	log.Info("Reconciling livness service")
 
-	_, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, &service, func() error {
-		if err := ctrlutil.SetOwnerReference(&r.driver, &service, r.Scheme); err != nil {
-			r.log.Error(err, "Faild setting an owner reference on service")
+	opResult, err := ctrlutil.CreateOrUpdate(r.ctx, r.Client, service, func() error {
+		if err := ctrlutil.SetOwnerReference(&r.driver, service, r.Scheme); err != nil {
+			log.Error(err, "Failed setting an owner reference on service")
 			return err
 		}
 
@@ -1015,10 +1017,8 @@ func (r *driverReconcile) reconcileLivnessService() error {
 		return nil
 	})
 
-	if err != nil {
-		r.log.Error(err, "")
-	}
-	return nil
+	logCreateOrUpdateResult(log, "livness service", service, opResult, err)
+	return err
 }
 
 func (r *driverReconcile) isRdbDriver() bool {
@@ -1037,6 +1037,29 @@ func (r *driverReconcile) generateName(suffix string) string {
 	return fmt.Sprintf("%s-%s", r.driver.Name, suffix)
 }
 
+func logCreateOrUpdateResult(
+	log logr.Logger,
+	subject string,
+	obj client.Object,
+	opRes ctrlutil.OperationResult,
+	err error,
+) {
+	if err != nil {
+		verb := utils.If(obj.GetUID() == "", "create", "update")
+		log.Error(err, fmt.Sprintf("Failed to %s %s", verb, subject))
+		return
+	}
+
+	switch opRes {
+	case ctrlutil.OperationResultNone:
+		log.Info(fmt.Sprintf("%s is already up to date", subject))
+	case ctrlutil.OperationResultUpdated:
+		log.Info(fmt.Sprintf("%s updated successfully", subject))
+	case ctrlutil.OperationResultCreated:
+		log.Info(fmt.Sprintf("%s created successfully", subject))
+	}
+}
+
 // mergeDriverSpecs will fill in any unset fields in dest with a copy of the same field in src
 func mergeDriverSpecs(dest, src *csiv1a1.DriverSpec) {
 	// Create a copy of the src, making sure that any value copied into dest is a not shared