fix: filter hook node to find the correct lastNode. Fixes: #12109

test/e2e/hooks_test.go

@@ -650,6 +650,133 @@ 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
+          import sys
+          time.sleep(1) # 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")
+          if {{retries}} == 2:
+          	sys.exit(0)
+          sys.exit(1)
+      retryStrategy: 
+        limit: 2
+      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())
+			assert.Equal(t, v1alpha1.WorkflowSucceeded, status.Phase)
+			for _, node := range status.Nodes {
+				if node.Type == v1alpha1.NodeTypeRetry {
+					assert.Equal(t, v1alpha1.NodeSucceeded, node.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)
+			assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
+		})).
+		ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
+			return status.Name == "retries-with-hooks-and-artifact[0].build.hooks.success"
+		}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
+			assert.Contains(t, children, status.ID)
+			assert.Equal(t, true, status.NodeFlag.Hooked)
+			assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
+		}).
+		ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
+			return status.Name == "retries-with-hooks-and-artifact[1].print"
+		}, 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))
 }

workflow/controller/operator.go

@@ -1764,7 +1764,17 @@ 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)
+		// If a retry node has hooks, the hook nodes will also become its children,
+		// so we need to filter out the hook nodes when finding the last child node of the retry node.
+		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
 }