fix(cli): Allow full node name in node-field-selector (#4913)

Signed-off-by: Simon Behar <simbeh7@gmail.com>
argoproj · Jan 26, 2021 · 7c69898 · 7c69898
1 parent c729306
commit 7c69898
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 6 deletions.
diff --git a/docs/node-field-selector.md b/docs/node-field-selector.md
@@ -22,12 +22,13 @@ The field can be any of:
 
 | Field | Description|
 |----------|------------|
-| displayName | Display name of the node |
-| templateName | Template name of the node |
-| phase | Phase status of the node - eg Running |
-| templateRef.name | The name of the WorkflowTemplate the node is referring to |
-| templateRef.template | The template within the WorkflowTemplate the node is referring to |
-| inputs.parameters.<NAME>.value | The value of input parameter NAME |
+| `displayName`| Display name of the node. This is the name of the node as it is displayed on the CLI or UI, without considering its ancestors (see example below). This is a useful shortcut if there is only one node with the same `displayName` |
+| `name`| Full name of the node. This is the full name of the node, including its ancestors (see example below). Using `name` is necessary when two or more nodes share the same `displayName` and disambiguation is required. |
+| `templateName`| Template name of the node |
+| `phase`| Phase status of the node - eg Running |
+| `templateRef.name`| The name of the WorkflowTemplate the node is referring to |
+| `templateRef.template`| The template within the WorkflowTemplate the node is referring to |
+| `inputs.parameters.<NAME>.value`| The value of input parameter NAME |
 
 The operator can be '=' or '!='. Multiple selectors can be combined with a comma, in which case they are ANDed together.
 
@@ -40,3 +41,44 @@ To filter for nodes where the input parameter 'foo' is equal to 'bar':
 To filter for nodes where the input parameter 'foo' is equal to 'bar' and phase is not running:
 
 ```--node-field-selector=foo1=bar1,phase!=Running```
+
+Consider the following workflow:
+
+```
+ ● appr-promotion-ffsv4    code-release
+ ├─✔ start                 sample-template/email                 appr-promotion-ffsv4-3704914002  2s
+ ├─● app1                  wftempl1/approval-and-promotion
+ │ ├─✔ notification-email  sample-template/email                 appr-promotion-ffsv4-524476380   2s
+ │ └─ǁ wait-approval       sample-template/waiting-for-approval
+ ├─✔ app2                  wftempl2/promotion
+ │ ├─✔ notification-email  sample-template/email                 appr-promotion-ffsv4-2580536603  2s
+ │ ├─✔ pr-approval         sample-template/approval              appr-promotion-ffsv4-3445567645  2s
+ │ └─✔ deployment          sample-template/promote               appr-promotion-ffsv4-970728982   1s
+ └─● app3                  wftempl1/approval-and-promotion
+   ├─✔ notification-email  sample-template/email                 appr-promotion-ffsv4-388318034   2s
+   └─ǁ wait-approval       sample-template/waiting-for-approval
+```
+
+Here we have two steps with the same `displayName`: `wait-approval`. To select one to suspend, we need to use their
+`name`, either `appr-promotion-ffsv4.app1.wait-approval` or `appr-promotion-ffsv4.app2.wait-approval`. If it is not clear
+what the full name of a done is, it can be found using `kubectl`:
+
+```
+$ kubectl get wf appr-promotion-ffsv4 -o yaml
+
+...
+    appr-promotion-ffsv4-3235686597:
+      boundaryID: appr-promotion-ffsv4-3079407832
+      displayName: wait-approval                        # <- Display Name
+      finishedAt: null
+      id: appr-promotion-ffsv4-3235686597
+      name: appr-promotion-ffsv4.app1.wait-approval     # <- Full Name
+      phase: Running
+      startedAt: "2021-01-20T17:00:25Z"
+      templateRef:
+        name: sample-template
+        template: waiting-for-approval
+      templateScope: namespaced/wftempl1
+      type: Suspend
+...
+```
diff --git a/workflow/util/util.go b/workflow/util/util.go
@@ -416,6 +416,7 @@ func SelectorMatchesNode(selector fields.Selector, node wfv1.NodeStatus) bool {
 		"displayName":  node.DisplayName,
 		"templateName": node.TemplateName,
 		"phase":        string(node.Phase),
+		"name":         node.Name,
 	}
 	if node.TemplateRef != nil {
 		nodeFields["templateRef.name"] = node.TemplateRef.Name

diff --git a/workflow/util/util_test.go b/workflow/util/util_test.go
@@ -431,6 +431,9 @@ status:
         - name: message
           valueFrom:
             supplied: {}
+        - name: message2
+          valueFrom:
+            supplied: {}
       phase: Running
       startedAt: "2020-06-25T18:01:56Z"
       templateName: approve
@@ -455,6 +458,8 @@ func TestUpdateSuspendedNode(t *testing.T) {
 		assert.EqualError(t, err, "node is not expecting output parameter 'does-not-exist'")
 		err = updateSuspendedNode(ctx, wfIf, hydratorfake.Noop, "suspend-template", "displayName=approve", SetOperationValues{OutputParameters: map[string]string{"message": "Hello World"}})
 		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)
 	}
 
 	noSpaceWf := unmarshalWF(susWorkflow)