From 27df4f0c1b28fbe50a92c5f4b950aa6467d262b7 Mon Sep 17 00:00:00 2001 From: Daniel Milchev Date: Thu, 23 Nov 2023 11:35:31 +0200 Subject: [PATCH] Remove command should accept more than one container ID (#212) [#210] Remove command should accept more than one container ID Signed-off-by: Daniel Milchev fixed-term.daniel.milchev@bosch.io --- containerm/cli/cli_command_ctrs_remove.go | 40 ++++++++++---- .../cli/cli_command_ctrs_remove_test.go | 53 +++++++++++++++++++ containerm/util/cli/args_util.go | 2 +- containerm/util/error/error_util.go | 8 ++- integration/testdata/remove-help.golden | 7 +-- 5 files changed, 94 insertions(+), 16 deletions(-) diff --git a/containerm/cli/cli_command_ctrs_remove.go b/containerm/cli/cli_command_ctrs_remove.go index 8981e3c..c1cfee5 100644 --- a/containerm/cli/cli_command_ctrs_remove.go +++ b/containerm/cli/cli_command_ctrs_remove.go @@ -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" ) @@ -33,30 +35,46 @@ type removeConfig struct { func (cc *removeCmd) init(cli *cli) { cc.cli = cli cc.cmd = &cobra.Command{ - Use: "remove ", - Short: "Remove a container.", - Long: "Remove a container and frees the associated resources.", - Args: cobra.MaximumNArgs(1), + Use: "remove ...", + 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 \n remove --name \n remove -n ", + Example: " remove \n remove \n remove --name \n remove -n ", } 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() { diff --git a/containerm/cli/cli_command_ctrs_remove_test.go b/containerm/cli/cli_command_ctrs_remove_test.go index 3959661..9a531dc 100644 --- a/containerm/cli/cli_command_ctrs_remove_test.go +++ b/containerm/cli/cli_command_ctrs_remove_test.go @@ -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" ) @@ -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, @@ -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{ @@ -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") @@ -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}} @@ -224,6 +271,7 @@ 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") @@ -231,30 +279,35 @@ func (rmTc *removeCommandTest) mockExecRemoveByNameListError(args []string) erro 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") diff --git a/containerm/util/cli/args_util.go b/containerm/util/cli/args_util.go index 9382cef..c2dbd8f 100644 --- a/containerm/util/cli/args_util.go +++ b/containerm/util/cli/args_util.go @@ -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 { diff --git a/containerm/util/error/error_util.go b/containerm/util/error/error_util.go index 60e451a..abd9809 100644 --- a/containerm/util/error/error_util.go +++ b/containerm/util/error/error_util.go @@ -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") } @@ -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")) } diff --git a/integration/testdata/remove-help.golden b/integration/testdata/remove-help.golden index da894a5..e28dc33 100644 --- a/integration/testdata/remove-help.golden +++ b/integration/testdata/remove-help.golden @@ -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 + kanto-cm remove ... Examples: -remove + remove + remove remove --name remove -n