diff --git a/test/e2e/hooks_test.go b/test/e2e/hooks_test.go index f1119a282b1c..8d18c729a3ed 100644 --- a/test/e2e/hooks_test.go +++ b/test/e2e/hooks_test.go @@ -650,6 +650,122 @@ spec: }) } +func (s *HooksSuite) TestTemplateLevelHooksWithRetry() { + var children []string + s.Given(). + Workflow(` +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + name: retries-with-hooks-and-artifact + labels: + workflows.argoproj.io/test: "true" + annotations: + workflows.argoproj.io/description: | + when retries and hooks are both included, the workflow cannot resolve the artifact + workflows.argoproj.io/version: '>= 3.5.0' +spec: + entrypoint: main + templates: + - name: main + steps: + - - name: build + template: output-artifact + hooks: + started: + expression: steps["build"].status == "Running" + template: started + success: + expression: steps["build"].status == "Succeeded" + template: success + failed: + expression: steps["build"].status == "Failed" || steps["build"].status == "Error" + template: failed + - - name: print + template: print-artifact + arguments: + artifacts: + - name: message + from: "{{steps.build.outputs.artifacts.result}}" + + - name: output-artifact + script: + image: python:alpine3.6 + command: [ python ] + source: | + import time + import random + time.sleep(random.randint(0,20)) # lifecycle hook for running won't trigger unless it runs for more than "a few seconds" + with open("result.txt", "w") as f: + f.write("Welcome") + # sys.exit("fail!") + timeout: 10s + retryStrategy: + limit: 4 + outputs: + artifacts: + - name: result + path: /result.txt + + - name: started + container: + image: python:alpine3.6 + command: [sh, -c] + args: ["echo STARTED!"] + + - name: success + container: + image: python:alpine3.6 + command: [sh, -c] + args: ["echo SUCCEEDED!"] + + - name: failed + container: + image: python:alpine3.6 + command: [sh, -c] + args: ["echo FAILED or ERROR!"] + + - name: print-artifact + inputs: + artifacts: + - name: message + path: /tmp/message + container: + image: python:alpine3.6 + command: [sh, -c] + args: ["cat /tmp/message"] +`).When(). + SubmitWorkflow(). + WaitForWorkflow(fixtures.ToBeCompleted). + Then(). + ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) { + assert.True(t, status.Fulfilled()) + for _, node :=range status.Nodes { + if node.Type == v1alpha1.NodeTypeRetry { + if node.Phase == v1alpha1.NodeSucceeded { + assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase) + } + if node.Phase == v1alpha1.NodeFailed { + assert.Equal(t, v1alpha1.WorkflowFailed, status.Phase) + } + children = node.Children + } + } + }). + ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool { + return status.Name == "retries-with-hooks-and-artifact[0].build(0)" + }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { + assert.Contains(t, children, status.ID) + assert.Equal(t, false, status.NodeFlag.Hooked) + }). + ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool { + return status.Name == "retries-with-hooks-and-artifact[0].build.hooks.started" + }, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) { + assert.Contains(t, children, status.ID) + assert.Equal(t, true, status.NodeFlag.Hooked) + }) +} + func TestHooksSuite(t *testing.T) { suite.Run(t, new(HooksSuite)) } diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 7f6feaeb5f67..e31baebae1c7 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -1764,7 +1764,15 @@ func (woc *wfOperationCtx) deletePVCs(ctx context.Context) error { // Check if we have a retry node which wasn't memoized and return that if we do func (woc *wfOperationCtx) possiblyGetRetryChildNode(node *wfv1.NodeStatus) *wfv1.NodeStatus { if node.Type == wfv1.NodeTypeRetry && !(node.MemoizationStatus != nil && node.MemoizationStatus.Hit) { - return getChildNodeIndex(node, woc.wf.Status.Nodes, -1) + for i := len(node.Children)-1 ; i >= 0; i-- { + childNode := getChildNodeIndex(node, woc.wf.Status.Nodes, i) + if childNode == nil { + continue + } + if childNode.NodeFlag == nil || !childNode.NodeFlag.Hooked { + return childNode + } + } } return nil }