Skip to content

Commit 10f5db6

Browse files
smile-luobinsarabala1979
smile-luobin
authored and
sarabala1979
committed
fix(controller): fix bugs in processing retry node output parameters. Fixes argoproj#6948 (argoproj#6956)
* fix(controller): fix bugs when process retry node ouput Signed-off-by: smile-luobin <[email protected]> * fix(controller): fix runOnExitNode unable to get retryNode outputs Signed-off-by: smile-luobin <[email protected]>
1 parent 8897fff commit 10f5db6

File tree

5 files changed

+191
-6
lines changed

5 files changed

+191
-6
lines changed

‎workflow/controller/dag.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ func (woc *wfOperationCtx) executeDAG(ctx context.Context, nodeName string, tmpl
249249
if taskNode.Completed() {
250250
// Run the node's onExit node, if any. Since this is a target task, we don't need to consider the status
251251
// of the onExit node before continuing. That will be done in assesDAGPhase
252-
_, _, err := woc.runOnExitNode(ctx, dagCtx.GetTask(taskName).GetExitHook(woc.execWf.Spec.Arguments), taskNode.Name, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName, taskNode.Outputs)
252+
_, _, err := woc.runOnExitNode(ctx, dagCtx.GetTask(taskName).GetExitHook(woc.execWf.Spec.Arguments), taskNode, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName)
253253
if err != nil {
254254
return node, err
255255
}
@@ -350,7 +350,7 @@ func (woc *wfOperationCtx) executeDAGTask(ctx context.Context, dagCtx *dagContex
350350

351351
if node.Completed() {
352352
// Run the node's onExit node, if any.
353-
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, task.GetExitHook(woc.execWf.Spec.Arguments), node.Name, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName, node.Outputs)
353+
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, task.GetExitHook(woc.execWf.Spec.Arguments), node, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName)
354354
if hasOnExitNode && (onExitNode == nil || !onExitNode.Fulfilled() || err != nil) {
355355
// The onExit node is either not complete or has errored out, return.
356356
return

‎workflow/controller/exit_handler.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,17 @@ import (
1111
"github.com/argoproj/argo-workflows/v3/workflow/templateresolution"
1212
)
1313

14-
func (woc *wfOperationCtx) runOnExitNode(ctx context.Context, exitHook *wfv1.LifecycleHook, parentNodeName, boundaryID string, tmplCtx *templateresolution.Context, prefix string, outputs *wfv1.Outputs) (bool, *wfv1.NodeStatus, error) {
14+
func (woc *wfOperationCtx) runOnExitNode(ctx context.Context, exitHook *wfv1.LifecycleHook, parentNode *wfv1.NodeStatus, boundaryID string, tmplCtx *templateresolution.Context, prefix string) (bool, *wfv1.NodeStatus, error) {
15+
outputs := parentNode.Outputs
16+
if parentNode.Type == wfv1.NodeTypeRetry {
17+
lastChildNode := getChildNodeIndex(parentNode, woc.wf.Status.Nodes, -1)
18+
outputs = lastChildNode.Outputs
19+
}
20+
1521
if exitHook != nil && woc.GetShutdownStrategy().ShouldExecute(true) {
1622
woc.log.WithField("lifeCycleHook", exitHook).Infof("Running OnExit handler")
1723

18-
onExitNodeName := common.GenerateOnExitNodeName(parentNodeName)
24+
onExitNodeName := common.GenerateOnExitNodeName(parentNode.Name)
1925
resolvedArgs := exitHook.Arguments
2026
var err error
2127
if !resolvedArgs.IsEmpty() && outputs != nil {
@@ -30,7 +36,7 @@ func (woc *wfOperationCtx) runOnExitNode(ctx context.Context, exitHook *wfv1.Lif
3036
boundaryID: boundaryID,
3137
onExitTemplate: true,
3238
})
33-
woc.addChildNode(parentNodeName, onExitNodeName)
39+
woc.addChildNode(parentNode.Name, onExitNodeName)
3440
return true, onExitNode, err
3541
}
3642
return false, nil, nil

‎workflow/controller/operator.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1789,6 +1789,11 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string,
17891789
if processedTmpl.Synchronization != nil {
17901790
woc.controller.syncManager.Release(woc.wf, node.ID, processedTmpl.Synchronization)
17911791
}
1792+
lastChildNode := getChildNodeIndex(retryParentNode, woc.wf.Status.Nodes, -1)
1793+
if lastChildNode != nil {
1794+
retryParentNode.Outputs = lastChildNode.Outputs.DeepCopy()
1795+
woc.wf.Status.Nodes[node.ID] = *retryParentNode
1796+
}
17921797
return retryParentNode, nil
17931798
}
17941799
lastChildNode := getChildNodeIndex(retryParentNode, woc.wf.Status.Nodes, -1)

‎workflow/controller/operator_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7412,3 +7412,177 @@ func TestBuildRetryStrategyLocalScope(t *testing.T) {
74127412
assert.Equal(t, string(wfv1.NodeFailed), localScope[common.LocalVarRetriesLastStatus])
74137413
assert.Equal(t, "6", localScope[common.LocalVarRetriesLastDuration])
74147414
}
7415+
7416+
var exitHandlerWithRetryNodeParam = `
7417+
apiVersion: argoproj.io/v1alpha1
7418+
kind: Workflow
7419+
metadata:
7420+
name: exit-handler-with-param-xbh52
7421+
namespace: argo
7422+
spec:
7423+
arguments: {}
7424+
entrypoint: main
7425+
serviceAccountName: argo
7426+
templates:
7427+
- inputs: {}
7428+
metadata: {}
7429+
name: main
7430+
outputs: {}
7431+
steps:
7432+
- - arguments: {}
7433+
hooks:
7434+
exit:
7435+
arguments:
7436+
parameters:
7437+
- name: message
7438+
value: '{{steps.step-1.outputs.parameters.result}}'
7439+
template: exit
7440+
name: step-1
7441+
template: output
7442+
- container:
7443+
args:
7444+
- echo -n hello world > /tmp/hello_world.txt; exit 1
7445+
command:
7446+
- sh
7447+
- -c
7448+
image: alpine:latest
7449+
name: ""
7450+
resources: {}
7451+
inputs: {}
7452+
metadata: {}
7453+
name: output
7454+
outputs:
7455+
parameters:
7456+
- name: result
7457+
valueFrom:
7458+
default: Foobar
7459+
path: /tmp/hello_world.txt
7460+
retryStrategy:
7461+
backoff:
7462+
duration: 1s
7463+
limit: "1"
7464+
retryPolicy: Always
7465+
- inputs:
7466+
parameters:
7467+
- name: message
7468+
value: GoodValue
7469+
metadata: {}
7470+
name: exit
7471+
outputs: {}
7472+
script:
7473+
args:
7474+
- echo {{inputs.parameters.message}}
7475+
command:
7476+
- sh
7477+
- -c
7478+
image: alpine:latest
7479+
name: ""
7480+
resources: {}
7481+
source: ""
7482+
status:
7483+
nodes:
7484+
exit-handler-with-param-xbh52:
7485+
children:
7486+
- exit-handler-with-param-xbh52-3621967439
7487+
displayName: exit-handler-with-param-xbh52
7488+
finishedAt: "2021-10-18T03:28:14Z"
7489+
id: exit-handler-with-param-xbh52
7490+
name: exit-handler-with-param-xbh52
7491+
startedAt: "2021-10-18T03:27:44Z"
7492+
templateName: main
7493+
templateScope: local/exit-handler-with-param-xbh52
7494+
type: Steps
7495+
exit-handler-with-param-xbh52-1429999455:
7496+
boundaryID: exit-handler-with-param-xbh52
7497+
displayName: step-1(1)
7498+
finishedAt: "2021-10-18T03:27:58Z"
7499+
hostNodeName: smile
7500+
id: exit-handler-with-param-xbh52-1429999455
7501+
message: Error (exit code 1)
7502+
name: exit-handler-with-param-xbh52[0].step-1(1)
7503+
outputs:
7504+
exitCode: "1"
7505+
parameters:
7506+
- name: result
7507+
value: hello world
7508+
valueFrom:
7509+
default: Foobar
7510+
path: /tmp/hello_world.txt
7511+
phase: Failed
7512+
progress: 1/1
7513+
resourcesDuration:
7514+
cpu: 5
7515+
memory: 5
7516+
startedAt: "2021-10-18T03:27:54Z"
7517+
templateName: output
7518+
templateScope: local/exit-handler-with-param-xbh52
7519+
type: Pod
7520+
exit-handler-with-param-xbh52-2034140834:
7521+
boundaryID: exit-handler-with-param-xbh52
7522+
displayName: step-1(0)
7523+
finishedAt: "2021-10-18T03:27:48Z"
7524+
hostNodeName: smile
7525+
id: exit-handler-with-param-xbh52-2034140834
7526+
message: Error (exit code 1)
7527+
name: exit-handler-with-param-xbh52[0].step-1(0)
7528+
outputs:
7529+
exitCode: "1"
7530+
parameters:
7531+
- name: result
7532+
value: hello world
7533+
valueFrom:
7534+
default: Foobar
7535+
path: /tmp/hello_world.txt
7536+
phase: Failed
7537+
progress: 1/1
7538+
resourcesDuration:
7539+
cpu: 5
7540+
memory: 5
7541+
startedAt: "2021-10-18T03:27:44Z"
7542+
templateName: output
7543+
templateScope: local/exit-handler-with-param-xbh52
7544+
type: Pod
7545+
exit-handler-with-param-xbh52-3203867295:
7546+
boundaryID: exit-handler-with-param-xbh52
7547+
children:
7548+
- exit-handler-with-param-xbh52-2034140834
7549+
- exit-handler-with-param-xbh52-1429999455
7550+
displayName: step-1
7551+
finishedAt: "2021-10-18T03:28:04Z"
7552+
id: exit-handler-with-param-xbh52-3203867295
7553+
message: No more retries left
7554+
name: exit-handler-with-param-xbh52[0].step-1
7555+
startedAt: "2021-10-18T03:27:44Z"
7556+
templateName: output
7557+
templateScope: local/exit-handler-with-param-xbh52
7558+
type: Retry
7559+
exit-handler-with-param-xbh52-3621967439:
7560+
boundaryID: exit-handler-with-param-xbh52
7561+
children:
7562+
- exit-handler-with-param-xbh52-3203867295
7563+
displayName: '[0]'
7564+
finishedAt: "2021-10-18T03:28:14Z"
7565+
id: exit-handler-with-param-xbh52-3621967439
7566+
message: child 'exit-handler-with-param-xbh52-3203867295' failed
7567+
name: exit-handler-with-param-xbh52[0]
7568+
startedAt: "2021-10-18T03:27:44Z"
7569+
templateScope: local/exit-handler-with-param-xbh52
7570+
type: StepGroup
7571+
startedAt: "2021-10-18T03:27:44Z"
7572+
`
7573+
7574+
func TestExitHandlerWithRetryNodeParam(t *testing.T) {
7575+
wf := wfv1.MustUnmarshalWorkflow(exitHandlerWithRetryNodeParam)
7576+
cancel, controller := newController(wf)
7577+
defer cancel()
7578+
7579+
ctx := context.Background()
7580+
woc := newWorkflowOperationCtx(wf, controller)
7581+
7582+
woc.operate(ctx)
7583+
retryStepNode := woc.wf.GetNodeByName("exit-handler-with-param-xbh52[0].step-1")
7584+
assert.Equal(t, 1, len(retryStepNode.Outputs.Parameters))
7585+
assert.Equal(t, "hello world", retryStepNode.Outputs.Parameters[0].Value.String())
7586+
onExitNode := woc.wf.GetNodeByName("exit-handler-with-param-xbh52[0].step-1.onExit")
7587+
assert.Equal(t, "hello world", onExitNode.Inputs.Parameters[0].Value.String())
7588+
}

‎workflow/controller/steps.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (woc *wfOperationCtx) executeStepGroup(ctx context.Context, stepGroup []wfv
260260
if !childNode.Fulfilled() {
261261
completed = false
262262
} else if childNode.Completed() {
263-
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, step.GetExitHook(woc.execWf.Spec.Arguments), childNode.Name, stepsCtx.boundaryID, stepsCtx.tmplCtx, "steps."+step.Name, childNode.Outputs)
263+
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, step.GetExitHook(woc.execWf.Spec.Arguments), &childNode, stepsCtx.boundaryID, stepsCtx.tmplCtx, "steps."+step.Name)
264264
if hasOnExitNode && (onExitNode == nil || !onExitNode.Fulfilled() || err != nil) {
265265
// The onExit node is either not complete or has errored out, return.
266266
completed = false

0 commit comments

Comments
 (0)