Skip to content

feat: allow deletion of multiple resources at once #719

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 3, 2024
Merged

feat: allow deletion of multiple resources at once #719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46dff1c
feat: allow deletion of multiple resources at once
phm07 Mar 20, 2024
c70ca50
fix argument suggestions
phm07 Apr 2, 2024
8327b12
add tests
phm07 Apr 2, 2024
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
38 changes: 22 additions & 16 deletions internal/cmd/base/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package base

import (
"errors"
"fmt"
"reflect"

Expand Down Expand Up @@ -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),
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

The current code deletes the resources in series. Doing it in parallel would be faster and we could wait for all actions at once.

I am fine with merging it as it is and leaving this for the future to optimize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing it this way we can guarantee the order of operations (which is also important for the tests for example). I agree that we can leave it like this for now.

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
}
8 changes: 8 additions & 0 deletions internal/cmd/base/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
})
}
46 changes: 46 additions & 0 deletions internal/cmd/certificate/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package certificate_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
}
46 changes: 46 additions & 0 deletions internal/cmd/firewall/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package firewall_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
}
46 changes: 46 additions & 0 deletions internal/cmd/floatingip/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package floatingip_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
}
46 changes: 46 additions & 0 deletions internal/cmd/image/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package image_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
}
46 changes: 46 additions & 0 deletions internal/cmd/loadbalancer/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package loadbalancer_test

import (
"fmt"
"strings"
"testing"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -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)
}
Loading