From 3b896fef82f50bc27351eec9e2da38032615c6e0 Mon Sep 17 00:00:00 2001 From: phm07 <22707808+phm07@users.noreply.github.com> Date: Wed, 3 Apr 2024 13:48:12 +0200 Subject: [PATCH] feat: allow deletion of multiple resources at once (#719) Currently, the CLI only allows you to delete one resource at a time. This can be time consuming and tedious if you want to delete multiple resources, because you have to wait for each resource to be deleted before you can delete the next one. This PR adds the ability to delete multiple resources at once by passing them to the `delete` subcommand as multiple positional arguments, e.g. `hcloud server delete server1 server2 ...`. It also adds tests to verify that this works as intended. --- internal/cmd/base/delete.go | 38 +++++++++------- internal/cmd/base/delete_test.go | 8 ++++ internal/cmd/certificate/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/firewall/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/floatingip/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/image/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/loadbalancer/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/network/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/placementgroup/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/primaryip/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/server/delete_test.go | 50 ++++++++++++++++++++++ internal/cmd/sshkey/delete_test.go | 46 ++++++++++++++++++++ internal/cmd/volume/delete_test.go | 46 ++++++++++++++++++++ 13 files changed, 540 insertions(+), 16 deletions(-) diff --git a/internal/cmd/base/delete.go b/internal/cmd/base/delete.go index 1fb0b3b0..150d260a 100644 --- a/internal/cmd/base/delete.go +++ b/internal/cmd/base/delete.go @@ -1,6 +1,7 @@ package base import ( + "errors" "fmt" "reflect" @@ -31,10 +32,10 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command { } cmd := &cobra.Command{ - Use: fmt.Sprintf("delete %s<%s>", opts, util.ToKebabCase(dc.ResourceNameSingular)), + Use: fmt.Sprintf("delete %s<%s>...", opts, util.ToKebabCase(dc.ResourceNameSingular)), Short: dc.ShortDescription, Args: util.Validate, - ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client()))), + ValidArgsFunction: cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client())), TraverseChildren: true, DisableFlagsInUseLine: true, PreRunE: util.ChainRunE(s.EnsureToken), @@ -50,22 +51,27 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command { // Run executes a describe command. func (dc *DeleteCmd) Run(s state.State, cmd *cobra.Command, args []string) error { + var cmdErr error - idOrName := args[0] - resource, _, err := dc.Fetch(s, cmd, idOrName) - if err != nil { - return err - } + for _, idOrName := range args { + resource, _, err := dc.Fetch(s, cmd, idOrName) + if err != nil { + cmdErr = errors.Join(cmdErr, err) + continue + } - // 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() { - return fmt.Errorf("%s not found: %s", dc.ResourceNameSingular, idOrName) - } + // 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 + } - if err := dc.Delete(s, cmd, resource); err != nil { - return fmt.Errorf("deleting %s %s failed: %s", dc.ResourceNameSingular, idOrName, err) + if err = dc.Delete(s, cmd, resource); err != nil { + cmdErr = errors.Join(cmdErr, fmt.Errorf("deleting %s %s failed: %s", dc.ResourceNameSingular, idOrName, err)) + } + cmd.Printf("%s %v deleted\n", dc.ResourceNameSingular, idOrName) } - cmd.Printf("%s %v deleted\n", dc.ResourceNameSingular, idOrName) - return nil + + return cmdErr } diff --git a/internal/cmd/base/delete_test.go b/internal/cmd/base/delete_test.go index f72cf9b2..eff0bcfb 100644 --- a/internal/cmd/base/delete_test.go +++ b/internal/cmd/base/delete_test.go @@ -41,8 +41,16 @@ func TestDelete(t *testing.T) { Args: []string{"delete", "123"}, ExpOut: "Fetching fake resource\nDeleting fake resource\nFake resource 123 deleted\n", }, + "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", + }, "quiet": { Args: []string{"delete", "123", "--quiet"}, }, + "quiet multiple": { + Args: []string{"delete", "123", "456", "789", "--quiet"}, + }, }) } diff --git a/internal/cmd/certificate/delete_test.go b/internal/cmd/certificate/delete_test.go index afeb0090..4fe0056b 100644 --- a/internal/cmd/certificate/delete_test.go +++ b/internal/cmd/certificate/delete_test.go @@ -1,6 +1,8 @@ package certificate_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := certificate.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + certs := []*hcloud.Certificate{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + 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) + fx.Client.CertificateClient.EXPECT(). + Delete(gomock.Any(), cert). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/firewall/delete_test.go b/internal/cmd/firewall/delete_test.go index e33a0dd2..b78450d4 100644 --- a/internal/cmd/firewall/delete_test.go +++ b/internal/cmd/firewall/delete_test.go @@ -1,6 +1,8 @@ package firewall_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := firewall.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + firewalls := []*hcloud.Firewall{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + 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) + fx.Client.FirewallClient.EXPECT(). + Delete(gomock.Any(), fw). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/floatingip/delete_test.go b/internal/cmd/floatingip/delete_test.go index e2beda8e..afca2dcf 100644 --- a/internal/cmd/floatingip/delete_test.go +++ b/internal/cmd/floatingip/delete_test.go @@ -1,6 +1,8 @@ package floatingip_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := floatingip.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + ips := []*hcloud.FloatingIP{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + 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) + fx.Client.FloatingIPClient.EXPECT(). + Delete(gomock.Any(), ip). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/image/delete_test.go b/internal/cmd/image/delete_test.go index 98a866e6..70c954e3 100644 --- a/internal/cmd/image/delete_test.go +++ b/internal/cmd/image/delete_test.go @@ -1,6 +1,8 @@ package image_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := image.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + images := []*hcloud.Image{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + 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) + fx.Client.ImageClient.EXPECT(). + Delete(gomock.Any(), img). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/loadbalancer/delete_test.go b/internal/cmd/loadbalancer/delete_test.go index e7f43895..a770b30f 100644 --- a/internal/cmd/loadbalancer/delete_test.go +++ b/internal/cmd/loadbalancer/delete_test.go @@ -1,6 +1,8 @@ package loadbalancer_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := loadbalancer.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + loadBalancers := []*hcloud.LoadBalancer{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for _, lb := range loadBalancers { + names = append(names, lb.Name) + expOutBuilder.WriteString(fmt.Sprintf("Load Balancer %s deleted\n", lb.Name)) + fx.Client.LoadBalancerClient.EXPECT(). + Get(gomock.Any(), lb.Name). + Return(lb, nil, nil) + fx.Client.LoadBalancerClient.EXPECT(). + Delete(gomock.Any(), lb). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/network/delete_test.go b/internal/cmd/network/delete_test.go index f5b8a10d..39dba670 100644 --- a/internal/cmd/network/delete_test.go +++ b/internal/cmd/network/delete_test.go @@ -1,6 +1,8 @@ package network_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := network.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + networks := []*hcloud.Network{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for _, net := range networks { + names = append(names, net.Name) + expOutBuilder.WriteString(fmt.Sprintf("Network %s deleted\n", net.Name)) + fx.Client.NetworkClient.EXPECT(). + Get(gomock.Any(), net.Name). + Return(net, nil, nil) + fx.Client.NetworkClient.EXPECT(). + Delete(gomock.Any(), net). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/placementgroup/delete_test.go b/internal/cmd/placementgroup/delete_test.go index fd68ba9d..ee248b56 100644 --- a/internal/cmd/placementgroup/delete_test.go +++ b/internal/cmd/placementgroup/delete_test.go @@ -1,6 +1,8 @@ package placementgroup_test import ( + "fmt" + "strings" "testing" "time" @@ -42,3 +44,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := placementgroup.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + groups := []*hcloud.PlacementGroup{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for _, pg := range groups { + names = append(names, pg.Name) + expOutBuilder.WriteString(fmt.Sprintf("placement group %s deleted\n", pg.Name)) + fx.Client.PlacementGroupClient.EXPECT(). + Get(gomock.Any(), pg.Name). + Return(pg, nil, nil) + fx.Client.PlacementGroupClient.EXPECT(). + Delete(gomock.Any(), pg). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/primaryip/delete_test.go b/internal/cmd/primaryip/delete_test.go index 2e2f0cd4..c7aa31d0 100644 --- a/internal/cmd/primaryip/delete_test.go +++ b/internal/cmd/primaryip/delete_test.go @@ -1,6 +1,8 @@ package primaryip_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -46,3 +48,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := primaryip.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + ips := []*hcloud.PrimaryIP{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for _, ip := range ips { + names = append(names, ip.Name) + expOutBuilder.WriteString(fmt.Sprintf("Primary IP %s deleted\n", ip.Name)) + fx.Client.PrimaryIPClient.EXPECT(). + Get(gomock.Any(), ip.Name). + Return(ip, nil, nil) + fx.Client.PrimaryIPClient.EXPECT(). + Delete(gomock.Any(), ip). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/server/delete_test.go b/internal/cmd/server/delete_test.go index f322846f..f8f310e1 100644 --- a/internal/cmd/server/delete_test.go +++ b/internal/cmd/server/delete_test.go @@ -1,6 +1,8 @@ package server_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -42,3 +44,51 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := server.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + servers := []*hcloud.Server{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for i, srv := range servers { + names = append(names, srv.Name) + expOutBuilder.WriteString(fmt.Sprintf("Server %s deleted\n", srv.Name)) + fx.Client.ServerClient.EXPECT(). + Get(gomock.Any(), srv.Name). + Return(srv, nil, nil) + fx.Client.ServerClient.EXPECT(). + DeleteWithResult(gomock.Any(), srv). + Return(&hcloud.ServerDeleteResult{ + Action: &hcloud.Action{ID: int64(i)}, + }, nil, nil) + fx.ActionWaiter.EXPECT(). + ActionProgress(gomock.Any(), gomock.Any(), &hcloud.Action{ID: int64(i)}) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/sshkey/delete_test.go b/internal/cmd/sshkey/delete_test.go index 02674537..0c557bcf 100644 --- a/internal/cmd/sshkey/delete_test.go +++ b/internal/cmd/sshkey/delete_test.go @@ -1,6 +1,8 @@ package sshkey_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := sshkey.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + keys := []*hcloud.SSHKey{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for _, key := range keys { + names = append(names, key.Name) + expOutBuilder.WriteString(fmt.Sprintf("SSH Key %s deleted\n", key.Name)) + fx.Client.SSHKeyClient.EXPECT(). + Get(gomock.Any(), key.Name). + Return(key, nil, nil) + fx.Client.SSHKeyClient.EXPECT(). + Delete(gomock.Any(), key). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +} diff --git a/internal/cmd/volume/delete_test.go b/internal/cmd/volume/delete_test.go index 69a2cea9..30ffdffd 100644 --- a/internal/cmd/volume/delete_test.go +++ b/internal/cmd/volume/delete_test.go @@ -1,6 +1,8 @@ package volume_test import ( + "fmt" + "strings" "testing" "github.com/golang/mock/gomock" @@ -38,3 +40,47 @@ func TestDelete(t *testing.T) { assert.Empty(t, errOut) assert.Equal(t, expOut, out) } + +func TestDeleteMultiple(t *testing.T) { + fx := testutil.NewFixture(t) + defer fx.Finish() + + cmd := volume.DeleteCmd.CobraCommand(fx.State()) + fx.ExpectEnsureToken() + + volumes := []*hcloud.Volume{ + { + ID: 123, + Name: "test1", + }, + { + ID: 456, + Name: "test2", + }, + { + ID: 789, + Name: "test3", + }, + } + + expOutBuilder := strings.Builder{} + + var names []string + for _, v := range volumes { + names = append(names, v.Name) + expOutBuilder.WriteString(fmt.Sprintf("Volume %s deleted\n", v.Name)) + fx.Client.VolumeClient.EXPECT(). + Get(gomock.Any(), v.Name). + Return(v, nil, nil) + fx.Client.VolumeClient.EXPECT(). + Delete(gomock.Any(), v). + Return(nil, nil) + } + + out, errOut, err := fx.Run(cmd, names) + expOut := expOutBuilder.String() + + assert.NoError(t, err) + assert.Empty(t, errOut) + assert.Equal(t, expOut, out) +}