From f2c365c6f6b1263569ea6c45cd7404d7acf53aaa Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 4 Apr 2024 16:13:24 +0200 Subject: [PATCH] rm --force work for more than one arg When we remove with --force we do not return a error if the input does not exists, however if we get more than on input we must try to remove all and not just NOP out and not remove anything just because one arg did not exists. Also make the code simpler for commands that do have the --ignore option and just make --force imply --ignore which reduces the ugly error handling. Fixes #21529 Signed-off-by: Paul Holzinger --- cmd/podman/containers/rm.go | 9 +++------ cmd/podman/images/rm.go | 24 ++++++++---------------- cmd/podman/pods/rm.go | 10 ++++------ pkg/domain/infra/tunnel/helpers.go | 8 +++++++- pkg/domain/infra/tunnel/pods.go | 18 +++++++++--------- test/system/010-images.bats | 9 +++++++++ test/system/055-rm.bats | 7 +++++++ test/system/160-volumes.bats | 7 +++++++ test/system/200-pod.bats | 6 ++++++ test/system/500-networking.bats | 6 ++++++ 10 files changed, 66 insertions(+), 38 deletions(-) diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index 45b4b4ec58..0efac2a281 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -132,6 +132,9 @@ func rm(cmd *cobra.Command, args []string) error { logrus.Debug("--all is set: enforcing --depend=true") rmOptions.Depend = true } + if rmOptions.Force { + rmOptions.Ignore = true + } return removeContainers(utils.RemoveSlash(args), rmOptions, true, false) } @@ -144,9 +147,6 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit var errs utils.OutputErrors responses, err := registry.ContainerEngine().ContainerRm(context.Background(), namesOrIDs, rmOptions) if err != nil { - if rmOptions.Force && strings.Contains(err.Error(), define.ErrNoSuchCtr.Error()) { - return nil - } if setExit { setExitCode(err) } @@ -158,9 +158,6 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit if errors.Is(r.Err, define.ErrWillDeadlock) { logrus.Errorf("Potential deadlock detected - please run 'podman system renumber' to resolve") } - if rmOptions.Force && strings.Contains(r.Err.Error(), define.ErrNoSuchCtr.Error()) { - continue - } if setExit { setExitCode(r.Err) } diff --git a/cmd/podman/images/rm.go b/cmd/podman/images/rm.go index b0409ce208..c03d5b0b72 100644 --- a/cmd/podman/images/rm.go +++ b/cmd/podman/images/rm.go @@ -3,14 +3,11 @@ package images import ( "errors" "fmt" - "strings" "github.com/containers/podman/v5/cmd/podman/common" "github.com/containers/podman/v5/cmd/podman/registry" - "github.com/containers/podman/v5/cmd/podman/utils" "github.com/containers/podman/v5/pkg/domain/entities" "github.com/containers/podman/v5/pkg/errorhandling" - "github.com/containers/storage/types" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -72,6 +69,10 @@ func rm(cmd *cobra.Command, args []string) error { return errors.New("when using the --all switch, you may not pass any images names or IDs") } + if imageOpts.Force { + imageOpts.Ignore = true + } + // Note: certain image-removal errors are non-fatal. Hence, the report // might be set even if err != nil. report, rmErrors := registry.ImageEngine().Remove(registry.GetContext(), args, imageOpts) @@ -85,19 +86,10 @@ func rm(cmd *cobra.Command, args []string) error { fmt.Println("Deleted: " + d) } } - for _, err := range rmErrors { - if !imageOpts.Force || !strings.Contains(err.Error(), types.ErrImageUnknown.Error()) { - registry.SetExitCode(report.ExitCode) - } - } + } + if len(rmErrors) > 0 { + registry.SetExitCode(report.ExitCode) } - var errs utils.OutputErrors - for _, err := range rmErrors { - if imageOpts.Force && strings.Contains(err.Error(), types.ErrImageUnknown.Error()) { - continue - } - errs = append(errs, err) - } - return errorhandling.JoinErrors(errs) + return errorhandling.JoinErrors(rmErrors) } diff --git a/cmd/podman/pods/rm.go b/cmd/podman/pods/rm.go index b96ef15d0a..ac5bde0807 100644 --- a/cmd/podman/pods/rm.go +++ b/cmd/podman/pods/rm.go @@ -83,6 +83,10 @@ func rm(cmd *cobra.Command, args []string) error { rmOptions.Timeout = &timeout } + if rmOptions.Force { + rmOptions.Ignore = true + } + errs = append(errs, removePods(args, rmOptions.PodRmOptions, true)...) for _, idFile := range rmOptions.PodIDFiles { @@ -110,9 +114,6 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b responses, err := registry.ContainerEngine().PodRm(context.Background(), namesOrIDs, rmOptions) if err != nil { - if rmOptions.Force && strings.Contains(err.Error(), define.ErrNoSuchPod.Error()) { - return nil - } setExitCode(err) errs = append(errs, err) if !strings.Contains(err.Error(), define.ErrRemovingCtrs.Error()) { @@ -127,9 +128,6 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b fmt.Println(r.Id) } } else { - if rmOptions.Force && strings.Contains(r.Err.Error(), define.ErrNoSuchPod.Error()) { - continue - } setExitCode(r.Err) errs = append(errs, r.Err) for ctr, err := range r.RemovedCtrs { diff --git a/pkg/domain/infra/tunnel/helpers.go b/pkg/domain/infra/tunnel/helpers.go index 3d33f179d6..a60b67e9e2 100644 --- a/pkg/domain/infra/tunnel/helpers.go +++ b/pkg/domain/infra/tunnel/helpers.go @@ -81,7 +81,7 @@ func getContainersAndInputByContext(contextWithConnection context.Context, all, return filtered, rawInputs, nil } -func getPodsByContext(contextWithConnection context.Context, all bool, namesOrIDs []string) ([]*entities.ListPodsReport, error) { +func getPodsByContext(contextWithConnection context.Context, all bool, ignore bool, namesOrIDs []string) ([]*entities.ListPodsReport, error) { if all && len(namesOrIDs) > 0 { return nil, errors.New("cannot look up specific pods and all") } @@ -108,6 +108,9 @@ func getPodsByContext(contextWithConnection context.Context, all bool, namesOrID inspectData, err := pods.Inspect(contextWithConnection, nameOrID, nil) if err != nil { if errorhandling.Contains(err, define.ErrNoSuchPod) { + if ignore { + continue + } return nil, fmt.Errorf("unable to find pod %q: %w", nameOrID, define.ErrNoSuchPod) } return nil, err @@ -126,6 +129,9 @@ func getPodsByContext(contextWithConnection context.Context, all bool, namesOrID } if !found { + if ignore { + continue + } return nil, fmt.Errorf("unable to find pod %q: %w", nameOrID, define.ErrNoSuchPod) } } diff --git a/pkg/domain/infra/tunnel/pods.go b/pkg/domain/infra/tunnel/pods.go index f485b8a386..f31205418e 100644 --- a/pkg/domain/infra/tunnel/pods.go +++ b/pkg/domain/infra/tunnel/pods.go @@ -23,7 +23,7 @@ func (ic *ContainerEngine) PodKill(ctx context.Context, namesOrIds []string, opt return nil, err } - foundPods, err := getPodsByContext(ic.ClientCtx, opts.All, namesOrIds) + foundPods, err := getPodsByContext(ic.ClientCtx, opts.All, false, namesOrIds) if err != nil { return nil, err } @@ -55,7 +55,7 @@ func (ic *ContainerEngine) PodLogs(ctx context.Context, nameOrIDs string, option } func (ic *ContainerEngine) PodPause(ctx context.Context, namesOrIds []string, options entities.PodPauseOptions) ([]*entities.PodPauseReport, error) { - foundPods, err := getPodsByContext(ic.ClientCtx, options.All, namesOrIds) + foundPods, err := getPodsByContext(ic.ClientCtx, options.All, false, namesOrIds) if err != nil { return nil, err } @@ -76,7 +76,7 @@ func (ic *ContainerEngine) PodPause(ctx context.Context, namesOrIds []string, op } func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string, options entities.PodunpauseOptions) ([]*entities.PodUnpauseReport, error) { - foundPods, err := getPodsByContext(ic.ClientCtx, options.All, namesOrIds) + foundPods, err := getPodsByContext(ic.ClientCtx, options.All, false, namesOrIds) if err != nil { return nil, err } @@ -102,8 +102,8 @@ func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string, func (ic *ContainerEngine) PodStop(ctx context.Context, namesOrIds []string, opts entities.PodStopOptions) ([]*entities.PodStopReport, error) { timeout := -1 - foundPods, err := getPodsByContext(ic.ClientCtx, opts.All, namesOrIds) - if err != nil && !(opts.Ignore && errors.Is(err, define.ErrNoSuchPod)) { + foundPods, err := getPodsByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) + if err != nil { return nil, err } if opts.Timeout != -1 { @@ -127,7 +127,7 @@ func (ic *ContainerEngine) PodStop(ctx context.Context, namesOrIds []string, opt } func (ic *ContainerEngine) PodRestart(ctx context.Context, namesOrIds []string, options entities.PodRestartOptions) ([]*entities.PodRestartReport, error) { - foundPods, err := getPodsByContext(ic.ClientCtx, options.All, namesOrIds) + foundPods, err := getPodsByContext(ic.ClientCtx, options.All, false, namesOrIds) if err != nil { return nil, err } @@ -148,7 +148,7 @@ func (ic *ContainerEngine) PodRestart(ctx context.Context, namesOrIds []string, } func (ic *ContainerEngine) PodStart(ctx context.Context, namesOrIds []string, options entities.PodStartOptions) ([]*entities.PodStartReport, error) { - foundPods, err := getPodsByContext(ic.ClientCtx, options.All, namesOrIds) + foundPods, err := getPodsByContext(ic.ClientCtx, options.All, false, namesOrIds) if err != nil { return nil, err } @@ -169,8 +169,8 @@ func (ic *ContainerEngine) PodStart(ctx context.Context, namesOrIds []string, op } func (ic *ContainerEngine) PodRm(ctx context.Context, namesOrIds []string, opts entities.PodRmOptions) ([]*entities.PodRmReport, error) { - foundPods, err := getPodsByContext(ic.ClientCtx, opts.All, namesOrIds) - if err != nil && !(opts.Ignore && errors.Is(err, define.ErrNoSuchPod)) { + foundPods, err := getPodsByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) + if err != nil { return nil, err } reports := make([]*entities.PodRmReport, 0, len(foundPods)) diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 977c58684d..074359c0d1 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -312,6 +312,15 @@ Deleted: $pauseID" is "$output" "Error: bogus: image not known" "Should print error" run_podman image rm --force bogus is "$output" "" "Should print no output" + + random_image_name=$(random_string) + random_image_name=${random_image_name,,} # name must be lowercase + run_podman image tag $IMAGE $random_image_name + run_podman image rm --force bogus $random_image_name + assert "$output" = "Untagged: localhost/$random_image_name:latest" "removed image" + + run_podman images + assert "$output" !~ "$random_image_name" "image must be removed" } @test "podman images - commit docker with comment" { diff --git a/test/system/055-rm.bats b/test/system/055-rm.bats index 7027753cdd..c8169746ec 100644 --- a/test/system/055-rm.bats +++ b/test/system/055-rm.bats @@ -111,6 +111,13 @@ load helpers is "$output" "Error: no container with ID or name \"bogus\" found: no such container" "Should print error" run_podman container rm --force bogus is "$output" "" "Should print no output" + + run_podman create --name test $IMAGE + run_podman container rm --force bogus test + assert "$output" = "test" "should delete test" + + run_podman ps -a -q + assert "$output" = "" "container should be removed" } function __run_healthcheck_container() { diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index 3316e119f8..7a8fc11b37 100644 --- a/test/system/160-volumes.bats +++ b/test/system/160-volumes.bats @@ -550,6 +550,13 @@ EOF is "$output" "Error: no volume with name \"bogus\" found: no such volume" "Should print error" run_podman volume rm --force bogus is "$output" "" "Should print no output" + + run_podman volume create testvol + run_podman volume rm --force bogus testvol + assert "$output" = "testvol" "removed volume" + + run_podman volume ls -q + assert "$output" = "" "no volumes" } @test "podman ps -f" { diff --git a/test/system/200-pod.bats b/test/system/200-pod.bats index fcc012ee01..8eccee12ca 100644 --- a/test/system/200-pod.bats +++ b/test/system/200-pod.bats @@ -579,6 +579,12 @@ io.max | $lomajmin rbps=1048576 wbps=1048576 riops=max wiops=max is "$output" "Error: .*bogus.*: no such pod" "Should print error" run_podman pod rm -t -1 --force bogus is "$output" "" "Should print no output" + + run_podman pod create --name testpod + run_podman pod rm --force bogus testpod + assert "$output" =~ "[0-9a-f]{64}" "rm pod" + run_podman pod ps -q + assert "$output" = "" "no pods listed" } @test "podman pod create on failure" { diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index 294c7cb914..66f45b2b0b 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -848,6 +848,12 @@ EOF is "$output" "Error: unable to find network with name or ID bogus: network not found" "Should print error" run_podman network rm -t -1 --force bogus is "$output" "" "Should print no output" + + run_podman network create testnet + run_podman network rm --force bogus testnet + assert "$output" = "testnet" "rm network" + run_podman network ls -q + assert "$output" = "podman" "only podman network listed" } @test "podman network rm --dns-option " {