hooks: improve logging for hooks (#2208)

internal/controller/hooks/hooks_util.go

@@ -36,8 +36,14 @@ func getResourcesUsingNameSelector(r client.Reader, hook *kubeobjects.HookSpec,
 	if isValidK8sName(hook.NameSelector) {
 		// use nameSelector for Matching field
 		objs, err := getObjectsUsingValidK8sName(r, hook, objList)
+		if err != nil {
+			return ValidNameSelector, filteredObjs, fmt.Errorf(
+				"list resources using valid nameSelector=%q failed (hook=%q namespace=%q): %w",
+				hook.NameSelector, hook.Name, hook.Namespace, err,
+			)
+		}
 
-		return ValidNameSelector, objs, err
+		return ValidNameSelector, objs, nil
 	} else if isValidRegex(hook.NameSelector) {
 		// after listing without the fields selector, match with the regex for filtering
 		listOps := &client.ListOptions{
@@ -46,13 +52,19 @@ func getResourcesUsingNameSelector(r client.Reader, hook *kubeobjects.HookSpec,
 
 		err = r.List(context.Background(), objList, listOps)
 		if err != nil {
-			return RegexNameSelector, filteredObjs, err
+			return RegexNameSelector, filteredObjs, fmt.Errorf(
+				"list resources using regex nameSelector=%q failed (hook=%q namespace=%q): %w",
+				hook.NameSelector, hook.Name, hook.Namespace, err,
+			)
 		}
 
 		return RegexNameSelector, getObjectsBasedOnTypeAndRegex(objList, hook.NameSelector), nil
 	}
 
-	return InvalidNameSelector, filteredObjs, fmt.Errorf("nameSelector is neither distinct name nor regex")
+	return InvalidNameSelector, filteredObjs, fmt.Errorf(
+		"invalid nameSelector=%q (must be valid k8s name or regex)",
+		hook.NameSelector,
+	)
 }
 
 func getObjectsUsingValidK8sName(r client.Reader, hook *kubeobjects.HookSpec,
@@ -64,7 +76,10 @@ func getObjectsUsingValidK8sName(r client.Reader, hook *kubeobjects.HookSpec,
 
 	err := r.List(context.Background(), objList, listOps)
 	if err != nil {
-		return nil, fmt.Errorf("error listing resources using nameSelector: %w", err)
+		return nil, fmt.Errorf(
+			"list resources using nameSelector=%q failed (hook=%q namespace=%q): %w",
+			hook.NameSelector, hook.Name, hook.Namespace, err,
+		)
 	}
 
 	return getFilteredObjectsBasedOnTypeAndNameSelector(objList, hook.NameSelector), err
@@ -168,9 +183,18 @@ func getObjectsBasedOnTypeAndRegex(objList client.ObjectList, nameSelector strin
 func getResourcesUsingLabelSelector(r client.Reader, hook *kubeobjects.HookSpec,
 	objList client.ObjectList,
 ) error {
+	if len(hook.LabelSelector.MatchLabels) == 0 && len(hook.LabelSelector.MatchExpressions) == 0 {
+		return fmt.Errorf(
+			"list resources using labelSelector failed: empty MatchLabels and MatchExpressions "+
+				"(hook=%q namespace=%q)",
+			hook.Name, hook.Namespace,
+		)
+	}
+
 	selector, err := metav1.LabelSelectorAsSelector(hook.LabelSelector)
 	if err != nil {
-		return fmt.Errorf("error converting labelSelector to selector")
+		return fmt.Errorf("convert labelSelector to selector failed (hook=%q namespace=%q): %w",
+			hook.Name, hook.Namespace, err)
 	}
 
 	listOps := &client.ListOptions{
@@ -180,7 +204,8 @@ func getResourcesUsingLabelSelector(r client.Reader, hook *kubeobjects.HookSpec,
 
 	err = r.List(context.Background(), objList, listOps)
 	if err != nil {
-		return fmt.Errorf("error listing resources using labelSelector: %w", err)
+		return fmt.Errorf("list resources using labelSelector failed (hook=%q namespace=%q): %w",
+			hook.Name, hook.Namespace, err)
 	}
 
 	return nil

internal/controller/hooks/scale_hook.go

@@ -1,3 +1,6 @@
+// SPDX-FileCopyrightText: The RamenDR authors
+// SPDX-License-Identifier: Apache-2.0
+
 package hooks
 
 import (
@@ -116,12 +119,39 @@ func (s ScaleHook) Execute(log logr.Logger) error {
 
 	resources, err := s.getResourcesToScale()
 	if err != nil {
+		log.Error(err, "error occurred while fetching resources to scale",
+			"hook", s.Hook.Name,
+			"namespace", s.Hook.Namespace,
+			"operation", s.Hook.Scale.Operation,
+			"selectResource", s.Hook.SelectResource,
+			"nameSelector", s.Hook.NameSelector,
+			"labelSelector", s.Hook.LabelSelector,
+		)
+
 		return err
 	}
 
 	scaleOp := s.Hook.Scale.Operation
 
-	return s.processResources(resources, scaleOp, log)
+	if err := s.processResources(resources, scaleOp, log); err != nil {
+		log.Error(err, "error occurred while processing scale operation",
+			"hook", s.Hook.Name,
+			"namespace", s.Hook.Namespace,
+			"operation", scaleOp,
+			"resourcesCount", len(resources),
+		)
+
+		return err
+	}
+
+	log.Info("scale hook executed successfully",
+		"hook", s.Hook.Name,
+		"namespace", s.Hook.Namespace,
+		"operation", scaleOp,
+		"resourcesScaled", len(resources),
+	)
+
+	return nil
 }
 
 func (s ScaleHook) processResources(resources []client.Object, scaleOp string, log logr.Logger) error {
@@ -193,10 +223,10 @@ func (s ScaleHook) scaleResource(resource Resource, operation string, log logr.L
 	case ScaleSync:
 		return s.SyncResource(resource, log)
 	default:
-		return fmt.Errorf("unsupported scale operation: hook=%s, namespace=%s, operation=%s, resource=%s",
+		return fmt.Errorf("unsupported scale operation=%q: hook=%q namespace=%q resource=%q",
+			operation,
 			s.Hook.Name,
 			s.Hook.Namespace,
-			s.Hook.Scale.Operation,
 			resource.GetObjectMeta().GetName(),
 		)
 	}