From bdd2f12ce6ccb2e81147ee9ff398897f0dbb8ae4 Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Wed, 7 Jul 2021 09:47:52 -0700 Subject: [PATCH 1/2] fix(cli): Only list needed fields Signed-off-by: Alex Collins --- cmd/argo/commands/list.go | 24 +++++++++++-- cmd/argo/commands/list_test.go | 4 +-- cmd/argo/commands/resubmit.go | 1 + cmd/argo/commands/retry.go | 1 + cmd/argo/commands/retry_test.go | 2 ++ cmd/argo/commands/stop.go | 1 + cmd/argo/commands/stop_test.go | 2 ++ test/e2e/cli_test.go | 61 +++++++++++++++++++++++++++------ 8 files changed, 82 insertions(+), 14 deletions(-) diff --git a/cmd/argo/commands/list.go b/cmd/argo/commands/list.go index d5cb4c8c6889..f5a7f7c3db89 100644 --- a/cmd/argo/commands/list.go +++ b/cmd/argo/commands/list.go @@ -37,6 +37,22 @@ type listFlags struct { fields string } +var ( + nameFields = "metadata,items.metadata.name" + defaultFields = "metadata,items.metadata,items.spec,items.status.phase,items.status.message,items.status.finishedAt,items.status.startedAt,items.status.estimatedDuration,items.status.progress" +) + +func (f listFlags) displayFields() string { + switch f.output { + case "name": + return nameFields + case "json", "yaml": + return "" + default: + return defaultFields + } +} + func NewListCommand() *cobra.Command { var ( listArgs listFlags @@ -68,7 +84,7 @@ func NewListCommand() *cobra.Command { 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().StringVarP(&listArgs.output, "output", "o", "", "Output format. One of: name|wide|yaml|json") 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.") command.Flags().BoolVar(&listArgs.noHeaders, "no-headers", false, "Don't print headers (default print headers).") @@ -109,7 +125,11 @@ func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic var workflows wfv1.Workflows for { log.WithField("listOpts", listOpts).Debug() - wfList, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{Namespace: flags.namespace, ListOptions: listOpts}) + wfList, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{ + Namespace: flags.namespace, + ListOptions: listOpts, + Fields: flags.displayFields(), + }) if err != nil { return nil, err } diff --git a/cmd/argo/commands/list_test.go b/cmd/argo/commands/list_test.go index f6339ad28add..408d4ee73ee3 100644 --- a/cmd/argo/commands/list_test.go +++ b/cmd/argo/commands/list_test.go @@ -80,7 +80,7 @@ func Test_listWorkflows(t *testing.T) { func list(listOptions *metav1.ListOptions, flags listFlags) (wfv1.Workflows, error) { c := &workflowmocks.WorkflowServiceClient{} - c.On("ListWorkflows", mock.Anything, &workflow.WorkflowListRequest{ListOptions: listOptions}).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{ + c.On("ListWorkflows", mock.Anything, &workflow.WorkflowListRequest{ListOptions: listOptions, Fields: defaultFields}).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{ {ObjectMeta: metav1.ObjectMeta{Name: "foo-", CreationTimestamp: metav1.Time{Time: time.Now().Add(-2 * time.Hour)}}, Status: wfv1.WorkflowStatus{FinishedAt: metav1.Time{Time: time.Now().Add(-2 * time.Hour)}}}, {ObjectMeta: metav1.ObjectMeta{Name: "bar-", CreationTimestamp: metav1.Time{Time: time.Now()}}}, {ObjectMeta: metav1.ObjectMeta{ @@ -95,7 +95,7 @@ func list(listOptions *metav1.ListOptions, flags listFlags) (wfv1.Workflows, err func listEmpty(listOptions *metav1.ListOptions, flags listFlags) (wfv1.Workflows, error) { c := &workflowmocks.WorkflowServiceClient{} - c.On("ListWorkflows", mock.Anything, &workflow.WorkflowListRequest{ListOptions: listOptions}).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{}}, nil) + c.On("ListWorkflows", mock.Anything, &workflow.WorkflowListRequest{ListOptions: listOptions, Fields: defaultFields}).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{}}, nil) workflows, err := listWorkflows(context.Background(), c, flags) return workflows, err } diff --git a/cmd/argo/commands/resubmit.go b/cmd/argo/commands/resubmit.go index 20a437fda33e..9e68277728a1 100644 --- a/cmd/argo/commands/resubmit.go +++ b/cmd/argo/commands/resubmit.go @@ -104,6 +104,7 @@ func resubmitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowSe namespace: resubmitOpts.namespace, fields: resubmitOpts.fieldSelector, labels: resubmitOpts.labelSelector, + output: "json", }) if err != nil { return err diff --git a/cmd/argo/commands/retry.go b/cmd/argo/commands/retry.go index e35e62d9252e..033fe5cfbdd9 100644 --- a/cmd/argo/commands/retry.go +++ b/cmd/argo/commands/retry.go @@ -108,6 +108,7 @@ func retryWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServi namespace: retryOpts.namespace, fields: retryOpts.fieldSelector, labels: retryOpts.labelSelector, + output: "name", }) if err != nil { return err diff --git a/cmd/argo/commands/retry_test.go b/cmd/argo/commands/retry_test.go index 0bf230e46ad4..99a79b9164fd 100644 --- a/cmd/argo/commands/retry_test.go +++ b/cmd/argo/commands/retry_test.go @@ -43,6 +43,7 @@ func Test_retryWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: retryOpts.labelSelector, }, + Fields: nameFields, } wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{ @@ -83,6 +84,7 @@ func Test_retryWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: retryOpts.labelSelector, }, + Fields: nameFields, } wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{ diff --git a/cmd/argo/commands/stop.go b/cmd/argo/commands/stop.go index 34821a373fd1..7a929bb5d5e0 100644 --- a/cmd/argo/commands/stop.go +++ b/cmd/argo/commands/stop.go @@ -87,6 +87,7 @@ func stopWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic namespace: stopArgs.namespace, fields: stopArgs.fieldSelector, labels: stopArgs.labelSelector, + output: "name", }) if err != nil { return err diff --git a/cmd/argo/commands/stop_test.go b/cmd/argo/commands/stop_test.go index 81943edc33a4..843613c14c77 100644 --- a/cmd/argo/commands/stop_test.go +++ b/cmd/argo/commands/stop_test.go @@ -53,6 +53,7 @@ func Test_stopWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: stopArgs.labelSelector, }, + Fields: nameFields, } c.On("ListWorkflows", mock.Anything, wfListReq).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{ @@ -81,6 +82,7 @@ func Test_stopWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: stopArgs.labelSelector, }, + Fields: nameFields, } c.On("ListWorkflows", mock.Anything, wfListReq).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{ diff --git a/test/e2e/cli_test.go b/test/e2e/cli_test.go index 3f201e8a2004..5709d5f0baa2 100644 --- a/test/e2e/cli_test.go +++ b/test/e2e/cli_test.go @@ -3,6 +3,7 @@ package e2e import ( + "encoding/json" "fmt" "io/ioutil" "os" @@ -11,6 +12,8 @@ import ( "testing" "time" + "sigs.k8s.io/yaml" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -383,16 +386,54 @@ func (s *CLISuite) TestRoot() { }) }) s.Run("List", func() { - s.Given(). - RunCli([]string{"list"}, func(t *testing.T, output string, err error) { - if assert.NoError(t, err) { - assert.Contains(t, output, "NAME") - assert.Contains(t, output, "STATUS") - assert.Contains(t, output, "AGE") - assert.Contains(t, output, "DURATION") - assert.Contains(t, output, "PRIORITY") - } - }) + s.Run("DefaultOutput", func() { + s.Given(). + RunCli([]string{"list"}, func(t *testing.T, output string, err error) { + if assert.NoError(t, err) { + assert.Contains(t, output, "NAME") + assert.Contains(t, output, "STATUS") + assert.Contains(t, output, "AGE") + assert.Contains(t, output, "DURATION") + assert.Contains(t, output, "PRIORITY") + } + }) + }) + s.Run("NameOutput", func() { + s.Given(). + RunCli([]string{"list", "-o", "name"}, func(t *testing.T, output string, err error) { + if assert.NoError(t, err) { + assert.NotContains(t, output, "NAME") + } + }) + }) + s.Run("WideOutput", func() { + s.Given(). + RunCli([]string{"list", "-o", "wide"}, func(t *testing.T, output string, err error) { + if assert.NoError(t, err) { + assert.Contains(t, output, "PARAMETERS") + } + }) + }) + s.Run("JSONOutput", func() { + s.Given(). + RunCli([]string{"list", "-o", "json"}, func(t *testing.T, output string, err error) { + if assert.NoError(t, err) { + list := wfv1.Workflows{} + assert.NoError(t, json.Unmarshal([]byte(output), &list)) + assert.Len(t, list, 1) + } + }) + }) + s.Run("YAMLOutput", func() { + s.Given(). + RunCli([]string{"list", "-o", "yaml"}, func(t *testing.T, output string, err error) { + if assert.NoError(t, err) { + list := wfv1.Workflows{} + assert.NoError(t, yaml.UnmarshalStrict([]byte(output), &list)) + assert.Len(t, list, 1) + } + }) + }) }) s.Run("Get", func() { s.Given().RunCli([]string{"get", "@latest"}, func(t *testing.T, output string, err error) { From 36cf3b5ef5c4ee96526c7b51f61d5cb6f911d628 Mon Sep 17 00:00:00 2001 From: Alex Collins Date: Wed, 7 Jul 2021 10:25:36 -0700 Subject: [PATCH 2/2] ok Signed-off-by: Alex Collins --- cmd/argo/commands/resubmit.go | 1 - cmd/argo/commands/resubmit_test.go | 2 ++ cmd/argo/commands/retry.go | 1 - cmd/argo/commands/retry_test.go | 4 ++-- cmd/argo/commands/stop.go | 1 - cmd/argo/commands/stop_test.go | 4 ++-- docs/cli/argo_list.md | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/argo/commands/resubmit.go b/cmd/argo/commands/resubmit.go index 9e68277728a1..20a437fda33e 100644 --- a/cmd/argo/commands/resubmit.go +++ b/cmd/argo/commands/resubmit.go @@ -104,7 +104,6 @@ func resubmitWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowSe namespace: resubmitOpts.namespace, fields: resubmitOpts.fieldSelector, labels: resubmitOpts.labelSelector, - output: "json", }) if err != nil { return err diff --git a/cmd/argo/commands/resubmit_test.go b/cmd/argo/commands/resubmit_test.go index 42783e165d55..f0d2b69ba62f 100644 --- a/cmd/argo/commands/resubmit_test.go +++ b/cmd/argo/commands/resubmit_test.go @@ -64,6 +64,7 @@ func Test_resubmitWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: resubmitOpts.labelSelector, }, + Fields: defaultFields, } wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{ @@ -103,6 +104,7 @@ func Test_resubmitWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: resubmitOpts.labelSelector, }, + Fields: defaultFields, } wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{ diff --git a/cmd/argo/commands/retry.go b/cmd/argo/commands/retry.go index 033fe5cfbdd9..e35e62d9252e 100644 --- a/cmd/argo/commands/retry.go +++ b/cmd/argo/commands/retry.go @@ -108,7 +108,6 @@ func retryWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServi namespace: retryOpts.namespace, fields: retryOpts.fieldSelector, labels: retryOpts.labelSelector, - output: "name", }) if err != nil { return err diff --git a/cmd/argo/commands/retry_test.go b/cmd/argo/commands/retry_test.go index 99a79b9164fd..6fdddbe38edd 100644 --- a/cmd/argo/commands/retry_test.go +++ b/cmd/argo/commands/retry_test.go @@ -43,7 +43,7 @@ func Test_retryWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: retryOpts.labelSelector, }, - Fields: nameFields, + Fields: defaultFields, } wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{ @@ -84,7 +84,7 @@ func Test_retryWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: retryOpts.labelSelector, }, - Fields: nameFields, + Fields: defaultFields, } wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{ diff --git a/cmd/argo/commands/stop.go b/cmd/argo/commands/stop.go index 7a929bb5d5e0..34821a373fd1 100644 --- a/cmd/argo/commands/stop.go +++ b/cmd/argo/commands/stop.go @@ -87,7 +87,6 @@ func stopWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServic namespace: stopArgs.namespace, fields: stopArgs.fieldSelector, labels: stopArgs.labelSelector, - output: "name", }) if err != nil { return err diff --git a/cmd/argo/commands/stop_test.go b/cmd/argo/commands/stop_test.go index 843613c14c77..97a0773951f9 100644 --- a/cmd/argo/commands/stop_test.go +++ b/cmd/argo/commands/stop_test.go @@ -53,7 +53,7 @@ func Test_stopWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: stopArgs.labelSelector, }, - Fields: nameFields, + Fields: defaultFields, } c.On("ListWorkflows", mock.Anything, wfListReq).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{ @@ -82,7 +82,7 @@ func Test_stopWorkflows(t *testing.T) { ListOptions: &metav1.ListOptions{ LabelSelector: stopArgs.labelSelector, }, - Fields: nameFields, + Fields: defaultFields, } c.On("ListWorkflows", mock.Anything, wfListReq).Return(&wfv1.WorkflowList{Items: wfv1.Workflows{ diff --git a/docs/cli/argo_list.md b/docs/cli/argo_list.md index 7566a15d05bc..107eb5a8ae12 100644 --- a/docs/cli/argo_list.md +++ b/docs/cli/argo_list.md @@ -20,7 +20,7 @@ argo list [flags] -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 + -o, --output string Output format. One of: name|wide|yaml|json --prefix string Filter workflows by prefix --resubmitted Show resubmitted workflows --running Show running workflows. Mutually exclusive with --completed.