feat(cli): More argo list and argo delete options (argoproj#3117)

cmd/argo/commands/delete.go

@@ -2,88 +2,68 @@ package commands
 
 import (
 	"fmt"
-	"time"
 
 	"github.com/argoproj/pkg/errors"
-	argotime "github.com/argoproj/pkg/time"
 	"github.com/spf13/cobra"
-	apierr "k8s.io/apimachinery/pkg/api/errors"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/argoproj/argo/cmd/argo/commands/client"
 	workflowpkg "github.com/argoproj/argo/pkg/apiclient/workflow"
-	"github.com/argoproj/argo/workflow/common"
-)
-
-var (
-	completedLabelSelector = fmt.Sprintf("%s=true", common.LabelKeyCompleted)
+	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 )
 
 // NewDeleteCommand returns a new instance of an `argo delete` command
 func NewDeleteCommand() *cobra.Command {
 	var (
-		selector  string
-		all       bool
-		completed bool
-		older     string
+		flags         listFlags
+		all           bool
+		allNamespaces bool
+		dryRun        bool
 	)
-
 	var command = &cobra.Command{
-		Use: "delete WORKFLOW...",
+		Use:   "delete [--dry-run] [WORKFLOW...|[--all] [--older] [--completed] [--prefix PREFIX] [--selector SELECTOR]]",
+		Short: "delete workflows",
 		Run: func(cmd *cobra.Command, args []string) {
 			ctx, apiClient := client.NewAPIClient()
 			serviceClient := apiClient.NewWorkflowServiceClient()
-			namespace := client.Namespace()
-			var workflowsToDelete []metav1.ObjectMeta
+			var workflows wfv1.Workflows
+			if !allNamespaces {
+				flags.namespace = client.Namespace()
+			}
 			for _, name := range args {
-				workflowsToDelete = append(workflowsToDelete, metav1.ObjectMeta{
-					Name:      name,
-					Namespace: namespace,
+				workflows = append(workflows, wfv1.Workflow{
+					ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: flags.namespace},
 				})
 			}
-			if all || completed || older != "" {
-				// all is effectively the default, completed takes precedence over all
-				if completed {
-					if selector != "" {
-						selector = selector + "," + completedLabelSelector
-					} else {
-						selector = completedLabelSelector
-					}
-				}
-				// you can mix older with either of these
-				var olderTime *time.Time
-				if older != "" {
-					var err error
-					olderTime, err = argotime.ParseSince(older)
-					errors.CheckError(err)
-				}
-				list, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{
-					Namespace:   namespace,
-					ListOptions: &metav1.ListOptions{LabelSelector: selector},
-				})
+			if all || flags.completed || flags.prefix != "" || flags.labels != "" {
+				listed, err := listWorkflows(ctx, serviceClient, flags)
 				errors.CheckError(err)
-				for _, wf := range list.Items {
-					if olderTime != nil && (wf.Status.FinishedAt.IsZero() || wf.Status.FinishedAt.After(*olderTime)) {
+				workflows = append(workflows, listed...)
+			}
+			for _, wf := range workflows {
+				if !dryRun {
+					_, err := serviceClient.DeleteWorkflow(ctx, &workflowpkg.WorkflowDeleteRequest{Name: wf.Name, Namespace: wf.Namespace})
+					if err != nil && status.Code(err) == codes.NotFound {
+						fmt.Printf("Workflow '%s' not found\n", wf.Name)
 						continue
 					}
-					workflowsToDelete = append(workflowsToDelete, wf.ObjectMeta)
-				}
-			}
-			for _, md := range workflowsToDelete {
-				_, err := serviceClient.DeleteWorkflow(ctx, &workflowpkg.WorkflowDeleteRequest{Name: md.Name, Namespace: md.Namespace})
-				if err != nil && apierr.IsNotFound(err) {
-					fmt.Printf("Workflow '%s' not found\n", md.Name)
-					continue
+					errors.CheckError(err)
+					fmt.Printf("Workflow '%s' deleted\n", wf.Name)
+				} else {
+					fmt.Printf("Workflow '%s' deleted (dry-run)\n", wf.Name)
 				}
-				errors.CheckError(err)
-				fmt.Printf("Workflow '%s' deleted\n", md.Name)
 			}
 		},
 	}
 
+	command.Flags().BoolVar(&allNamespaces, "all-namespaces", false, "Delete workflows from all namespaces")
 	command.Flags().BoolVar(&all, "all", false, "Delete all workflows")
-	command.Flags().BoolVar(&completed, "completed", false, "Delete completed workflows")
-	command.Flags().StringVar(&older, "older", "", "Delete completed workflows older than the specified duration (e.g. 10m, 3h, 1d)")
-	command.Flags().StringVarP(&selector, "selector", "l", "", "Selector (label query) to filter on, not including uninitialized ones")
+	command.Flags().BoolVar(&flags.completed, "completed", false, "Delete completed workflows")
+	command.Flags().StringVar(&flags.prefix, "prefix", "", "Delete workflows by prefix")
+	command.Flags().StringVar(&flags.finishedAfter, "older", "", "Delete completed workflows finished before the specified duration (e.g. 10m, 3h, 1d)")
+	command.Flags().StringVarP(&flags.labels, "selector", "l", "", "Selector (label query) to filter on, not including uninitialized ones")
+	command.Flags().BoolVar(&dryRun, "dry-run", false, "Do not delete the workflow, only print what would happen")
 	return command
 }

cmd/argo/commands/list.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"context"
 	"os"
 	"sort"
 	"strings"
@@ -21,117 +22,110 @@ import (
 )
 
 type listFlags struct {
-	allNamespaces bool     // --all-namespaces
-	status        []string // --status
-	completed     bool     // --completed
-	running       bool     // --running
-	prefix        string   // --prefix
-	output        string   // --output
-	since         string   // --since
-	chunkSize     int64    // --chunk-size
-	noHeaders     bool     // --no-headers
-	labels        string   // --selector
-	fields        string   // --field-selector
+	namespace     string
+	status        []string
+	completed     bool
+	running       bool
+	prefix        string
+	output        string
+	createdSince  string
+	finishedAfter string
+	chunkSize     int64
+	noHeaders     bool
+	labels        string
+	fields        string
 }
 
 func NewListCommand() *cobra.Command {
 	var (
-		listArgs listFlags
+		listArgs      listFlags
+		allNamespaces bool
 	)
 	var command = &cobra.Command{
 		Use:   "list",
 		Short: "list workflows",
 		Run: func(cmd *cobra.Command, args []string) {
-			listOpts := &metav1.ListOptions{
-				Limit: listArgs.chunkSize,
-			}
-			labelSelector := labels.NewSelector()
-			if len(listArgs.status) != 0 {
-				req, _ := labels.NewRequirement(common.LabelKeyPhase, selection.In, listArgs.status)
-				if req != nil {
-					labelSelector = labelSelector.Add(*req)
-				}
-			}
-			if listArgs.completed {
-				req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.Equals, []string{"true"})
-				labelSelector = labelSelector.Add(*req)
-			}
-			if listArgs.running {
-				req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.NotEquals, []string{"true"})
-				labelSelector = labelSelector.Add(*req)
-			}
-
-			if listOpts.LabelSelector = labelSelector.String(); listOpts.LabelSelector != "" {
-				listOpts.LabelSelector = listOpts.LabelSelector + ","
-			}
-
-			listOpts.LabelSelector = listOpts.LabelSelector + listArgs.labels
-
-			listOpts.FieldSelector = listArgs.fields
-
 			ctx, apiClient := client.NewAPIClient()
 			serviceClient := apiClient.NewWorkflowServiceClient()
-			namespace := client.Namespace()
-			if listArgs.allNamespaces {
-				namespace = ""
-			}
-
-			var tmpWorkFlows []wfv1.Workflow
-			for {
-				log.WithField("listOpts", listOpts).Debug()
-				wfList, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{Namespace: namespace, ListOptions: listOpts})
-				errors.CheckError(err)
-				tmpWorkFlows = append(tmpWorkFlows, wfList.Items...)
-				if wfList.Continue == "" {
-					break
-				}
-				listOpts.Continue = wfList.Continue
-			}
-
-			var tmpWorkFlowsSelected []wfv1.Workflow
-			if listArgs.prefix == "" {
-				tmpWorkFlowsSelected = tmpWorkFlows
-			} else {
-				tmpWorkFlowsSelected = make([]wfv1.Workflow, 0)
-				for _, wf := range tmpWorkFlows {
-					if strings.HasPrefix(wf.ObjectMeta.Name, listArgs.prefix) {
-						tmpWorkFlowsSelected = append(tmpWorkFlowsSelected, wf)
-					}
-				}
+			if !allNamespaces {
+				listArgs.namespace = client.Namespace()
 			}
-
-			var workflows wfv1.Workflows
-			if listArgs.since == "" {
-				workflows = tmpWorkFlowsSelected
-			} else {
-				workflows = make(wfv1.Workflows, 0)
-				minTime, err := argotime.ParseSince(listArgs.since)
-				errors.CheckError(err)
-				for _, wf := range tmpWorkFlowsSelected {
-					if wf.Status.FinishedAt.IsZero() || wf.ObjectMeta.CreationTimestamp.After(*minTime) {
-						workflows = append(workflows, wf)
-					}
-				}
-			}
-			sort.Sort(workflows)
-			err := printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{
+			workflows, err := listWorkflows(ctx, serviceClient, listArgs)
+			errors.CheckError(err)
+			err = printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{
 				NoHeaders: listArgs.noHeaders,
-				Namespace: listArgs.allNamespaces,
+				Namespace: allNamespaces,
 				Output:    listArgs.output,
 			})
 			errors.CheckError(err)
 		},
 	}
-	command.Flags().BoolVar(&listArgs.allNamespaces, "all-namespaces", false, "Show workflows from all namespaces")
+	command.Flags().BoolVar(&allNamespaces, "all-namespaces", false, "Show workflows from all namespaces")
 	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().StringVarP(&listArgs.output, "output", "o", "", "Output format. One of: wide|name")
-	command.Flags().StringVar(&listArgs.since, "since", "", "Show only workflows newer than a relative duration")
+	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).")
 	command.Flags().StringVarP(&listArgs.labels, "selector", "l", "", "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
 	command.Flags().StringVar(&listArgs.fields, "field-selector", "", "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.")
 	return command
 }
+
+func listWorkflows(ctx context.Context, serviceClient workflowpkg.WorkflowServiceClient, flags listFlags) (wfv1.Workflows, error) {
+	listOpts := &metav1.ListOptions{
+		Limit: flags.chunkSize,
+	}
+	labelSelector := labels.NewSelector()
+	if len(flags.status) != 0 {
+		req, _ := labels.NewRequirement(common.LabelKeyPhase, selection.In, flags.status)
+		if req != nil {
+			labelSelector = labelSelector.Add(*req)
+		}
+	}
+	if flags.completed {
+		req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.Equals, []string{"true"})
+		labelSelector = labelSelector.Add(*req)
+	}
+	if flags.running {
+		req, _ := labels.NewRequirement(common.LabelKeyCompleted, selection.NotEquals, []string{"true"})
+		labelSelector = labelSelector.Add(*req)
+	}
+	if listOpts.LabelSelector = labelSelector.String(); listOpts.LabelSelector != "" {
+		listOpts.LabelSelector = listOpts.LabelSelector + ","
+	}
+	listOpts.LabelSelector = listOpts.LabelSelector + flags.labels
+	listOpts.FieldSelector = flags.fields
+	var workflows wfv1.Workflows
+	for {
+		log.WithField("listOpts", listOpts).Debug()
+		wfList, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{Namespace: flags.namespace, ListOptions: listOpts})
+		if err != nil {
+			return nil, err
+		}
+		workflows = append(workflows, wfList.Items...)
+		if wfList.Continue == "" {
+			break
+		}
+		listOpts.Continue = wfList.Continue
+	}
+	workflows = workflows.
+		Filter(func(wf wfv1.Workflow) bool {
+			return strings.HasPrefix(wf.ObjectMeta.Name, flags.prefix)
+		})
+	if flags.createdSince != "" {
+		t, err := argotime.ParseSince(flags.createdSince)
+		errors.CheckError(err)
+		workflows = workflows.Filter(wfv1.WorkflowCreatedAfter(*t))
+	}
+	if flags.finishedAfter != "" {
+		t, err := argotime.ParseSince(flags.finishedAfter)
+		errors.CheckError(err)
+		workflows = workflows.Filter(wfv1.WorkflowFinishedBefore(*t))
+	}
+	sort.Sort(workflows)
+	return workflows, nil
+}

cmd/argo/commands/list_test.go

@@ -0,0 +1,70 @@
+package commands
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/argoproj/argo/pkg/apiclient/workflow"
+	workflowmocks "github.com/argoproj/argo/pkg/apiclient/workflow/mocks"
+	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
+)
+
+func Test_listWorkflows(t *testing.T) {
+	t.Run("Status", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{LabelSelector: "workflows.argoproj.io/phase in (Pending,Running),"}, listFlags{status: []string{"Running", "Pending"}})
+		if assert.NoError(t, err) {
+			assert.NotNil(t, workflows)
+		}
+	})
+	t.Run("Completed", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{LabelSelector: "workflows.argoproj.io/completed=true,"}, listFlags{completed: true})
+		if assert.NoError(t, err) {
+			assert.NotNil(t, workflows)
+		}
+	})
+	t.Run("Running", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{LabelSelector: "workflows.argoproj.io/completed!=true,"}, listFlags{running: true})
+		if assert.NoError(t, err) {
+			assert.NotNil(t, workflows)
+		}
+	})
+	t.Run("Labels", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{LabelSelector: "foo"}, listFlags{labels: "foo"})
+		if assert.NoError(t, err) {
+			assert.NotNil(t, workflows)
+		}
+	})
+	t.Run("Prefix", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{}, listFlags{prefix: "foo-"})
+		if assert.NoError(t, err) {
+			assert.Len(t, workflows, 1)
+		}
+	})
+	t.Run("Since", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{}, listFlags{createdSince: "1h"})
+		if assert.NoError(t, err) {
+			assert.Len(t, workflows, 1)
+		}
+	})
+	t.Run("Older", func(t *testing.T) {
+		workflows, err := list(&metav1.ListOptions{}, listFlags{finishedAfter: "1h"})
+		if assert.NoError(t, err) {
+			assert.Len(t, workflows, 1)
+		}
+	})
+}
+
+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{
+		{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()}}},
+	}}, nil)
+	workflows, err := listWorkflows(context.Background(), c, flags)
+	return workflows, err
+}