fix: Exit lifecycle hook should respect expression. Fixes #8742 (#8744)

Signed-off-by: Yuan Tang <[email protected]>
argoproj · May 13, 2022 · 354dee8 · 354dee8
1 parent 91fc3ea
commit 354dee8
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 18 deletions.
diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go
@@ -52,7 +52,6 @@ spec:
  }).ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
  return strings.Contains(status.Name, "hook")
  }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
-
  assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
  })
 }
@@ -71,7 +70,6 @@ spec:
  - - name: step-1
  hooks:
  exit:
- expression: steps["step-1"].status == "Running"
  template: http
  success:
  expression: steps["step-1"].status == "Succeeded"
@@ -85,11 +83,43 @@ spec:
  WaitForWorkflow(fixtures.ToBeSucceeded).
  Then().
  ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
- assert.Equal(t, status.Phase, v1alpha1.WorkflowSucceeded)
+ assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase)
  }).ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
  return strings.Contains(status.Name, "hook")
  }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
+ assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
+ })
+}
 
+func (s *HooksSuite) TestExitHookWithExpression() {
+ s.Given().
+ Workflow(`apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+ generateName: lifecycle-hook-exit-hook-
+spec:
+ entrypoint: main
+ templates:
+ - name: main
+ steps:
+ - - name: step-1
+ hooks:
+ exit:
+ expression: steps["step-1"].status == "Running"
+ template: http
+ template: http
+ - name: http
+ http:
+ url: "https://httpstat.us"
+`).When().
+ SubmitWorkflow().
+ WaitForWorkflow(fixtures.ToBeSucceeded).
+ Then().
+ ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
+ assert.Equal(t, v1alpha1.WorkflowRunning, status.Phase)
+ }).ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
+ return strings.Contains(status.Name, "hook")
+ }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
  assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
  })
 }

diff --git a/workflow/controller/exit_handler.go b/workflow/controller/exit_handler.go
@@ -6,6 +6,8 @@ import (
  "fmt"
 
  wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
+ "github.com/argoproj/argo-workflows/v3/util/expr/argoexpr"
+ "github.com/argoproj/argo-workflows/v3/util/expr/env"
  "github.com/argoproj/argo-workflows/v3/util/template"
  "github.com/argoproj/argo-workflows/v3/workflow/common"
  "github.com/argoproj/argo-workflows/v3/workflow/templateresolution"
@@ -17,27 +19,35 @@ func (woc *wfOperationCtx) runOnExitNode(ctx context.Context, exitHook *wfv1.Lif
  lastChildNode := getChildNodeIndex(parentNode, woc.wf.Status.Nodes, -1)
  outputs = lastChildNode.Outputs
  }
-
  if exitHook != nil && woc.GetShutdownStrategy().ShouldExecute(true) {
- woc.log.WithField("lifeCycleHook", exitHook).Infof("Running OnExit handler")
-
- onExitNodeName := common.GenerateOnExitNodeName(parentNode.Name)
- resolvedArgs := exitHook.Arguments
+ execute := true
  var err error
- if !resolvedArgs.IsEmpty() && outputs != nil {
- resolvedArgs, err = woc.resolveExitTmplArgument(exitHook.Arguments, prefix, outputs)
+ if exitHook.Expression != "" {
+ execute, err = argoexpr.EvalBool(exitHook.Expression, env.GetFuncMap(template.EnvMap(woc.globalParams)))
  if err != nil {
  return true, nil, err
  }
-
  }
- onExitNode, err := woc.executeTemplate(ctx, onExitNodeName, &wfv1.WorkflowStep{Template: exitHook.Template, TemplateRef: exitHook.TemplateRef}, tmplCtx, resolvedArgs, &executeTemplateOpts{
+ if execute {
+ woc.log.WithField("lifeCycleHook", exitHook).Infof("Running OnExit handler")
+
+ onExitNodeName := common.GenerateOnExitNodeName(parentNode.Name)
+ resolvedArgs := exitHook.Arguments
+ if !resolvedArgs.IsEmpty() && outputs != nil {
+ resolvedArgs, err = woc.resolveExitTmplArgument(exitHook.Arguments, prefix, outputs)
+ if err != nil {
+ return true, nil, err
+ }
 
- boundaryID: boundaryID,
- onExitTemplate: true,
- })
- woc.addChildNode(parentNode.Name, onExitNodeName)
- return true, onExitNode, err
+ }
+ onExitNode, err := woc.executeTemplate(ctx, onExitNodeName, &wfv1.WorkflowStep{Template: exitHook.Template, TemplateRef: exitHook.TemplateRef}, tmplCtx, resolvedArgs, &executeTemplateOpts{
+
+ boundaryID: boundaryID,
+ onExitTemplate: true,
+ })
+ woc.addChildNode(parentNode.Name, onExitNodeName)
+ return true, onExitNode, err
+ }
  }
  return false, nil, nil
 }

diff --git a/workflow/controller/hooks.go b/workflow/controller/hooks.go
@@ -13,7 +13,7 @@ import (
 
 func (woc *wfOperationCtx) executeWfLifeCycleHook(ctx context.Context, tmplCtx *templateresolution.Context) error {
  for hookName, hook := range woc.execWf.Spec.Hooks {
- //exit hook will be execute in runOnExitNode
+ //exit hook will be executed in runOnExitNode
  if hookName == wfv1.ExitLifecycleEvent {
  continue
  }