feat(cli): add selector and field-selector option to the stop command. #4853

noahingh · 2021-01-10T09:40:23Z

Checklist:

My organization is added to USERS.md.

I add the selector and field-selector option to the stop command, it can stop workflows that are matched by labels and field. it is related to #3962.

alexec · 2021-01-10T17:36:55Z

test/e2e/cli_test.go

@@ -784,6 +784,54 @@ func (s *CLISuite) TestWorkflowRetry() {
 		})
 }

+func (s *CLISuite) TestWorkflowStop() {


this is the first time anyone new to the project has submitted complete tests!

cmd/argo/commands/stop.go

alexec · 2021-01-10T17:40:43Z

cmd/argo/commands/stop.go

@@ -33,29 +58,55 @@ func NewStopCommand() *cobra.Command {
  argo stop @latest
 `,
 		Run: func(cmd *cobra.Command, args []string) {
+			if len(args) == 0 && !o.isList() {


it should be ok to have no args, e.g. if you were to do

kubectl get wf -o name | xargs argo stop

That should be fine, even if kubectl returned zero workflows.

What happens with both list options AND args? Should that be an error?

Is this consistent with argo list and argo delete?

When we use the xargs command it works fine, because the xargs command generates and executes lines. Indeed, kubectl get wf -o name | xargs argo stop works, but argo stop returns the help message.

At now it works only with the list option when we use both list options and args, and it is not consistent with argo delete, so I need to fix it. But in my opinion, Argo should block using both because it makes the user confused. In kubectl it returns the error when we use both list options and args like the below.

k get pods -l selector=true foo error: name cannot be provided when a selector is specified

I've fixed it to be consistent with the delete & list command.

alexec

See comments.

Signed-off-by: hanjunlee <opsdownn@gmail.com>

alexec

LGTM

alexec · 2021-01-17T18:41:23Z

@hanjunlee this is failing builds on master, so I'm going to use admin privileges to revert this merge. Can you please re-submit the PR?:

Failing build

exit status 1workflow node-suspend stopped
time="2021-01-17T18:21:53.299Z" level=fatal msg="rpc error: code = Unknown desc = currently, set only targets suspend nodes: no suspend nodes matching nodeFieldSelector: inputs.parameters.tag.value=suspend2-tag1"

    --- FAIL: TestCLIWithServerOverGRPCSuite/TestNodeSuspendResume (6.76s)
        cli_test.go:474: 
            	Error Trace:	cli_test.go:474
            	            				when.go:307
            	            				when.go:316
            	            				cli_test.go:473
            	Error:      	Received unexpected error:
            	            	exit status 1
            	Test:       	TestCLIWithServerOverGRPCSuite/TestNodeSuspendResume

=== FAIL: test/e2e TestCLIWithServerOverGRPCSuite (173.65s)
time="2021-01-17T18:21:06Z" level=info msg="config map" name=workflow-controller-configmap
time="2021-01-17T18:21:06Z" level=info msg="Creating DB session"
time="2021-01-17T18:21:06Z" level=info msg="Node status offloading config" ttl=5m0s

=== FAIL: test/e2e TestCLIWithServerOverHTTP1Suite/TestNodeSuspendResume (6.53s)
mode: HTTP1
Submitting workflow node-suspend 
Waiting 30s for workflow metadata.name=node-suspend suspended node
Condition met after 4s
../../dist/argo -n argo resume node-suspend --node-field-selector inputs.parameters.tag.value=suspend1-tag1
workflow node-suspend resumed

Waiting 30s for workflow metadata.name=node-suspend suspended node
Condition met after 1s
../../dist/argo -n argo stop node-suspend --node-field-selector inputs.parameters.tag.value=suspend2-tag1 --message because
exit status 1workflow node-suspend stopped
time="2021-01-17T18:24:42.799Z" level=fatal msg="rpc error: code = Unknown desc = currently, set only targets suspend nodes: no suspend nodes matching nodeFieldSelector: inputs.parameters.tag.value=suspend2-tag1"

    --- FAIL: TestCLIWithServerOverHTTP1Suite/TestNodeSuspendResume (6.53s)
        cli_test.go:474: 
            	Error Trace:	cli_test.go:474
            	            				when.go:307
            	            				when.go:316
            	            				cli_test.go:473
            	Error:      	Received unexpected error:
            	            	exit status 1
            	Test:       	TestCLIWithServerOverHTTP1Suite/TestNodeSuspendResume

… command. (#4853)" This reverts commit 53f7998. Signed-off-by: Alex Collins <alex_collins@intuit.com>

noahingh force-pushed the cli/stop-options branch from cde5b74 to c09acbe Compare January 10, 2021 09:40

alexec reviewed Jan 10, 2021

View reviewed changes

alexec self-assigned this Jan 11, 2021

alexec requested changes Jan 12, 2021

View reviewed changes

noahingh added 4 commits January 17, 2021 20:11

feat(cli): add selector, field-selector options to stop command.

bd6ee7f

Signed-off-by: hanjunlee <opsdownn@gmail.com>

fix(doc): fix the doc of stop command.

334958e

Signed-off-by: hanjunlee <opsdownn@gmail.com>

fix(command): fix the name of fields.

7a3775f

Signed-off-by: hanjunlee <opsdownn@gmail.com>

fix(command): fix to be constist with delete and list.

6381751

Signed-off-by: hanjunlee <opsdownn@gmail.com>

noahingh force-pushed the cli/stop-options branch from d802059 to 6381751 Compare January 17, 2021 11:11

Merge branch 'master' into cli/stop-options

8546704

noahingh requested a review from alexec January 17, 2021 11:43

alexec approved these changes Jan 17, 2021

View reviewed changes

alexec merged commit 53f7998 into argoproj:master Jan 17, 2021

alexec added this to the v3.0 milestone Jan 17, 2021

alexec added a commit that referenced this pull request Jan 17, 2021

Revert "feat(cli): add selector and field-selector option to the stop…

16f25ba

… command. (#4853)" This reverts commit 53f7998. Signed-off-by: Alex Collins <alex_collins@intuit.com>

noahingh deleted the cli/stop-options branch January 18, 2021 00:27

noahingh mentioned this pull request Jan 18, 2021

feat(cli): add selector and field-selector option to the stop command. #4898

Closed

1 task

This was referenced Jan 19, 2021

v2.12.5 cherry-pick #4904

Closed

v2.12.6 cherry-pick #4945

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): add selector and field-selector option to the stop command. #4853

feat(cli): add selector and field-selector option to the stop command. #4853

noahingh commented Jan 10, 2021

alexec Jan 10, 2021

alexec Jan 10, 2021

noahingh Jan 17, 2021 •

edited

noahingh Jan 17, 2021

alexec left a comment

alexec left a comment

alexec commented Jan 17, 2021 •

edited

feat(cli): add selector and field-selector option to the stop command. #4853

feat(cli): add selector and field-selector option to the stop command. #4853

Conversation

noahingh commented Jan 10, 2021

alexec Jan 10, 2021

Choose a reason for hiding this comment

alexec Jan 10, 2021

Choose a reason for hiding this comment

noahingh Jan 17, 2021 • edited

Choose a reason for hiding this comment

noahingh Jan 17, 2021

Choose a reason for hiding this comment

alexec left a comment

Choose a reason for hiding this comment

alexec left a comment

Choose a reason for hiding this comment

alexec commented Jan 17, 2021 • edited

noahingh Jan 17, 2021 •

edited

alexec commented Jan 17, 2021 •

edited