Merge pull request #473 from red-hat-storage/sync_us--main

Syncing latest changes from upstream main for ramen

Author: ShyamsundarR

internal/controller/hooks/check_hook.go

@@ -0,0 +1,47 @@
+// SPDX-FileCopyrightText: The RamenDR authors
+// SPDX-License-Identifier: Apache-2.0
+
+package hooks
+
+import (
+	"fmt"
+
+	"github.com/go-logr/logr"
+	"github.com/ramendr/ramen/internal/controller/kubeobjects"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+type CheckHook struct {
+	Hook *kubeobjects.HookSpec
+}
+
+func (c CheckHook) Execute(client client.Client, log logr.Logger) error {
+	hookResult, err := EvaluateCheckHook(client, c.Hook, log)
+	if err != nil {
+		log.Error(err, "error occurred while evaluating check hook")
+
+		return err
+	}
+
+	hookName := c.Hook.Name + "/" + c.Hook.Chk.Name
+	log.Info("check hook executed successfully", "hook", hookName, "result", hookResult)
+
+	if !hookResult && shouldHookBeFailedOnError(c.Hook) {
+		return fmt.Errorf("stopping workflow as hook %s failed", c.Hook.Name)
+	}
+
+	return nil
+}
+
+func shouldHookBeFailedOnError(hook *kubeobjects.HookSpec) bool {
+	// hook.Check.OnError overwrites the feature of hook.OnError -- defaults to fail
+	if hook.Chk.OnError != "" && hook.Chk.OnError == "continue" {
+		return false
+	}
+
+	if hook.OnError != "" && hook.OnError == "continue" {
+		return false
+	}
+
+	return true
+}

internal/controller/hooks/exec_hook.go

@@ -0,0 +1,18 @@
+// SPDX-FileCopyrightText: The RamenDR authors
+// SPDX-License-Identifier: Apache-2.0
+
+package hooks
+
+import (
+	"github.com/go-logr/logr"
+	"github.com/ramendr/ramen/internal/controller/kubeobjects"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+type ExecHook struct {
+	Hook *kubeobjects.HookSpec
+}
+
+func (e ExecHook) Execute(client client.Client, log logr.Logger) error {
+	return nil
+}

internal/controller/hooks/hooks_factory.go

@@ -0,0 +1,31 @@
+// SPDX-FileCopyrightText: The RamenDR authors
+// SPDX-License-Identifier: Apache-2.0
+
+package hooks
+
+import (
+	"fmt"
+
+	"github.com/go-logr/logr"
+	"github.com/ramendr/ramen/internal/controller/kubeobjects"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// Hook interface will help in executing the hooks based on the types.
+// Supported types are "check", "scale" and "exec". The implementor needs
+// return the result which would be boolean and error if any.
+type HookExecutor interface {
+	Execute(client client.Client, log logr.Logger) error
+}
+
+// Based on the hook type, return the appropriate implementation of the hook.
+func GetHookExecutor(hook kubeobjects.HookSpec) (HookExecutor, error) {
+	switch hook.Type {
+	case "check":
+		return CheckHook{Hook: &hook}, nil
+	case "exec":
+		return ExecHook{Hook: &hook}, nil
+	default:
+		return nil, fmt.Errorf("unsupported hook type")
+	}
+}

internal/controller/util/json_util.go internal/controller/hooks/json_util.go

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: The RamenDR authors
 // SPDX-License-Identifier: Apache-2.0
 
-package util
+package hooks
 
 import (
 	"context"

internal/controller/util/json_util_test.go internal/controller/hooks/json_util_test.go

@@ -1,15 +1,15 @@
 // SPDX-FileCopyrightText: The RamenDR authors
 // SPDX-License-Identifier: Apache-2.0
 
-package util_test
+package hooks_test
 
 import (
 	"encoding/json"
 	"strconv"
 	"testing"
 
+	"github.com/ramendr/ramen/internal/controller/hooks"
 	"github.com/ramendr/ramen/internal/controller/kubeobjects"
-	"github.com/ramendr/ramen/internal/controller/util"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -166,7 +166,7 @@ func TestEvaluateCheckHookExp(t *testing.T) {
 				t.Error(err)
 			}
 
-			_, err = util.EvaluateCheckHookExp(test.jsonPathExprs, jsonData)
+			_, err = hooks.EvaluateCheckHookExp(test.jsonPathExprs, jsonData)
 			if (err == nil) != test.result {
 				t.Errorf("EvaluateCheckHookExp() = %v, want %v", err, test.result)
 			}
@@ -182,7 +182,7 @@ func TestEvaluateCheckHookForObjects(t *testing.T) {
 		objs := []client.Object{test.jsonObj}
 
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			_, err := util.EvaluateCheckHookForObjects(objs, test.hook, log)
+			_, err := hooks.EvaluateCheckHookForObjects(objs, test.hook, log)
 			if (err == nil) != test.result {
 				t.Errorf("EvaluateCheckHookExpObject() = %v, want %v", err, test.result)
 			}
@@ -318,7 +318,7 @@ func Test_isValidJsonPathExpression(t *testing.T) {
 		test := tt
 
 		t.Run(test.name, func(t *testing.T) {
-			if got := util.IsValidJSONPathExpression(test.args.expr); got != test.want {
+			if got := hooks.IsValidJSONPathExpression(test.args.expr); got != test.want {
 				t.Errorf("IsValidJSONPathExpression() = %v, want %v", got, test.want)
 			}
 		})

internal/controller/vrg_kubeobjects.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/go-logr/logr"
 	ramen "github.com/ramendr/ramen/api/v1alpha1"
+	"github.com/ramendr/ramen/internal/controller/hooks"
 	"github.com/ramendr/ramen/internal/controller/kubeobjects"
 	"github.com/ramendr/ramen/internal/controller/util"
 	Recipe "github.com/ramendr/recipe/api/v1alpha1"
@@ -280,7 +281,15 @@ func (v *VRGInstance) executeCaptureSteps(result *ctrl.Result, pathName, capture
 		isEssentialStep := cg.GroupEssential != nil && *cg.GroupEssential
 
 		if cg.IsHook {
-			err = v.executeHook(cg.Hook, log1)
+			executor, err1 := hooks.GetHookExecutor(cg.Hook)
+			if err1 != nil {
+				// continue if hook type is not supported. Supported types are "check" and "exec"
+				log1.Info("Hook type not supported", "hook", cg.Hook)
+
+				continue
+			}
+
+			err = executor.Execute(v.reconciler.Client, log1)
 		}
 
 		if !cg.IsHook {
@@ -325,39 +334,6 @@ func (v *VRGInstance) executeCaptureSteps(result *ctrl.Result, pathName, capture
 	return allEssentialStepsFailed, nil
 }
 
-func (v *VRGInstance) executeHook(hook kubeobjects.HookSpec, log1 logr.Logger) error {
-	if hook.Type == "check" {
-		hookResult, err := util.EvaluateCheckHook(v.reconciler.APIReader, &hook, log1)
-		if err != nil {
-			log1.Error(err, "error occurred during check hook ")
-		} else {
-			hookName := hook.Name + "/" + hook.Chk.Name
-			log1.Info("Check hook executed successfully", "check hook is ", hookName, " result is ", hookResult)
-		}
-
-		if !hookResult && shouldHookBeFailedOnError(&hook) {
-			return fmt.Errorf("stopping workflow sequence as check hook failed")
-		}
-
-		return nil
-	}
-
-	return nil
-}
-
-func shouldHookBeFailedOnError(hook *kubeobjects.HookSpec) bool {
-	// hook.Check.OnError overwrites the feature of hook.OnError -- defaults to fail
-	if hook.Chk.OnError != "" && hook.Chk.OnError == "continue" {
-		return false
-	}
-
-	if hook.OnError != "" && hook.OnError == "continue" {
-		return false
-	}
-
-	return true
-}
-
 func (v *VRGInstance) kubeObjectsGroupCapture(
 	result *ctrl.Result,
 	captureGroup kubeobjects.CaptureSpec,
@@ -758,6 +734,7 @@ func (v *VRGInstance) kubeObjectsRecoveryStartOrResume(
 	return v.kubeObjectsRecoverRequestsDelete(result, v.veleroNamespaceName(), labels)
 }
 
+// nolint: gocognit,cyclop
 func (v *VRGInstance) executeRecoverSteps(result *ctrl.Result, s3StoreAccessor s3StoreAccessor,
 	captureToRecoverFromIdentifier *ramen.KubeObjectsCaptureIdentifier, captureRequests,
 	recoverRequests map[string]kubeobjects.Request, requests []kubeobjects.Request, log logr.Logger,
@@ -776,7 +753,15 @@ func (v *VRGInstance) executeRecoverSteps(result *ctrl.Result, s3StoreAccessor s
 		isEssentialStep := rg.GroupEssential != nil && *rg.GroupEssential
 
 		if rg.IsHook {
-			err = v.executeHook(rg.Hook, log1)
+			executor, err1 := hooks.GetHookExecutor(rg.Hook)
+			if err1 != nil {
+				// continue if hook type is not supported. Supported types are "check" and "exec"
+				log1.Info("Hook type not supported", "hook", rg.Hook)
+
+				continue
+			}
+
+			err = executor.Execute(v.reconciler.Client, log1)
 		}
 
 		if !rg.IsHook {