fix: Node set updating global output parameter updates global. #5699 (#…

…5700)
argoproj · May 3, 2021 · 8f2acee · 8f2acee
1 parent 076ff18
commit 8f2acee
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 22 deletions.
diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -2608,31 +2608,11 @@ func (woc *wfOperationCtx) addParamToGlobalScope(param wfv1.Parameter) {
  if param.GlobalName == "" {
  return
  }
- index := -1
- if woc.wf.Status.Outputs != nil {
- for i, gParam := range woc.wf.Status.Outputs.Parameters {
- if gParam.Name == param.GlobalName {
- index = i
- break
- }
- }
- } else {
- woc.wf.Status.Outputs = &wfv1.Outputs{}
- }
  paramName := fmt.Sprintf("workflow.outputs.parameters.%s", param.GlobalName)
  woc.globalParams[paramName] = param.Value.String()
- if index == -1 {
- woc.log.Infof("setting %s: '%s'", paramName, param.Value)
- gParam := wfv1.Parameter{Name: param.GlobalName, Value: param.Value}
- woc.wf.Status.Outputs.Parameters = append(woc.wf.Status.Outputs.Parameters, gParam)
+ wfUpdated := wfutil.AddParamToGlobalScope(woc.wf, woc.log, param)
+ if wfUpdated {
  woc.updated = true
- } else {
- prevVal := *woc.wf.Status.Outputs.Parameters[index].Value
- if prevVal != *param.Value {
- woc.log.Infof("overwriting %s: '%s' -> '%s'", paramName, woc.wf.Status.Outputs.Parameters[index].Value, param.Value)
- woc.wf.Status.Outputs.Parameters[index].Value = param.Value
- woc.updated = true
- }
  }
 }
 

diff --git a/workflow/util/util.go b/workflow/util/util.go
@@ -450,6 +450,39 @@ type SetOperationValues struct {
  OutputParameters map[string]string
 }
 
+func AddParamToGlobalScope(wf *wfv1.Workflow, log *log.Entry, param wfv1.Parameter) bool {
+ wfUpdated := false
+ if param.GlobalName == "" {
+ return wfUpdated
+ }
+ index := -1
+ if wf.Status.Outputs != nil {
+ for i, gParam := range wf.Status.Outputs.Parameters {
+ if gParam.Name == param.GlobalName {
+ index = i
+ break
+ }
+ }
+ } else {
+ wf.Status.Outputs = &wfv1.Outputs{}
+ }
+ paramName := fmt.Sprintf("workflow.outputs.parameters.%s", param.GlobalName)
+ if index == -1 {
+ log.Infof("setting %s: '%s'", paramName, param.Value)
+ gParam := wfv1.Parameter{Name: param.GlobalName, Value: param.Value}
+ wf.Status.Outputs.Parameters = append(wf.Status.Outputs.Parameters, gParam)
+ wfUpdated = true
+ } else {
+ prevVal := *wf.Status.Outputs.Parameters[index].Value
+ if prevVal != *param.Value {
+ log.Infof("overwriting %s: '%s' -> '%s'", paramName, wf.Status.Outputs.Parameters[index].Value, param.Value)
+ wf.Status.Outputs.Parameters[index].Value = param.Value
+ wfUpdated = true
+ }
+ }
+ return wfUpdated
+}
+
 func updateSuspendedNode(ctx context.Context, wfIf v1alpha1.WorkflowInterface, hydrator hydrator.Interface, workflowName string, nodeFieldSelector string, values SetOperationValues) error {
  selector, err := fields.ParseSelector(nodeFieldSelector)
  if err != nil {
@@ -502,6 +535,7 @@ func updateSuspendedNode(ctx context.Context, wfIf v1alpha1.WorkflowInterface, h
  node.Outputs.Parameters[i].ValueFrom = nil
  nodeUpdated = true
  hit = true
+ AddParamToGlobalScope(wf, log.NewEntry(log.StandardLogger()), node.Outputs.Parameters[i])
  break
  }
  }

diff --git a/workflow/util/util_test.go b/workflow/util/util_test.go
@@ -410,6 +410,7 @@ status:
  valueFrom:
  supplied: {}
  - name: message2
+ globalName: message-global-param
  valueFrom:
  supplied: {}
  phase: Running
@@ -438,6 +439,11 @@ func TestUpdateSuspendedNode(t *testing.T) {
  assert.NoError(t, err)
  err = updateSuspendedNode(ctx, wfIf, hydratorfake.Noop, "suspend-template", "name=suspend-template-kgfn7[0].approve", SetOperationValues{OutputParameters: map[string]string{"message2": "Hello World 2"}})
  assert.NoError(t, err)
+
+ //make sure global variable was updated
+ wf, err := wfIf.Get(ctx, "suspend-template", metav1.GetOptions{})
+ assert.NoError(t, err)
+ assert.Equal(t, "Hello World 2", wf.Status.Outputs.Parameters[0].Value.String())
  }
 
  noSpaceWf := wfv1.MustUnmarshalWorkflow(susWorkflow)