feat: Add support to delete by using labels. Depended on by #2116

cmd/argo/commands/delete.go

@@ -1,167 +1,85 @@
 package commands
 
 import (
-	"context"
 	"fmt"
-	"log"
-	"os"
 	"time"
 
+	"github.com/argoproj/pkg/errors"
 	argotime "github.com/argoproj/pkg/time"
 	"github.com/spf13/cobra"
 	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 (
-	completedWorkflowListOption = metav1.ListOptions{
-		LabelSelector: fmt.Sprintf("%s=true", common.LabelKeyCompleted),
-	}
+	completedLabelSelector = fmt.Sprintf("%s=true", common.LabelKeyCompleted)
 )
 
 // NewDeleteCommand returns a new instance of an `argo delete` command
 func NewDeleteCommand() *cobra.Command {
 	var (
+		selector  string
 		all       bool
 		completed bool
 		older     string
 	)
 
 	var command = &cobra.Command{
-		Use:   "delete WORKFLOW",
-		Short: "delete a workflow and its associated pods",
+		Use: "delete WORKFLOW...",
 		Run: func(cmd *cobra.Command, args []string) {
-			if client.ArgoServer != "" {
-				apiServerDeleteWorkflows(all, older, completed, args)
-			} else {
-				wfClient = InitWorkflowClient()
-				if all {
-					deleteWorkflows(metav1.ListOptions{}, nil)
-				} else if older != "" {
-					olderTime, err := argotime.ParseSince(older)
-					if err != nil {
-						log.Fatal(err)
-					}
-					deleteWorkflows(completedWorkflowListOption, olderTime)
-				} else if completed {
-					deleteWorkflows(completedWorkflowListOption, nil)
-				} else {
-					if len(args) == 0 {
-						cmd.HelpFunc()(cmd, args)
-						os.Exit(1)
+			ctx, apiClient := client.NewAPIClient()
+			var workflowsToDelete []metav1.ObjectMeta
+			for _, name := range args {
+				workflowsToDelete = append(workflowsToDelete, metav1.ObjectMeta{
+					Name:      name,
+					Namespace: client.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
 					}
-					for _, wfName := range args {
-						deleteWorkflow(wfName)
+				}
+				// 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 := apiClient.NewWorkflowServiceClient().ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{
+					Namespace:   client.Namespace(),
+					ListOptions: &metav1.ListOptions{LabelSelector: selector},
+				})
+				errors.CheckError(err)
+				for _, wf := range list.Items {
+					if olderTime != nil && (wf.Status.FinishedAt.IsZero() || wf.Status.FinishedAt.After(*olderTime)) {
+						continue
 					}
+					workflowsToDelete = append(workflowsToDelete, wf.ObjectMeta)
 				}
 			}
+			for _, md := range workflowsToDelete {
+				_, err := apiClient.NewWorkflowServiceClient().DeleteWorkflow(ctx, &workflowpkg.WorkflowDeleteRequest{
+					Name:      md.Name,
+					Namespace: md.Namespace,
+				})
+				errors.CheckError(err)
+				fmt.Printf("Workflow '%s' deleted\n", md.Name)
+			}
 		},
 	}
 
 	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")
 	return command
 }
-
-func apiServerDeleteWorkflows(allWFs bool, older string, completed bool, wfNames []string) {
-	conn := client.GetClientConn()
-	defer conn.Close()
-	ns, _, _ := client.Config.Namespace()
-	wfApiClient, ctx := GetWFApiServerGRPCClient(conn)
-
-	var delWFNames []string
-	var err error
-	if allWFs {
-		delWFNames, err = getWFList(wfApiClient, ctx, ns, &metav1.ListOptions{}, nil)
-		if err != nil {
-			log.Fatal(err)
-			return
-		}
-	} else if older != "" {
-		olderTime, err := argotime.ParseSince(older)
-		if err != nil {
-			log.Fatal(err)
-			return
-		}
-		delWFNames, err = getWFList(wfApiClient, ctx, ns, &completedWorkflowListOption, olderTime)
-		if err != nil {
-			log.Fatal(err)
-			return
-		}
-	} else if completed {
-		delWFNames, err = getWFList(wfApiClient, ctx, ns, &completedWorkflowListOption, nil)
-		if err != nil {
-			log.Fatal(err)
-			return
-		}
-	} else {
-		delWFNames = wfNames
-	}
-	for _, wfName := range delWFNames {
-		apiServerDeleteWorkflow(wfApiClient, ctx, wfName, ns)
-	}
-}
-
-func getWFList(client workflowpkg.WorkflowServiceClient, ctx context.Context, ns string, opts *metav1.ListOptions, older *time.Time) ([]string, error) {
-	wfReq := workflowpkg.WorkflowListRequest{
-		ListOptions: opts,
-		Namespace:   ns,
-	}
-	wfList, err := client.ListWorkflows(ctx, &wfReq)
-	if err != nil {
-		return nil, err
-	}
-	var wfNames []string
-	for _, wf := range wfList.Items {
-		if older != nil {
-			if wf.Status.FinishedAt.IsZero() || wf.Status.FinishedAt.After(*older) {
-				continue
-			}
-		}
-		wfNames = append(wfNames, wf.Name)
-	}
-	return wfNames, nil
-}
-
-func apiServerDeleteWorkflow(client workflowpkg.WorkflowServiceClient, ctx context.Context, wfName, ns string) {
-	wfReq := workflowpkg.WorkflowDeleteRequest{
-		Name:      wfName,
-		Namespace: ns,
-	}
-
-	_, err := client.DeleteWorkflow(ctx, &wfReq)
-	if err != nil {
-		log.Fatal(err)
-		return
-	}
-	fmt.Printf("workflow %s deleted\n", wfName)
-
-}
-
-func deleteWorkflow(wfName string) {
-	err := wfClient.Delete(wfName, &metav1.DeleteOptions{})
-	if err != nil {
-		log.Fatal(err)
-	}
-	fmt.Printf("Workflow '%s' deleted\n", wfName)
-}
-
-func deleteWorkflows(options metav1.ListOptions, older *time.Time) {
-	wfList, err := wfClient.List(options)
-	if err != nil {
-		log.Fatal(err)
-	}
-	for _, wf := range wfList.Items {
-		if older != nil {
-			if wf.Status.FinishedAt.IsZero() || wf.Status.FinishedAt.After(*older) {
-				continue
-			}
-		}
-		deleteWorkflow(wf.ObjectMeta.Name)
-	}
-}

pkg/apiclient/classic-workflow-service-client.go

@@ -108,7 +108,11 @@ func (k *classicWorkflowServiceClient) WatchWorkflows(ctx context.Context, in *w
 }
 
 func (k *classicWorkflowServiceClient) DeleteWorkflow(ctx context.Context, in *workflowpkg.WorkflowDeleteRequest, opts ...grpc.CallOption) (*workflowpkg.WorkflowDeleteResponse, error) {
-	panic("implement me")
+	err := k.ArgoprojV1alpha1().Workflows(in.Namespace).Delete(in.Name, in.DeleteOptions)
+	if err != nil {
+		return nil, err
+	}
+	return &workflowpkg.WorkflowDeleteResponse{}, nil
 }
 
 func (k *classicWorkflowServiceClient) RetryWorkflow(ctx context.Context, in *workflowpkg.WorkflowRetryRequest, opts ...grpc.CallOption) (*v1alpha1.Workflow, error) {

test/e2e/cli_test.go

@@ -3,6 +3,7 @@ package e2e
 import (
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
@@ -105,12 +106,69 @@ func (s *CLISuite) TestRoot() {
 			}
 		})
 	})
-	s.Run("Delete", func(t *testing.T) {
-		s.Given(t).RunCli([]string{"delete", "basic"}, func(t *testing.T, output string, err error) {
-			if assert.NoError(t, err) {
-				assert.Contains(t, output, "deleted")
-			}
-		})
+}
+
+func (s *CLISuite) TestWorkflowDelete() {
+	s.Run("DeleteByName", func(t *testing.T) {
+		s.Given(t).
+			Workflow("@smoke/basic.yaml").
+			When().
+			SubmitWorkflow().
+			WaitForWorkflow(15*time.Second).
+			Given().
+			RunCli([]string{"delete", "basic"}, func(t *testing.T, output string, err error) {
+				if assert.NoError(t, err) {
+					assert.Contains(t, output, "Workflow 'basic' deleted")
+				}
+			})
+	})
+	s.Run("DeleteAll", func(t *testing.T) {
+		s.Given(t).
+			Workflow("@smoke/basic.yaml").
+			When().
+			SubmitWorkflow().
+			WaitForWorkflow(15*time.Second).
+			Given().
+			RunCli([]string{"delete", "--all", "-l", "argo-e2e"}, func(t *testing.T, output string, err error) {
+				if assert.NoError(t, err) {
+					assert.Contains(t, output, "Workflow 'basic' deleted")
+				}
+			})
+	})
+	s.Run("DeleteCompleted", func(t *testing.T) {
+		s.Given(t).
+			Workflow("@smoke/basic.yaml").
+			When().
+			SubmitWorkflow().
+			Given().
+			RunCli([]string{"delete", "--completed", "-l", "argo-e2e"}, func(t *testing.T, output string, err error) {
+				if assert.NoError(t, err) {
+					// nothing should be deleted yet
+					assert.NotContains(t, output, "deleted")
+				}
+			}).
+			When().
+			WaitForWorkflow(15*time.Second).
+			Given().
+			RunCli([]string{"delete", "--completed", "-l", "argo-e2e"}, func(t *testing.T, output string, err error) {
+				if assert.NoError(t, err) {
+					assert.Contains(t, output, "Workflow 'basic' deleted")
+				}
+			})
+	})
+	s.Run("DeleteOlder", func(t *testing.T) {
+		s.Given(t).
+			Workflow("@smoke/basic.yaml").
+			When().
+			SubmitWorkflow().
+			WaitForWorkflow(15*time.Second).
+			Given().
+			RunCli([]string{"delete", "--older", "1d", "-l", "argo-e2e"}, func(t *testing.T, output string, err error) {
+				if assert.NoError(t, err) {
+					// nothing over a day should be deleted
+					assert.NotContains(t, output, "deleted")
+				}
+			})
 	})
 }
 

test/e2e/fixtures/when.go

@@ -170,3 +170,19 @@ func (w *When) Then() *Then {
 		kubeClient:            w.kubeClient,
 	}
 }
+
+func (w *When) Given() *Given {
+	return &Given{
+		t:                     w.t,
+		diagnostics:           w.diagnostics,
+		client:                w.client,
+		wfTemplateClient:      w.wfTemplateClient,
+		cronClient:            w.cronClient,
+		offloadNodeStatusRepo: w.offloadNodeStatusRepo,
+		wf:                    w.wf,
+		wfTemplates:           w.wfTemplates,
+		cronWf:                w.cronWf,
+		workflowName:          w.workflowName,
+		kubeClient:            w.kubeClient,
+	}
+}