Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(controller): Revert prepending ExecutorScriptSourcePath which brought a breaking change in args handling #4884

Merged
merged 1 commit into from
Feb 2, 2021
Merged

fix(controller): Revert prepending ExecutorScriptSourcePath which brought a breaking change in args handling #4884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 2 additions & 11 deletions workflow/controller/operator.go
View file
Open in desktop
Expand Up @@ -2345,16 +2345,6 @@ func getStepOrDAGTaskName(nodeName string) string {
return nodeName
}

func extractMainCtrFromScriptTemplate(tmpl *wfv1.Template) apiv1.Container {
mainCtr := tmpl.Script.Container
// If script source is provided then pass all container args to the
// script instead of passing them to the container command directly
if tmpl.Script.Source != "" {
mainCtr.Args = append([]string{common.ExecutorScriptSourcePath}, mainCtr.Args...)
}
return mainCtr
}

func (woc *wfOperationCtx) executeScript(ctx context.Context, nodeName string, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
node := woc.wf.GetNodeByName(nodeName)
if node == nil {
Expand All @@ -2370,7 +2360,8 @@ func (woc *wfOperationCtx) executeScript(ctx context.Context, nodeName string, t
return node, err
}

mainCtr := extractMainCtrFromScriptTemplate(tmpl)
mainCtr := tmpl.Script.Container
mainCtr.Args = append(mainCtr.Args, common.ExecutorScriptSourcePath)
_, err = woc.createWorkflowPod(ctx, nodeName, mainCtr, tmpl, &createWorkflowPodOpts{
includeScriptOutput: includeScriptOutput,
onExitPod: opts.onExitTemplate,
Expand Down
36 changes: 0 additions & 36 deletions workflow/controller/workflowpod_test.go
View file
Open in desktop
Expand Up @@ -69,42 +69,6 @@ func TestScriptTemplateWithVolume(t *testing.T) {
assert.NoError(t, err)
}

var scriptTemplateWithArgsAndWithSource = `
name: script-with-args-and-with-source
script:
image: alpine:latest
command: [sh]
args: ["hello world"]
source: |
echo $@
`

// TestScriptTemplateMainCtrArgsWhenArgsAndWhenSource ensure order of merged args
// and script path is correct when both args and script source are specified
func TestScriptTemplateMainCtrArgsWhenArgsAndWhenSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithArgsAndWithSource)
mainCtr := extractMainCtrFromScriptTemplate(tmpl)
assert.Equal(t, []string{"sh"}, mainCtr.Command)
assert.Equal(t, []string{common.ExecutorScriptSourcePath, "hello world"}, mainCtr.Args)
}

var scriptTemplateWithArgsAndWithoutSource = `
name: script-with-args-and-without-source
script:
image: alpine:latest
command: [echo]
args: ["hello world"]
`

// TestScriptTemplateMainCtrArgsWhenArgsAndWhenNoSource ensure only args are passed
// to the resulting container when script source is empty
func TestScriptTemplateMainCtrArgsWhenArgsAndWhenNoSource(t *testing.T) {
tmpl := unmarshalTemplate(scriptTemplateWithArgsAndWithoutSource)
mainCtr := extractMainCtrFromScriptTemplate(tmpl)
assert.Equal(t, []string{"echo"}, mainCtr.Command)
assert.Equal(t, []string{"hello world"}, mainCtr.Args)
}

var scriptTemplateWithOptionalInputArtifactProvided = `
name: script-with-input-artifact
inputs:
Expand Down