Skip to content

Commit

Permalink
fix: filter hook node to find the correct lastNode. Fixes: argoproj#1…
Browse files Browse the repository at this point in the history
…2109

Signed-off-by: shuangkun <tsk2013uestc@163.com>
  • Loading branch information
shuangkun committed Mar 18, 2024
1 parent ebce8ef commit e6edd28
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 1 deletion.
116 changes: 116 additions & 0 deletions test/e2e/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
10 changes: 9 additions & 1 deletion workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit e6edd28

Please sign in to comment.