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.

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

fix: Fix template scope #1836

Merged
merged 2 commits into from
Dec 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions workflow/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ func unmarshalWF(yamlStr string) *wfv1.Workflow {
return &wf
}

func unmarshalWFTmpl(yamlStr string) *wfv1.WorkflowTemplate {
var wftmpl wfv1.WorkflowTemplate
err := yaml.Unmarshal([]byte(yamlStr), &wftmpl)
if err != nil {
panic(err)
}
return &wftmpl
}

// makePodsRunning acts like a pod controller and simulates the transition of pods transitioning into a running state
func makePodsRunning(t *testing.T, kubeclientset kubernetes.Interface, namespace string) {
podcs := kubeclientset.CoreV1().Pods(namespace)
Expand Down
11 changes: 7 additions & 4 deletions workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1203,6 +1203,9 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
return node, ErrDeadlineExceeded
}

// Set templateScope from which the template resolution starts.
templateScope := tmplCtx.GetCurrentTemplateBase().GetTemplateScope()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests?


newTmplCtx, resolvedTmpl, err := tmplCtx.ResolveTemplate(orgTmpl)
if err != nil {
return woc.initializeNodeOrMarkError(node, nodeName, wfv1.NodeTypeSkipped, orgTmpl, boundaryID, err), err
Expand Down Expand Up @@ -1235,7 +1238,7 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
retryParentNode := node
if retryParentNode == nil {
woc.log.Debugf("Inject a retry node for node %s", retryNodeName)
retryParentNode = woc.initializeExecutableNode(retryNodeName, wfv1.NodeTypeRetry, newTmplCtx, processedTmpl, orgTmpl, boundaryID, wfv1.NodeRunning)
retryParentNode = woc.initializeExecutableNode(retryNodeName, wfv1.NodeTypeRetry, templateScope, processedTmpl, orgTmpl, boundaryID, wfv1.NodeRunning)
}
processedRetryParentNode, continueExecution, err := woc.processNodeRetries(retryParentNode, *processedTmpl.RetryStrategy)
if err != nil {
Expand Down Expand Up @@ -1288,7 +1291,7 @@ func (woc *wfOperationCtx) executeTemplate(nodeName string, orgTmpl wfv1.Templat
err := errors.InternalErrorf("Template '%s' has unknown node type", processedTmpl.Name)
return woc.initializeNode(nodeName, wfv1.NodeTypeSkipped, orgTmpl, boundaryID, wfv1.NodeError, err.Error()), err
}
node = woc.initializeExecutableNode(nodeName, nodeType, newTmplCtx, processedTmpl, orgTmpl, boundaryID, wfv1.NodePending)
node = woc.initializeExecutableNode(nodeName, nodeType, templateScope, processedTmpl, orgTmpl, boundaryID, wfv1.NodePending)
}

switch processedTmpl.GetType() {
Expand Down Expand Up @@ -1401,15 +1404,15 @@ func (woc *wfOperationCtx) markWorkflowError(err error, markCompleted bool) {
var stepsOrDagSeparator = regexp.MustCompile(`^(\[\d+\])?\.`)

// initializeExecutableNode initializes a node and stores the template.
func (woc *wfOperationCtx) initializeExecutableNode(nodeName string, nodeType wfv1.NodeType, tmplCtx *templateresolution.Context, executeTmpl *wfv1.Template, orgTmpl wfv1.TemplateHolder, boundaryID string, phase wfv1.NodePhase, messages ...string) *wfv1.NodeStatus {
func (woc *wfOperationCtx) initializeExecutableNode(nodeName string, nodeType wfv1.NodeType, templateScope string, executeTmpl *wfv1.Template, orgTmpl wfv1.TemplateHolder, boundaryID string, phase wfv1.NodePhase, messages ...string) *wfv1.NodeStatus {
node := woc.initializeNode(nodeName, nodeType, orgTmpl, boundaryID, phase)

// Set the input values to the node.
if executeTmpl.Inputs.HasInputs() {
node.Inputs = executeTmpl.Inputs.DeepCopy()
}

node.TemplateScope = tmplCtx.GetCurrentTemplateBase().GetTemplateScope()
node.TemplateScope = templateScope

// Update the node
woc.wf.Status.Nodes[node.ID] = *node
Expand Down
117 changes: 117 additions & 0 deletions workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1846,3 +1846,120 @@ func TestWithParamAsJsonList(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 4, len(pods.Items))
}

var testTemplateScopeWorkflowYaml = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
name: test-template-scope
spec:
entrypoint: entry
templates:
- name: entry
templateRef:
name: test-template-scope-1
template: steps
`

var testTemplateScopeWorkflowTemplateYaml1 = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: test-template-scope-1
spec:
templates:
- name: steps
steps:
- - name: hello
template: hello
- name: other-wftmpl
templateRef:
name: test-template-scope-2
template: steps
- name: hello
script:
image: python:alpine3.6
command: [python]
source: |
print("hello world")
`

var testTemplateScopeWorkflowTemplateYaml2 = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: test-template-scope-2
spec:
templates:
- name: steps
steps:
- - name: hello
template: hello
- name: hello
script:
image: python:alpine3.6
command: [python]
source: |
print("hello world")
`

func TestTemplateScope(t *testing.T) {
controller := newController()
wfcset := controller.wfclientset.ArgoprojV1alpha1().Workflows("")
wfctmplset := controller.wfclientset.ArgoprojV1alpha1().WorkflowTemplates("")

wf := unmarshalWF(testTemplateScopeWorkflowYaml)
_, err := wfcset.Create(wf)
assert.NoError(t, err)
wftmpl := unmarshalWFTmpl(testTemplateScopeWorkflowTemplateYaml1)
_, err = wfctmplset.Create(wftmpl)
assert.NoError(t, err)
wftmpl = unmarshalWFTmpl(testTemplateScopeWorkflowTemplateYaml2)
_, err = wfctmplset.Create(wftmpl)
assert.NoError(t, err)

woc := newWorkflowOperationCtx(wf, controller)
woc.operate()

wf, err = wfcset.Get(wf.Name, metav1.GetOptions{})
assert.NoError(t, err)

node := findNodeByName(wf.Status.Nodes, "test-template-scope")
if assert.NotNil(t, node, "Node %s not found", "test-templte-scope") {
assert.Equal(t, "", node.TemplateScope)
}

node = findNodeByName(wf.Status.Nodes, "test-template-scope[0]")
if assert.NotNil(t, node, "Node %s not found", "test-templte-scope[0]") {
assert.Equal(t, "", node.TemplateScope)
}

node = findNodeByName(wf.Status.Nodes, "test-template-scope[0].hello")
if assert.NotNil(t, node, "Node %s not found", "test-templte-scope[0].hello") {
assert.Equal(t, "test-template-scope-1", node.TemplateScope)
}

node = findNodeByName(wf.Status.Nodes, "test-template-scope[0].other-wftmpl")
if assert.NotNil(t, node, "Node %s not found", "test-template-scope[0].other-wftmpl") {
assert.Equal(t, "test-template-scope-1", node.TemplateScope)
}

node = findNodeByName(wf.Status.Nodes, "test-template-scope[0].other-wftmpl[0]")
if assert.NotNil(t, node, "Node %s not found", "test-template-scope[0].other-wftmpl[0]") {
assert.Equal(t, "", node.TemplateScope)
}

node = findNodeByName(wf.Status.Nodes, "test-template-scope[0].other-wftmpl[0].hello")
if assert.NotNil(t, node, "Node %s not found", "test-template-scope[0].other-wftmpl[0].hello") {
assert.Equal(t, "test-template-scope-2", node.TemplateScope)
}
}

func findNodeByName(nodes map[string]wfv1.NodeStatus, name string) *wfv1.NodeStatus {
for _, node := range nodes {
if node.Name == name {
return &node
}
}
return nil
}