feat: delete multiple resources in parallel (#761)

This PR builds onto #719 by making the deletion of multiple resources
parallel using the new action waiters introduced in #749.

Author: phm07

internal/cmd/base/delete.go

@@ -3,7 +3,8 @@ package base
 import (
 	"errors"
 	"fmt"
-	"reflect"
+	"strings"
+	"sync"
 
 	"github.com/spf13/cobra"
 
@@ -17,11 +18,12 @@ import (
 // DeleteCmd allows defining commands for deleting a resource.
 type DeleteCmd struct {
 	ResourceNameSingular string // e.g. "server"
+	ResourceNamePlural   string // e.g. "servers"
 	ShortDescription     string
 	NameSuggestions      func(client hcapi2.Client) func() []string
 	AdditionalFlags      func(*cobra.Command)
 	Fetch                func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error)
-	Delete               func(s state.State, cmd *cobra.Command, resource interface{}) error
+	Delete               func(s state.State, cmd *cobra.Command, resource interface{}) (*hcloud.Action, error)
 }
 
 // CobraCommand creates a command that can be registered with cobra.
@@ -49,29 +51,50 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command {
 	return cmd
 }
 
-// Run executes a describe command.
+// Run executes a delete command.
 func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error {
-	var cmdErr error
 
-	for _, idOrName := range args {
-		resource, _, err := dc.Fetch(s, cmd, idOrName)
-		if err != nil {
-			cmdErr = errors.Join(cmdErr, err)
-			continue
-		}
+	wg := sync.WaitGroup{}
+	wg.Add(len(args))
+	actions, errs :=
+		make([]*hcloud.Action, len(args)),
+		make([]error, len(args))
 
-		// resource is an interface that always has a type, so the interface is never nil
-		// (i.e. == nil) is always false.
-		if reflect.ValueOf(resource).IsNil() {
-			cmdErr = errors.Join(cmdErr, fmt.Errorf("%s not found: %s", dc.ResourceNameSingular, idOrName))
-			continue
-		}
+	for i, idOrName := range args {
+		i, idOrName := i, idOrName
+		go func() {
+			defer wg.Done()
+			resource, _, err := dc.Fetch(s, cmd, idOrName)
+			if err != nil {
+				errs[i] = err
+				return
+			}
+			if util.IsNil(resource) {
+				errs[i] = fmt.Errorf("%s not found: %s", dc.ResourceNameSingular, idOrName)
+				return
+			}
+			actions[i], errs[i] = dc.Delete(s, cmd, resource)
+		}()
+	}
 
-		if err = dc.Delete(s, cmd, resource); err != nil {
-			cmdErr = errors.Join(cmdErr, fmt.Errorf("deleting %s %s failed: %s", dc.ResourceNameSingular, idOrName, err))
+	wg.Wait()
+	filtered := util.FilterNil(actions)
+	var err error
+	if len(filtered) > 0 {
+		err = s.WaitForActions(cmd, s, filtered...)
+	}
+
+	var actuallyDeleted []string
+	for i, idOrName := range args {
+		if errs[i] == nil {
+			actuallyDeleted = append(actuallyDeleted, idOrName)
 		}
-		cmd.Printf("%s %v deleted\n", dc.ResourceNameSingular, idOrName)
 	}
 
-	return cmdErr
+	if len(actuallyDeleted) == 1 {
+		cmd.Printf("%s %s deleted\n", dc.ResourceNameSingular, actuallyDeleted[0])
+	} else if len(actuallyDeleted) > 1 {
+		cmd.Printf("%s %s deleted\n", dc.ResourceNamePlural, strings.Join(actuallyDeleted, ", "))
+	}
+	return errors.Join(append(errs, err)...)
 }

internal/cmd/base/delete_test.go

@@ -1,6 +1,7 @@
 package base_test
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -12,14 +13,19 @@ import (
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 )
 
+var mu = sync.Mutex{}
+
 var fakeDeleteCmd = &base.DeleteCmd{
 	ResourceNameSingular: "Fake resource",
-	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) error {
+	ResourceNamePlural:   "Fake resources",
+	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) (*hcloud.Action, error) {
+		defer mu.Unlock()
 		cmd.Println("Deleting fake resource")
-		return nil
+		return nil, nil
 	},
 
 	Fetch: func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error) {
+		mu.Lock()
 		cmd.Println("Fetching fake resource")
 
 		resource := &fakeResource{
@@ -43,8 +49,8 @@ func TestDelete(t *testing.T) {
 		},
 		"no flags multiple": {
 			Args: []string{"delete", "123", "456", "789"},
-			ExpOut: "Fetching fake resource\nDeleting fake resource\nFake resource 123 deleted\nFetching fake resource\n" +
-				"Deleting fake resource\nFake resource 456 deleted\nFetching fake resource\nDeleting fake resource\nFake resource 789 deleted\n",
+			ExpOut: "Fetching fake resource\nDeleting fake resource\nFetching fake resource\nDeleting fake resource\n" +
+				"Fetching fake resource\nDeleting fake resource\nFake resources 123, 456, 789 deleted\n",
 		},
 		"quiet": {
 			Args: []string{"delete", "123", "--quiet"},

internal/cmd/certificate/delete.go

@@ -11,16 +11,15 @@ import (
 
 var DeleteCmd = base.DeleteCmd{
 	ResourceNameSingular: "certificate",
+	ResourceNamePlural:   "certificates",
 	ShortDescription:     "Delete a certificate",
 	NameSuggestions:      func(c hcapi2.Client) func() []string { return c.Firewall().Names },
 	Fetch: func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error) {
 		return s.Client().Certificate().Get(s, idOrName)
 	},
-	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) error {
+	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) (*hcloud.Action, error) {
 		certificate := resource.(*hcloud.Certificate)
-		if _, err := s.Client().Certificate().Delete(s, certificate); err != nil {
-			return err
-		}
-		return nil
+		_, err := s.Client().Certificate().Delete(s, certificate)
+		return nil, err
 	},
 }

internal/cmd/certificate/delete_test.go

@@ -1,8 +1,6 @@
 package certificate_test
 
 import (
-	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -63,12 +61,9 @@ func TestDeleteMultiple(t *testing.T) {
 		},
 	}
 
-	expOutBuilder := strings.Builder{}
-
 	var names []string
 	for _, cert := range certs {
 		names = append(names, cert.Name)
-		expOutBuilder.WriteString(fmt.Sprintf("certificate %s deleted\n", cert.Name))
 		fx.Client.CertificateClient.EXPECT().
 			Get(gomock.Any(), cert.Name).
 			Return(cert, nil, nil)
@@ -78,9 +73,8 @@ func TestDeleteMultiple(t *testing.T) {
 	}
 
 	out, errOut, err := fx.Run(cmd, names)
-	expOut := expOutBuilder.String()
 
 	assert.NoError(t, err)
 	assert.Empty(t, errOut)
-	assert.Equal(t, expOut, out)
+	assert.Equal(t, "certificates test1, test2, test3 deleted\n", out)
 }

internal/cmd/firewall/delete.go

@@ -11,16 +11,15 @@ import (
 
 var DeleteCmd = base.DeleteCmd{
 	ResourceNameSingular: "firewall",
+	ResourceNamePlural:   "firewalls",
 	ShortDescription:     "Delete a firewall",
 	NameSuggestions:      func(c hcapi2.Client) func() []string { return c.Firewall().Names },
 	Fetch: func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error) {
 		return s.Client().Firewall().Get(s, idOrName)
 	},
-	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) error {
+	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) (*hcloud.Action, error) {
 		firewall := resource.(*hcloud.Firewall)
-		if _, err := s.Client().Firewall().Delete(s, firewall); err != nil {
-			return err
-		}
-		return nil
+		_, err := s.Client().Firewall().Delete(s, firewall)
+		return nil, err
 	},
 }

internal/cmd/firewall/delete_test.go

@@ -1,8 +1,6 @@
 package firewall_test
 
 import (
-	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -63,12 +61,9 @@ func TestDeleteMultiple(t *testing.T) {
 		},
 	}
 
-	expOutBuilder := strings.Builder{}
-
 	var names []string
 	for _, fw := range firewalls {
 		names = append(names, fw.Name)
-		expOutBuilder.WriteString(fmt.Sprintf("firewall %s deleted\n", fw.Name))
 		fx.Client.FirewallClient.EXPECT().
 			Get(gomock.Any(), fw.Name).
 			Return(fw, nil, nil)
@@ -78,9 +73,8 @@ func TestDeleteMultiple(t *testing.T) {
 	}
 
 	out, errOut, err := fx.Run(cmd, names)
-	expOut := expOutBuilder.String()
 
 	assert.NoError(t, err)
 	assert.Empty(t, errOut)
-	assert.Equal(t, expOut, out)
+	assert.Equal(t, "firewalls test1, test2, test3 deleted\n", out)
 }

internal/cmd/floatingip/delete.go

@@ -11,16 +11,15 @@ import (
 
 var DeleteCmd = base.DeleteCmd{
 	ResourceNameSingular: "Floating IP",
+	ResourceNamePlural:   "Floating IPs",
 	ShortDescription:     "Delete a Floating IP",
 	NameSuggestions:      func(c hcapi2.Client) func() []string { return c.FloatingIP().Names },
 	Fetch: func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error) {
 		return s.Client().FloatingIP().Get(s, idOrName)
 	},
-	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) error {
+	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) (*hcloud.Action, error) {
 		floatingIP := resource.(*hcloud.FloatingIP)
-		if _, err := s.Client().FloatingIP().Delete(s, floatingIP); err != nil {
-			return err
-		}
-		return nil
+		_, err := s.Client().FloatingIP().Delete(s, floatingIP)
+		return nil, err
 	},
 }

internal/cmd/floatingip/delete_test.go

@@ -1,8 +1,6 @@
 package floatingip_test
 
 import (
-	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -63,12 +61,9 @@ func TestDeleteMultiple(t *testing.T) {
 		},
 	}
 
-	expOutBuilder := strings.Builder{}
-
 	var names []string
 	for _, ip := range ips {
 		names = append(names, ip.Name)
-		expOutBuilder.WriteString(fmt.Sprintf("Floating IP %s deleted\n", ip.Name))
 		fx.Client.FloatingIPClient.EXPECT().
 			Get(gomock.Any(), ip.Name).
 			Return(ip, nil, nil)
@@ -78,9 +73,8 @@ func TestDeleteMultiple(t *testing.T) {
 	}
 
 	out, errOut, err := fx.Run(cmd, names)
-	expOut := expOutBuilder.String()
 
 	assert.NoError(t, err)
 	assert.Empty(t, errOut)
-	assert.Equal(t, expOut, out)
+	assert.Equal(t, "Floating IPs test1, test2, test3 deleted\n", out)
 }

internal/cmd/image/delete.go

@@ -11,16 +11,15 @@ import (
 
 var DeleteCmd = base.DeleteCmd{
 	ResourceNameSingular: "image",
+	ResourceNamePlural:   "images",
 	ShortDescription:     "Delete an image",
 	NameSuggestions:      func(c hcapi2.Client) func() []string { return c.Image().Names },
 	Fetch: func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error) {
 		return s.Client().Image().Get(s, idOrName)
 	},
-	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) error {
+	Delete: func(s state.State, cmd *cobra.Command, resource interface{}) (*hcloud.Action, error) {
 		image := resource.(*hcloud.Image)
-		if _, err := s.Client().Image().Delete(s, image); err != nil {
-			return err
-		}
-		return nil
+		_, err := s.Client().Image().Delete(s, image)
+		return nil, err
 	},
 }

internal/cmd/image/delete_test.go

@@ -1,8 +1,6 @@
 package image_test
 
 import (
-	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -63,12 +61,9 @@ func TestDeleteMultiple(t *testing.T) {
 		},
 	}
 
-	expOutBuilder := strings.Builder{}
-
 	var names []string
 	for _, img := range images {
 		names = append(names, img.Name)
-		expOutBuilder.WriteString(fmt.Sprintf("image %s deleted\n", img.Name))
 		fx.Client.ImageClient.EXPECT().
 			Get(gomock.Any(), img.Name).
 			Return(img, nil, nil)
@@ -78,9 +73,8 @@ func TestDeleteMultiple(t *testing.T) {
 	}
 
 	out, errOut, err := fx.Run(cmd, names)
-	expOut := expOutBuilder.String()
 
 	assert.NoError(t, err)
 	assert.Empty(t, errOut)
-	assert.Equal(t, expOut, out)
+	assert.Equal(t, "images test1, test2, test3 deleted\n", out)
 }