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(cli): Check mutual exclusivity for argo CLI flags #3493

Merged
merged 5 commits into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
4 changes: 4 additions & 0 deletions cmd/argo/commands/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package commands

import (
"fmt"
"log"

"github.com/argoproj/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -45,6 +46,9 @@ func NewDeleteCommand() *cobra.Command {
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: flags.namespace},
})
}
if all && (flags.completed || flags.prefix != "" || flags.labels != "" || flags.finishedAfter != "") {
log.Fatal("--all cannot be used together with any of the following flags: --completed, --prefix, --selector, and --older")
}
if all || flags.completed || flags.prefix != "" || flags.labels != "" {
listed, err := listWorkflows(ctx, serviceClient, flags)
errors.CheckError(err)
Expand Down
9 changes: 6 additions & 3 deletions cmd/argo/commands/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func NewListCommand() *cobra.Command {
command.Flags().StringVar(&listArgs.prefix, "prefix", "", "Filter workflows by prefix")
command.Flags().StringVar(&listArgs.finishedAfter, "older", "", "List completed workflows finished before the specified duration (e.g. 10m, 3h, 1d)")
command.Flags().StringSliceVar(&listArgs.status, "status", []string{}, "Filter by status (comma separated)")
command.Flags().BoolVar(&listArgs.completed, "completed", false, "Show only completed workflows")
command.Flags().BoolVar(&listArgs.running, "running", false, "Show only running workflows")
command.Flags().BoolVar(&listArgs.resubmitted, "resubmitted", false, "Show only resubmitted workflows")
command.Flags().BoolVar(&listArgs.completed, "completed", false, "Show completed workflows. Mutually exclusive with --running.")
command.Flags().BoolVar(&listArgs.running, "running", false, "Show running workflows. Mutually exclusive with --completed.")
command.Flags().BoolVar(&listArgs.resubmitted, "resubmitted", false, "Show resubmitted workflows")
command.Flags().StringVarP(&listArgs.output, "output", "o", "", "Output format. One of: wide|name")
command.Flags().StringVar(&listArgs.createdSince, "since", "", "Show only workflows created after than a relative duration")
command.Flags().Int64VarP(&listArgs.chunkSize, "chunk-size", "", 0, "Return large lists in chunks rather than all at once. Pass 0 to disable.")
Expand All @@ -89,6 +89,9 @@ func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic
labelSelector = labelSelector.Add(*req)
}
}
if flags.completed && flags.running {
log.Fatal("--completed and --running cannot be used together")
}
if flags.completed {
req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.Equals, []string{"true"})
labelSelector = labelSelector.Add(*req)
Expand Down
5 changes: 5 additions & 0 deletions cmd/argo/commands/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"log"
"os"
"time"

Expand Down Expand Up @@ -67,6 +68,10 @@ func NewLogsCommand() *cobra.Command {
os.Exit(1)
}

if since > 0 && sinceTime != "" {
log.Fatal("--since-time and --since cannot be used together")
}

if since > 0 {
logOptions.SinceSeconds = pointer.Int64Ptr(int64(since.Seconds()))
}
Expand Down
6 changes: 3 additions & 3 deletions docs/cli/argo_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ argo list [flags]
```
--all-namespaces Show workflows from all namespaces
--chunk-size int Return large lists in chunks rather than all at once. Pass 0 to disable.
--completed Show only completed workflows
--completed Show completed workflows. Mutually exclusive with --running.
--field-selector string Selector (field query) to filter on, supports '=', '==', and '!='.(e.g. --field-selectorkey1=value1,key2=value2). The server only supports a limited number of field queries per type.
-h, --help help for list
--no-headers Don't print headers (default print headers).
--older string List completed workflows finished before the specified duration (e.g. 10m, 3h, 1d)
-o, --output string Output format. One of: wide|name
--prefix string Filter workflows by prefix
--resubmitted Show only resubmitted workflows
--running Show only running workflows
--resubmitted Show resubmitted workflows
--running Show running workflows. Mutually exclusive with --completed.
-l, --selector string Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)
--since string Show only workflows created after than a relative duration
--status strings Filter by status (comma separated)
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func (s *CLISuite) TestWorkflowDeleteAll() {
SubmitWorkflow().
WaitForWorkflow(30*time.Second).
Given().
RunCli([]string{"delete", "--all", "-l", "argo-e2e"}, func(t *testing.T, output string, err error) {
RunCli([]string{"delete", "--all"}, func(t *testing.T, output string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this can't be correct - all workflows created by e2e test must get this label - so this probably means there is a bug

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you clarify where is incorrect? There is only one workflow called basic at this point so deleting all would just means only this workflow gets deleted.

Copy link
Contributor

Choose a reason for hiding this comment

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

All test workflows must be annotated with argo-e2e - if it fails to delete, then it indicates that --all is not deleting all workflows labelled with argo-e2e. --all should work with -l, but does not appear to.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay I see. I've reverted the flag check in delete command. Please take another look.

if assert.NoError(t, err) {
assert.Contains(t, output, "Workflow 'basic' deleted")
}
Expand Down