From d662a766af3faaf5d9c734090c3383f8b38f75a6 Mon Sep 17 00:00:00 2001 From: Annaraya Narasagond Date: Mon, 10 Mar 2025 23:41:54 +0530 Subject: [PATCH] recipe: refactoring hook execution Moving json_util to hooks package. And also moving the check hook execution logic to controller/hooks package. Signed-off-by: Annaraya Narasagond --- internal/controller/hooks/check_hook.go | 47 ++++++++++++++++ internal/controller/hooks/exec_hook.go | 18 ++++++ internal/controller/hooks/hooks_factory.go | 31 +++++++++++ .../controller/{util => hooks}/json_util.go | 2 +- .../{util => hooks}/json_util_test.go | 10 ++-- internal/controller/vrg_kubeobjects.go | 55 +++++++------------ 6 files changed, 122 insertions(+), 41 deletions(-) create mode 100644 internal/controller/hooks/check_hook.go create mode 100644 internal/controller/hooks/exec_hook.go create mode 100644 internal/controller/hooks/hooks_factory.go rename internal/controller/{util => hooks}/json_util.go (99%) rename internal/controller/{util => hooks}/json_util_test.go (96%) diff --git a/internal/controller/hooks/check_hook.go b/internal/controller/hooks/check_hook.go new file mode 100644 index 000000000..47fbdf14a --- /dev/null +++ b/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 +} diff --git a/internal/controller/hooks/exec_hook.go b/internal/controller/hooks/exec_hook.go new file mode 100644 index 000000000..76e5ad6d3 --- /dev/null +++ b/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 +} diff --git a/internal/controller/hooks/hooks_factory.go b/internal/controller/hooks/hooks_factory.go new file mode 100644 index 000000000..cf3fb30fb --- /dev/null +++ b/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") + } +} diff --git a/internal/controller/util/json_util.go b/internal/controller/hooks/json_util.go similarity index 99% rename from internal/controller/util/json_util.go rename to internal/controller/hooks/json_util.go index 9dfd49ab4..ebdb576bd 100644 --- a/internal/controller/util/json_util.go +++ b/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" diff --git a/internal/controller/util/json_util_test.go b/internal/controller/hooks/json_util_test.go similarity index 96% rename from internal/controller/util/json_util_test.go rename to internal/controller/hooks/json_util_test.go index b9a5f204a..d1decc499 100644 --- a/internal/controller/util/json_util_test.go +++ b/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) } }) diff --git a/internal/controller/vrg_kubeobjects.go b/internal/controller/vrg_kubeobjects.go index 0ac1532d5..2c089c5ad 100644 --- a/internal/controller/vrg_kubeobjects.go +++ b/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 {