Skip to content

Commit

Permalink
Remove command should accept more than one container ID (#212)
Browse files Browse the repository at this point in the history 
[#210] Remove command should accept more than one container ID

Signed-off-by: Daniel Milchev [email protected]
  • Loading branch information
@daniel-milchev
daniel-milchev authored Nov 23, 2023
1 parent 0dabb8c commit 27df4f0
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 16 deletions.
40 changes: 29 additions & 11 deletions containerm/cli/cli_command_ctrs_remove.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ package main

import (
"context"
"errors"

"github.com/eclipse-kanto/container-management/containerm/containers/types"
utilcli "github.com/eclipse-kanto/container-management/containerm/util/cli"
errorutil "github.com/eclipse-kanto/container-management/containerm/util/error"
"github.com/spf13/cobra"
)

Expand All @@ -33,30 +35,46 @@ type removeConfig struct {
func (cc *removeCmd) init(cli *cli) {
cc.cli = cli
cc.cmd = &cobra.Command{
Use: "remove <container-id>",
Short: "Remove a container.",
Long: "Remove a container and frees the associated resources.",
Args: cobra.MaximumNArgs(1),
Use: "remove <container-id> ...",
Short: "Remove one or more containers.",
Long: "Remove one or more containers and frees the associated resources.",
RunE: func(cmd *cobra.Command, args []string) error {
return cc.run(args)
},
Example: "remove <container-id>\n remove --name <container-name>\n remove -n <container-name>",
Example: " remove <container-id>\n remove <container-id> <container-id> \n remove --name <container-name>\n remove -n <container-name>",
}
cc.setupFlags()
}

func (cc *removeCmd) run(args []string) error {
var (
ctr *types.Container
err error
ctx = context.Background()
ctr *types.Container
err error
ctx = context.Background()
errs errorutil.CompoundError
)
// parse parameters
if ctr, err = utilcli.ValidateContainerByNameArgsSingle(ctx, args, cc.config.name, cc.cli.gwManClient); err != nil {
return err
if len(args) == 0 {
if ctr, err = utilcli.ValidateContainerByNameArgsSingle(ctx, nil, cc.config.name, cc.cli.gwManClient); err != nil {
return err
}
return cc.cli.gwManClient.Remove(ctx, ctr.ID, cc.config.force)
}
for _, arg := range args {
ctr, err = utilcli.ValidateContainerByNameArgsSingle(ctx, []string{arg}, cc.config.name, cc.cli.gwManClient)
if err == nil {
if err = cc.cli.gwManClient.Remove(ctx, ctr.ID, cc.config.force); err != nil {
errs.Append(err)
}
} else {
errs.Append(err)
}
}
if errs.Size() > 0 {
return errors.New(errs.ErrorWithMessage("containers couldn't be removed due to the following reasons: "))
}
return cc.cli.gwManClient.Remove(ctx, ctr.ID, cc.config.force)

return nil
}

func (cc *removeCmd) setupFlags() {
Expand Down
53 changes: 53 additions & 0 deletions containerm/cli/cli_command_ctrs_remove_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/eclipse-kanto/container-management/containerm/client"
"github.com/eclipse-kanto/container-management/containerm/containers/types"
"github.com/eclipse-kanto/container-management/containerm/log"
errorutil "github.com/eclipse-kanto/container-management/containerm/util/error"
"github.com/golang/mock/gomock"
)

Expand Down Expand Up @@ -121,6 +122,14 @@ func (rmTc *removeCommandTest) generateRunExecutionConfigs() map[string]testRunE
args: removeCmdArgs,
mockExecution: rmTc.mockExecRemoveNoErrors,
},
"test_remove_multiple": {
args: []string{"test-container-one", "test-container-two"},
mockExecution: rmTc.mockExecRemoveMultipleNoErrors,
},
"test_remove_multiple_with_err": {
args: []string{"test-container-one", "test-container-two"},
mockExecution: rmTc.mockExecRemoveMultipleWithErrors,
},
"test_remove_by_id_err": {
args: removeCmdArgs,
mockExecution: rmTc.mockExecRemoveGetError,
Expand Down Expand Up @@ -177,12 +186,14 @@ func (rmTc *removeCommandTest) mockExecRemoveIDAndName(args []string) error {
rmTc.mockClient.EXPECT().Remove(context.Background(), args[0], false).Times(0)
return log.NewError("Container ID and --name (-n) cannot be provided at the same time - use only one of them")
}

func (rmTc *removeCommandTest) mockExecRemoveNoIDorName(args []string) error {
rmTc.mockClient.EXPECT().Get(context.Background(), gomock.Any()).Times(0)
rmTc.mockClient.EXPECT().List(context.Background(), gomock.Any()).Times(0)
rmTc.mockClient.EXPECT().Get(context.Background(), gomock.Any()).Times(0)
return log.NewError("You must provide either an ID or a name for the container via --name (-n) ")
}

func (rmTc *removeCommandTest) mockExecRemoveNoErrors(args []string) error {
// setup expected calls
ctr := &types.Container{
Expand All @@ -195,6 +206,41 @@ func (rmTc *removeCommandTest) mockExecRemoveNoErrors(args []string) error {
return nil
}

func (rmTc *removeCommandTest) mockExecRemoveMultipleNoErrors(args []string) error {
// setup expected calls
ctr1 := &types.Container{
ID: args[0],
Name: removeContainerName,
}
ctr2 := &types.Container{
ID: args[1],
Name: removeContainerName,
}
rmTc.mockClient.EXPECT().Get(context.Background(), args[0]).Times(1).Return(ctr1, nil)
rmTc.mockClient.EXPECT().Get(context.Background(), args[1]).Times(1).Return(ctr2, nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), args[0], false).Times(1).Return(nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), args[1], false).Times(1).Return(nil)
// no error expected
return nil
}

func (rmTc *removeCommandTest) mockExecRemoveMultipleWithErrors(args []string) error {
// setup expected calls
var errs errorutil.CompoundError
err1 := log.NewErrorf("The requested container with ID = %s was not found.", args[0])
err2 := errors.New("failed to remove container")
ctr := &types.Container{
ID: args[1],
Name: removeContainerName,
}
rmTc.mockClient.EXPECT().Get(context.Background(), args[0]).Times(1).Return(nil, err1)
rmTc.mockClient.EXPECT().Get(context.Background(), args[1]).Times(1).Return(ctr, nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), args[1], false).Times(1).Return(err2)
errs.Append(err1, err2)

return errors.New(errs.ErrorWithMessage("containers couldn't be removed due to the following reasons: "))
}

func (rmTc *removeCommandTest) mockExecRemoveError(args []string) error {
// setup expected calls
err := errors.New("failed to remove container")
Expand All @@ -206,6 +252,7 @@ func (rmTc *removeCommandTest) mockExecRemoveError(args []string) error {
rmTc.mockClient.EXPECT().Remove(context.Background(), args[0], false).Times(1).Return(err)
return err
}

func (rmTc *removeCommandTest) mockExecRemoveByNameNoErrors(args []string) error {
// setup expected calls
ctrs := []*types.Container{{ID: removeContainerID, Name: removeContainerName}}
Expand All @@ -224,37 +271,43 @@ func (rmTc *removeCommandTest) mockExecRemoveByNameError(args []string) error {
// no error expected
return err
}

func (rmTc *removeCommandTest) mockExecRemoveByNameListError(args []string) error {
// setup expected calls
err := errors.New("failed to list containers")
rmTc.mockClient.EXPECT().List(context.Background(), gomock.AssignableToTypeOf(client.WithName(removeContainerName))).Times(1).Return(nil, err)
rmTc.mockClient.EXPECT().Remove(context.Background(), gomock.Any(), false).Times(0)
return err
}

func (rmTc *removeCommandTest) mockExecRemoveByNameListNilCtrs(args []string) error {
// setup expected calls
rmTc.mockClient.EXPECT().List(context.Background(), gomock.AssignableToTypeOf(client.WithName(removeContainerName))).Times(1).Return(nil, nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), gomock.Any(), false).Times(0)
return log.NewErrorf("The requested container with name = %s was not found. Try using an ID instead.", removeContainerName)
}

func (rmTc *removeCommandTest) mockExecRemoveByNameListZeroCtrs(args []string) error {
// setup expected calls
rmTc.mockClient.EXPECT().List(context.Background(), gomock.AssignableToTypeOf(client.WithName(removeContainerName))).Times(1).Return([]*types.Container{}, nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), gomock.Any(), false).Times(0)
return log.NewErrorf("The requested container with name = %s was not found. Try using an ID instead.", removeContainerName)
}

func (rmTc *removeCommandTest) mockExecRemoveByNameListMoreThanOneCtrs(args []string) error {
// setup expected calls
rmTc.mockClient.EXPECT().List(context.Background(), gomock.AssignableToTypeOf(client.WithName(removeContainerName))).Times(1).Return([]*types.Container{{}, {}}, nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), gomock.Any(), false).Times(0)
return log.NewErrorf("There are more than one containers with name = %s. Try using an ID instead.", removeContainerName)
}

func (rmTc *removeCommandTest) mockExecRemoveGetNilError(args []string) error {
// setup expected calls
rmTc.mockClient.EXPECT().Get(context.Background(), args[0]).Times(1).Return(nil, nil)
rmTc.mockClient.EXPECT().Remove(context.Background(), args[0], false).Times(0)
return log.NewErrorf("The requested container with ID = %s was not found.", args[0])
}

func (rmTc *removeCommandTest) mockExecRemoveGetError(args []string) error {
// setup expected calls
err := errors.New("failed to remove container")
Expand Down
2 changes: 1 addition & 1 deletion containerm/util/cli/args_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func ValidateContainerByNameArgsSingle(ctx context.Context, args []string, provi
err error
)
// parse parameters
if len(args) == 1 {
if len(args) >= 1 {
if container, err = kantoCMClient.Get(ctx, args[0]); err != nil {
return nil, err
} else if container == nil {
Expand Down
8 changes: 7 additions & 1 deletion containerm/util/error/error_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ func (m *CompoundError) Size() int {

// Error returns the combined error messages.
func (m *CompoundError) Error() string {
return m.ErrorWithMessage(fmt.Sprintf("%d errors:", len(m.errs)))
}

// ErrorWithMessage returns combined error messages with a custom message.
func (m *CompoundError) ErrorWithMessage(message string) string {
if len(m.errs) == 0 {
return fmt.Sprintf("no error")
}
Expand All @@ -46,5 +51,6 @@ func (m *CompoundError) Error() string {
for i, err := range m.errs {
serrs[i] = fmt.Sprintf("* %s", err)
}
return fmt.Sprintf("%d errors:\n\n%s", len(m.errs), strings.Join(serrs, "\n"))

return fmt.Sprintf("%s\n\n%s", message, strings.Join(serrs, "\n"))
}
7 changes: 4 additions & 3 deletions integration/testdata/remove-help.golden
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
Remove a container and frees the associated resources.
Remove one or more containers and frees the associated resources.

Usage:
kanto-cm remove <container-id>
kanto-cm remove <container-id> ...

Examples:
remove <container-id>
remove <container-id>
remove <container-id> <container-id>
remove --name <container-name>
remove -n <container-name>

Expand Down

0 comments on commit 27df4f0

Please sign in to comment.