Skip to content

Commit

Permalink
fix: avoid exit handler nil pointer when missing outputs. Fixes argop…
Browse files Browse the repository at this point in the history
…roj#13445 (argoproj#13448)

Signed-off-by: Miltiadis Alexis <alexmiltiadis@gmail.com>
  • Loading branch information
miltalex authored and Joibel committed Sep 19, 2024
1 parent 488660e commit 442266d
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 11 deletions.
39 changes: 39 additions & 0 deletions test/e2e/expectedfailures/exit-handler-fail-missing-output.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: exit-handler-fail-missing-output-
labels:
test: test
spec:
entrypoint: failure-workflow
templates:
- name: failure-workflow
steps:
- - name: step1
template: intentional-fail
hooks:
exit:
template: lifecycle-hook
arguments:
parameters:
- name: hello-param
value: '{{steps.step1.outputs.parameters.hello-param}}'
- name: intentional-fail
outputs:
parameters:
- name: hello-param
valueFrom:
path: /tmp/hello_world.txt
container:
image: alpine:latest
command: ["sh", "-c"]
args: ["echo intentional failure; exit 1"]
- name: lifecycle-hook
inputs:
parameters:
- name: hello-param
container:
image: busybox
command: [echo]
args: ["Hello param: {{inputs.parameters.hello-param}}"]
42 changes: 32 additions & 10 deletions test/e2e/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ func (s *FunctionalSuite) TestWhenExpressions() {
}

func (s *FunctionalSuite) TestJSONVariables() {

s.Given().
Workflow("@testdata/json-variables.yaml").
When().
Expand Down Expand Up @@ -1053,9 +1052,7 @@ spec:
}
}
assert.True(t, memoHit)

})

}

func (s *FunctionalSuite) TestStepLevelMemoizeNoOutput() {
Expand Down Expand Up @@ -1113,9 +1110,7 @@ spec:
}
}
assert.True(t, memoHit)

})

}

func (s *FunctionalSuite) TestDAGLevelMemoize() {
Expand Down Expand Up @@ -1179,9 +1174,7 @@ spec:
}
}
assert.True(t, memoHit)

})

}

func (s *FunctionalSuite) TestDAGLevelMemoizeNoOutput() {
Expand Down Expand Up @@ -1240,9 +1233,7 @@ spec:
}
}
assert.True(t, memoHit)

})

}

func (s *FunctionalSuite) TestContainerSetRetryFail() {
Expand Down Expand Up @@ -1323,7 +1314,6 @@ func (s *FunctionalSuite) TestEntrypointName() {
assert.Equal(t, wfv1.NodeSucceeded, n.Phase)
assert.Equal(t, "bar", n.Inputs.Parameters[0].Value.String())
})

}

func (s *FunctionalSuite) TestMissingStepsInUI() {
Expand Down Expand Up @@ -1372,3 +1362,35 @@ func (s *FunctionalSuite) TestTerminateWorkflowWhileOnExitHandlerRunning() {
assert.Equal(t, status.Phase, wfv1.WorkflowFailed)
})
}

// Exit handler ensure when failed steps ensure no crash and output parameter
func (s *FunctionalSuite) TestWorkflowExitHandlerCrashEnsureNodeIsPresent() {
s.Given().
Workflow("@expectedfailures/exit-handler-fail-missing-output.yaml").
When().
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeRunning).
WaitForWorkflow(fixtures.ToBeFailed).
Then().
ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *wfv1.WorkflowStatus) {
var hasExitNode bool
var exitNodeName string

for _, node := range status.Nodes {
if !node.IsExitNode() {
continue
}
hasExitNode = true
exitNodeName = node.DisplayName
}
assert.True(t, hasExitNode)
assert.NotEmpty(t, exitNodeName)

hookNode := status.Nodes.FindByDisplayName(exitNodeName)

require.NotNil(t, hookNode)
assert.NotNil(t, hookNode.Inputs)
require.Len(t, hookNode.Inputs.Parameters, 1)
assert.NotNil(t, hookNode.Inputs.Parameters[0].Value)
})
}
6 changes: 5 additions & 1 deletion workflow/controller/exit_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ func (woc *wfOperationCtx) resolveExitTmplArgument(args wfv1.Arguments, prefix s
}
if outputs != nil {
for _, param := range outputs.Parameters {
scope.addParamToScope(fmt.Sprintf("%s.outputs.parameters.%s", prefix, param.Name), param.Value.String())
value := ""
if param.Value != nil {
value = param.Value.String()
}
scope.addParamToScope(fmt.Sprintf("%s.outputs.parameters.%s", prefix, param.Name), value)
}
for _, arts := range outputs.Artifacts {
scope.addArtifactToScope(fmt.Sprintf("%s.outputs.artifacts.%s", prefix, arts.Name), arts)
Expand Down

0 comments on commit 442266d

Please sign in to comment.