Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: delete multiple resources in parallel #761

Merged
merged 5 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 43 additions & 20 deletions internal/cmd/base/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ package base
import (
"errors"
"fmt"
"reflect"
"strings"
"sync"

"github.com/spf13/cobra"

Expand All @@ -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.
Expand Down Expand Up @@ -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()
Comment on lines +65 to +66
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this complexity really needed? Most of the waiting time happens while waiting for the actions anyway?

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)...)
}
14 changes: 10 additions & 4 deletions internal/cmd/base/delete_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package base_test

import (
"sync"
"testing"

"github.com/spf13/cobra"
Expand All @@ -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{
Expand All @@ -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"},
Expand Down
9 changes: 4 additions & 5 deletions internal/cmd/certificate/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
8 changes: 1 addition & 7 deletions internal/cmd/certificate/delete_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package certificate_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
9 changes: 4 additions & 5 deletions internal/cmd/firewall/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
8 changes: 1 addition & 7 deletions internal/cmd/firewall/delete_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package firewall_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
9 changes: 4 additions & 5 deletions internal/cmd/floatingip/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
8 changes: 1 addition & 7 deletions internal/cmd/floatingip/delete_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package floatingip_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
9 changes: 4 additions & 5 deletions internal/cmd/image/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
8 changes: 1 addition & 7 deletions internal/cmd/image/delete_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package image_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
9 changes: 4 additions & 5 deletions internal/cmd/loadbalancer/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,15 @@ import (

var DeleteCmd = base.DeleteCmd{
ResourceNameSingular: "Load Balancer",
ResourceNamePlural: "Load Balancers",
ShortDescription: "Delete a Load Balancer",
NameSuggestions: func(c hcapi2.Client) func() []string { return c.LoadBalancer().Names },
Fetch: func(s state.State, cmd *cobra.Command, idOrName string) (interface{}, *hcloud.Response, error) {
return s.Client().LoadBalancer().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) {
loadBalancer := resource.(*hcloud.LoadBalancer)
if _, err := s.Client().LoadBalancer().Delete(s, loadBalancer); err != nil {
return err
}
return nil
_, err := s.Client().LoadBalancer().Delete(s, loadBalancer)
return nil, err
},
}
Loading
Loading