From 1357d08956cc3072b1e41ccac74c7002e4012610 Mon Sep 17 00:00:00 2001 From: nb-ohad Date: Mon, 15 Jul 2024 21:58:54 +0300 Subject: [PATCH] Driver Controller: Enhance logging Signed-off-by: nb-ohad --- internal/controller/driver_controller.go | 70 ++++++++++++++++-------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/internal/controller/driver_controller.go b/internal/controller/driver_controller.go index 6ee9286d..8ad2a4f7 100644 --- a/internal/controller/driver_controller.go +++ b/internal/controller/driver_controller.go @@ -191,19 +191,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/controller-runtime@v0.17.3/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 @@ -222,7 +229,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 } @@ -272,7 +278,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 } @@ -382,9 +388,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 @@ -706,9 +712,7 @@ func (r *driverReconcile) reconcileControllerPluginDeployment() error { return nil }) - if err != nil { - r.log.Error(err, "") - } + logCreateOrUpdateResult(log, "controller plugin deployment", deploy, opResult, err) return nil } @@ -720,7 +724,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 @@ -975,9 +979,7 @@ func (r *driverReconcile) reconcileNodePluginDeamonSet() error { return nil }) - if err != nil { - r.log.Error(err, "") - } + logCreateOrUpdateResult(log, "node plugin daemonset", daemonSet, opResult, err) return nil } @@ -986,16 +988,16 @@ func (r *driverReconcile) reconcileLivnessService() error { 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, "Faild setting an owner reference on service") return err } @@ -1013,9 +1015,7 @@ func (r *driverReconcile) reconcileLivnessService() error { return nil }) - if err != nil { - r.log.Error(err, "") - } + logCreateOrUpdateResult(log, "livness service", service, opResult, err) return nil } @@ -1035,6 +1035,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 @@ -1191,4 +1214,3 @@ func mergeDriverSpecs(dest, src *csiv1a1.DriverSpec) { dest.CephFsClientType = src.CephFsClientType } } -