Skip to content

Commit

Permalink
fix(controller): fix bugs in processing retry node output parameters. F…
Browse files Browse the repository at this point in the history 
…ixes #6948 (#6956)

* fix(controller): fix bugs when process retry node ouput

Signed-off-by: smile-luobin <luobin_smile@163.com>

* fix(controller): fix runOnExitNode unable to get retryNode outputs

Signed-off-by: smile-luobin <luobin_smile@163.com>
  • Loading branch information
@smile-luobin
smile-luobin authored Oct 21, 2021
1 parent 7ed5154 commit 50813da
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 6 deletions.
4 changes: 2 additions & 2 deletions workflow/controller/dag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (woc *wfOperationCtx) executeDAG(ctx context.Context, nodeName string, tmpl
if taskNode.Completed() {
// Run the node's onExit node, if any. Since this is a target task, we don't need to consider the status
// of the onExit node before continuing. That will be done in assesDAGPhase
_, _, err := woc.runOnExitNode(ctx, dagCtx.GetTask(taskName).GetExitHook(woc.execWf.Spec.Arguments), taskNode.Name, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName, taskNode.Outputs)
_, _, err := woc.runOnExitNode(ctx, dagCtx.GetTask(taskName).GetExitHook(woc.execWf.Spec.Arguments), taskNode, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName)
if err != nil {
return node, err
}
Expand Down Expand Up @@ -350,7 +350,7 @@ func (woc *wfOperationCtx) executeDAGTask(ctx context.Context, dagCtx *dagContex

if node.Completed() {
// Run the node's onExit node, if any.
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, task.GetExitHook(woc.execWf.Spec.Arguments), node.Name, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName, node.Outputs)
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, task.GetExitHook(woc.execWf.Spec.Arguments), node, dagCtx.boundaryID, dagCtx.tmplCtx, "tasks."+taskName)
if hasOnExitNode && (onExitNode == nil || !onExitNode.Fulfilled() || err != nil) {
// The onExit node is either not complete or has errored out, return.
return
Expand Down
12 changes: 9 additions & 3 deletions workflow/controller/exit_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@ import (
"github.com/argoproj/argo-workflows/v3/workflow/templateresolution"
)

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) {
func (woc *wfOperationCtx) runOnExitNode(ctx context.Context, exitHook *wfv1.LifecycleHook, parentNode *wfv1.NodeStatus, boundaryID string, tmplCtx *templateresolution.Context, prefix string) (bool, *wfv1.NodeStatus, error) {
outputs := parentNode.Outputs
if parentNode.Type == wfv1.NodeTypeRetry {
lastChildNode := getChildNodeIndex(parentNode, woc.wf.Status.Nodes, -1)
outputs = lastChildNode.Outputs
}

if exitHook != nil && woc.GetShutdownStrategy().ShouldExecute(true) {
woc.log.WithField("lifeCycleHook", exitHook).Infof("Running OnExit handler")

onExitNodeName := common.GenerateOnExitNodeName(parentNodeName)
onExitNodeName := common.GenerateOnExitNodeName(parentNode.Name)
resolvedArgs := exitHook.Arguments
var err error
if !resolvedArgs.IsEmpty() && outputs != nil {
Expand All @@ -30,7 +36,7 @@ func (woc *wfOperationCtx) runOnExitNode(ctx context.Context, exitHook *wfv1.Lif
boundaryID: boundaryID,
onExitTemplate: true,
})
woc.addChildNode(parentNodeName, onExitNodeName)
woc.addChildNode(parentNode.Name, onExitNodeName)
return true, onExitNode, err
}
return false, nil, nil
Expand Down
5 changes: 5 additions & 0 deletions workflow/controller/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,11 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string,
if processedTmpl.Synchronization != nil {
woc.controller.syncManager.Release(woc.wf, node.ID, processedTmpl.Synchronization)
}
lastChildNode := getChildNodeIndex(retryParentNode, woc.wf.Status.Nodes, -1)
if lastChildNode != nil {
retryParentNode.Outputs = lastChildNode.Outputs.DeepCopy()
woc.wf.Status.Nodes[node.ID] = *retryParentNode
}
return retryParentNode, nil
}
lastChildNode := getChildNodeIndex(retryParentNode, woc.wf.Status.Nodes, -1)
Expand Down
174 changes: 174 additions & 0 deletions workflow/controller/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7414,3 +7414,177 @@ func TestBuildRetryStrategyLocalScope(t *testing.T) {
assert.Equal(t, string(wfv1.NodeFailed), localScope[common.LocalVarRetriesLastStatus])
assert.Equal(t, "6", localScope[common.LocalVarRetriesLastDuration])
}

var exitHandlerWithRetryNodeParam = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
name: exit-handler-with-param-xbh52
namespace: argo
spec:
arguments: {}
entrypoint: main
serviceAccountName: argo
templates:
- inputs: {}
metadata: {}
name: main
outputs: {}
steps:
- - arguments: {}
hooks:
exit:
arguments:
parameters:
- name: message
value: '{{steps.step-1.outputs.parameters.result}}'
template: exit
name: step-1
template: output
- container:
args:
- echo -n hello world > /tmp/hello_world.txt; exit 1
command:
- sh
- -c
image: alpine:latest
name: ""
resources: {}
inputs: {}
metadata: {}
name: output
outputs:
parameters:
- name: result
valueFrom:
default: Foobar
path: /tmp/hello_world.txt
retryStrategy:
backoff:
duration: 1s
limit: "1"
retryPolicy: Always
- inputs:
parameters:
- name: message
value: GoodValue
metadata: {}
name: exit
outputs: {}
script:
args:
- echo {{inputs.parameters.message}}
command:
- sh
- -c
image: alpine:latest
name: ""
resources: {}
source: ""
status:
nodes:
exit-handler-with-param-xbh52:
children:
- exit-handler-with-param-xbh52-3621967439
displayName: exit-handler-with-param-xbh52
finishedAt: "2021-10-18T03:28:14Z"
id: exit-handler-with-param-xbh52
name: exit-handler-with-param-xbh52
startedAt: "2021-10-18T03:27:44Z"
templateName: main
templateScope: local/exit-handler-with-param-xbh52
type: Steps
exit-handler-with-param-xbh52-1429999455:
boundaryID: exit-handler-with-param-xbh52
displayName: step-1(1)
finishedAt: "2021-10-18T03:27:58Z"
hostNodeName: smile
id: exit-handler-with-param-xbh52-1429999455
message: Error (exit code 1)
name: exit-handler-with-param-xbh52[0].step-1(1)
outputs:
exitCode: "1"
parameters:
- name: result
value: hello world
valueFrom:
default: Foobar
path: /tmp/hello_world.txt
phase: Failed
progress: 1/1
resourcesDuration:
cpu: 5
memory: 5
startedAt: "2021-10-18T03:27:54Z"
templateName: output
templateScope: local/exit-handler-with-param-xbh52
type: Pod
exit-handler-with-param-xbh52-2034140834:
boundaryID: exit-handler-with-param-xbh52
displayName: step-1(0)
finishedAt: "2021-10-18T03:27:48Z"
hostNodeName: smile
id: exit-handler-with-param-xbh52-2034140834
message: Error (exit code 1)
name: exit-handler-with-param-xbh52[0].step-1(0)
outputs:
exitCode: "1"
parameters:
- name: result
value: hello world
valueFrom:
default: Foobar
path: /tmp/hello_world.txt
phase: Failed
progress: 1/1
resourcesDuration:
cpu: 5
memory: 5
startedAt: "2021-10-18T03:27:44Z"
templateName: output
templateScope: local/exit-handler-with-param-xbh52
type: Pod
exit-handler-with-param-xbh52-3203867295:
boundaryID: exit-handler-with-param-xbh52
children:
- exit-handler-with-param-xbh52-2034140834
- exit-handler-with-param-xbh52-1429999455
displayName: step-1
finishedAt: "2021-10-18T03:28:04Z"
id: exit-handler-with-param-xbh52-3203867295
message: No more retries left
name: exit-handler-with-param-xbh52[0].step-1
startedAt: "2021-10-18T03:27:44Z"
templateName: output
templateScope: local/exit-handler-with-param-xbh52
type: Retry
exit-handler-with-param-xbh52-3621967439:
boundaryID: exit-handler-with-param-xbh52
children:
- exit-handler-with-param-xbh52-3203867295
displayName: '[0]'
finishedAt: "2021-10-18T03:28:14Z"
id: exit-handler-with-param-xbh52-3621967439
message: child 'exit-handler-with-param-xbh52-3203867295' failed
name: exit-handler-with-param-xbh52[0]
startedAt: "2021-10-18T03:27:44Z"
templateScope: local/exit-handler-with-param-xbh52
type: StepGroup
startedAt: "2021-10-18T03:27:44Z"
`

func TestExitHandlerWithRetryNodeParam(t *testing.T) {
wf := wfv1.MustUnmarshalWorkflow(exitHandlerWithRetryNodeParam)
cancel, controller := newController(wf)
defer cancel()

ctx := context.Background()
woc := newWorkflowOperationCtx(wf, controller)

woc.operate(ctx)
retryStepNode := woc.wf.GetNodeByName("exit-handler-with-param-xbh52[0].step-1")
assert.Equal(t, 1, len(retryStepNode.Outputs.Parameters))
assert.Equal(t, "hello world", retryStepNode.Outputs.Parameters[0].Value.String())
onExitNode := woc.wf.GetNodeByName("exit-handler-with-param-xbh52[0].step-1.onExit")
assert.Equal(t, "hello world", onExitNode.Inputs.Parameters[0].Value.String())
}
2 changes: 1 addition & 1 deletion workflow/controller/steps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (woc *wfOperationCtx) executeStepGroup(ctx context.Context, stepGroup []wfv
if !childNode.Fulfilled() {
completed = false
} else if childNode.Completed() {
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, step.GetExitHook(woc.execWf.Spec.Arguments), childNode.Name, stepsCtx.boundaryID, stepsCtx.tmplCtx, "steps."+step.Name, childNode.Outputs)
hasOnExitNode, onExitNode, err := woc.runOnExitNode(ctx, step.GetExitHook(woc.execWf.Spec.Arguments), &childNode, stepsCtx.boundaryID, stepsCtx.tmplCtx, "steps."+step.Name)
if hasOnExitNode && (onExitNode == nil || !onExitNode.Fulfilled() || err != nil) {
// The onExit node is either not complete or has errored out, return.
completed = false
Expand Down

0 comments on commit 50813da

Please sign in to comment.