fix(cli): Only list needed fields. Fixes argoproj#6000 (argoproj#6298)

* fix(cli): Only list needed fields Signed-off-by: Alex Collins <[email protected]> * ok Signed-off-by: Alex Collins <[email protected]>
CashWilliams · Jul 9, 2021 · c2360c4 · c2360c4
1 parent 4de976c
commit c2360c4
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 15 deletions.
diff --git 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
@@ -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_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_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: defaultFields,
  }
 
  wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{
@@ -83,6 +84,7 @@ func Test_retryWorkflows(t *testing.T) {
  ListOptions: &metav1.ListOptions{
  LabelSelector: retryOpts.labelSelector,
  },
+ Fields: defaultFields,
  }
 
  wfList := &wfv1.WorkflowList{Items: wfv1.Workflows{

diff --git 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: defaultFields,
  }
 
  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: 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
@@ -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.

diff --git 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) {