From b04e1aa0f90fc3d7154594742a11abc6c21d1e04 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 18:59:12 +0800 Subject: [PATCH 1/8] fix: resolve output artifact of the step's hook. Fixes: #13860 Signed-off-by: shuangkun --- pkg/apis/workflow/v1alpha1/workflow_types.go | 2 +- test/e2e/hooks_test.go | 60 ++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go index c90ebd76dd38..4e5894bde21b 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types.go @@ -1592,7 +1592,7 @@ func (lch *LifecycleHook) WithArgs(args Arguments) *LifecycleHook { var _ TemplateReferenceHolder = &WorkflowStep{} func (step *WorkflowStep) HasExitHook() bool { - return (step.Hooks != nil && step.Hooks.HasExitHook()) || step.OnExit != "" + return (step.Hooks != nil) || step.OnExit != "" } func (step *WorkflowStep) GetExitHook(args Arguments) *LifecycleHook { diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index d10e06cb5491..8ad661a03a76 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -872,6 +872,66 @@ spec: })) } +func (s *HooksSuite) TestHooksWithArtifacts() { + var onExitNodeName string + (s.Given(). + Workflow(`apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: test-hook- +spec: + entrypoint: main + templates: + - name: main + steps: + - - name: hello1 + template: parameterization + hooks: + success: + template: success-hook + expression: steps["hello1"].status == "Succeeded" + arguments: + artifacts: + - name: file_path + from: "{{steps.hello1.outputs.artifacts.result}}" + + - name: parameterization + script: + image: python:alpine3.6 + command: [python] + source: | + import os + with open("foo.txt", "w") as f: + f.write("Hello") + os.rename('foo.txt', '/tmp/foo.txt') + outputs: + artifacts: + - name: result + path: /tmp/foo.txt + + - name: success-hook + inputs: + artifacts: + - name: file_path + path: /tmp/file_path + script: + image: python:alpine3.6 + command: [sh, -c] + source: | + echo "File Path: {{inputs.artifacts.file_path.path}}" +`).When(). + SubmitWorkflow(). + WaitForWorkflow(fixtures.ToBeCompleted). + ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) { + assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase) + }). + ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool { + return strings.Contains(status.Name, ".hooks.succeed") + }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { + assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase) + })) +} + func TestHooksSuite(t *testing.T) { suite.Run(t, new(HooksSuite)) } From 6911f3ce6b24880e333cb9d523299a56aacfacbd Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 19:03:59 +0800 Subject: [PATCH 2/8] fix: remove brackets Signed-off-by: shuangkun --- pkg/apis/workflow/v1alpha1/workflow_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go index 4e5894bde21b..6ed35aec3e7e 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types.go @@ -1592,7 +1592,7 @@ func (lch *LifecycleHook) WithArgs(args Arguments) *LifecycleHook { var _ TemplateReferenceHolder = &WorkflowStep{} func (step *WorkflowStep) HasExitHook() bool { - return (step.Hooks != nil) || step.OnExit != "" + return step.Hooks != nil || step.OnExit != "" } func (step *WorkflowStep) GetExitHook(args Arguments) *LifecycleHook { From cdd63038eecab880b2a194a6a639f920e0344323 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 19:11:51 +0800 Subject: [PATCH 3/8] fix: remove brackets Signed-off-by: shuangkun --- test/e2e/hooks_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index 8ad661a03a76..6c6973e54f4d 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -872,9 +872,8 @@ spec: })) } -func (s *HooksSuite) TestHooksWithArtifacts() { - var onExitNodeName string - (s.Given(). +func (s *HooksSuite) TestHooksWithArtifactsInSteps() { + s.Given(). Workflow(`apiVersion: argoproj.io/v1alpha1 kind: Workflow metadata: @@ -929,7 +928,7 @@ spec: return strings.Contains(status.Name, ".hooks.succeed") }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase) - })) + }) } func TestHooksSuite(t *testing.T) { From e6d937dbaf4651c9619c43fff8a31cbbe750d258 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 21:20:54 +0800 Subject: [PATCH 4/8] fix: test Signed-off-by: shuangkun --- test/e2e/hooks_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index 6c6973e54f4d..50b49e2d90fc 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -924,6 +924,7 @@ spec: ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) { assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase) }). + Then(). ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool { return strings.Contains(status.Name, ".hooks.succeed") }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { From 3fdb56250ed17139483b710e67d5acf4ba6e706b Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 21:29:37 +0800 Subject: [PATCH 5/8] fix: test Signed-off-by: shuangkun --- test/e2e/hooks_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index 50b49e2d90fc..286c53364ce4 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -921,10 +921,10 @@ spec: `).When(). SubmitWorkflow(). WaitForWorkflow(fixtures.ToBeCompleted). + Then(). ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) { assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase) }). - Then(). ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool { return strings.Contains(status.Name, ".hooks.succeed") }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { From 23e4440bf052825f787e4504af094e0cdfce46f2 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 22:07:45 +0800 Subject: [PATCH 6/8] fix: test Signed-off-by: shuangkun --- pkg/apis/workflow/v1alpha1/workflow_types.go | 2 +- workflow/validate/validate.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go index 6ed35aec3e7e..c90ebd76dd38 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types.go @@ -1592,7 +1592,7 @@ func (lch *LifecycleHook) WithArgs(args Arguments) *LifecycleHook { var _ TemplateReferenceHolder = &WorkflowStep{} func (step *WorkflowStep) HasExitHook() bool { - return step.Hooks != nil || step.OnExit != "" + return (step.Hooks != nil && step.Hooks.HasExitHook()) || step.OnExit != "" } func (step *WorkflowStep) GetExitHook(args Arguments) *LifecycleHook { diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index f27375a7b3a9..6de1a9ebf7d3 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -963,7 +963,7 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps[%d].%s %s", tmpl.Name, i, step.Name, err.Error()) } - if step.HasExitHook() { + if step.HasExitHook() || step.Hooks != nil { ctx.addOutputsToScope(resolvedTmpl, fmt.Sprintf("steps.%s", step.Name), scope, false, false) } resolvedTemplates[step.Name] = resolvedTmpl From dc0cb329d5ac7098f2bb7a02cb9d9e25091350df Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 22:44:32 +0800 Subject: [PATCH 7/8] fix: test Signed-off-by: shuangkun --- test/e2e/hooks_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index 286c53364ce4..c9adf144535a 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -915,7 +915,7 @@ spec: path: /tmp/file_path script: image: python:alpine3.6 - command: [sh, -c] + command: ["sh"] source: | echo "File Path: {{inputs.artifacts.file_path.path}}" `).When(). From 8f5ae6a0c06439c93a938041e0160022399795a6 Mon Sep 17 00:00:00 2001 From: shuangkun Date: Sun, 3 Nov 2024 22:47:19 +0800 Subject: [PATCH 8/8] fix: test Signed-off-by: shuangkun --- test/e2e/hooks_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index c9adf144535a..3c787304c63a 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -926,7 +926,7 @@ spec: assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase) }). ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool { - return strings.Contains(status.Name, ".hooks.succeed") + return strings.Contains(status.Name, ".hooks.success") }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase) })