From a4cef543504600db95453aa863b97ed82c833d41 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 12 Jan 2022 15:33:00 +0100 Subject: podman container rm: remove pod Support removing the entire pod when --depend is used on an infra container. --all now implies --depend to properly support removing all containers and not error out when hitting infra containers. Signed-off-by: Valentin Rothberg --- cmd/podman/containers/rm.go | 5 ++++ libpod/runtime_ctr.go | 25 ++++++++++++++++-- pkg/domain/infra/abi/containers.go | 34 ++++++++++++++++++++++++- pkg/domain/infra/tunnel/containers.go | 48 ++++++++++++++++++++++++----------- test/system/200-pod.bats | 34 +++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 18 deletions(-) diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index cede0ba14..a8fdee7fd 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -110,6 +110,11 @@ func rm(cmd *cobra.Command, args []string) error { args = append(args, id) } + if rmOptions.All { + logrus.Debug("--all is set: enforcing --depend=true") + rmOptions.Depend = true + } + return removeContainers(args, rmOptions, true) } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 53ccb9139..66e0c29a9 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -915,9 +915,30 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol return id, cleanupErr } -// RemoveDepend removes all dependencies for a container +// RemoveDepend removes all dependencies for a container. +// If the container is an infra container, the entire pod gets removed. func (r *Runtime) RemoveDepend(ctx context.Context, rmCtr *Container, force bool, removeVolume bool, timeout *uint) ([]*reports.RmReport, error) { + logrus.Debugf("Removing container %s and all dependent containers", rmCtr.ID()) rmReports := make([]*reports.RmReport, 0) + if rmCtr.IsInfra() { + pod, err := r.GetPod(rmCtr.PodID()) + if err != nil { + return nil, err + } + logrus.Debugf("Removing pod %s: depends on infra container %s", pod.ID(), rmCtr.ID()) + podContainerIDS, err := pod.AllContainersByID() + if err != nil { + return nil, err + } + if err := r.RemovePod(ctx, pod, true, force, timeout); err != nil { + return nil, err + } + for _, cID := range podContainerIDS { + rmReports = append(rmReports, &reports.RmReport{Id: cID}) + } + return rmReports, nil + } + deps, err := r.state.ContainerInUse(rmCtr) if err != nil { if err == define.ErrCtrRemoved { @@ -940,9 +961,9 @@ func (r *Runtime) RemoveDepend(ctx context.Context, rmCtr *Container, force bool } rmReports = append(rmReports, reports...) } + report := reports.RmReport{Id: rmCtr.ID()} report.Err = r.removeContainer(ctx, rmCtr, force, removeVolume, false, timeout) - return append(rmReports, &report), nil } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index a4522698e..afd25d313 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -357,24 +357,56 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, return rmReports, nil } + alreadyRemoved := make(map[string]bool) + addReports := func(newReports []*reports.RmReport) { + for i := range newReports { + alreadyRemoved[newReports[i].Id] = true + rmReports = append(rmReports, newReports[i]) + } + } if !options.All && options.Depend { // Add additional containers based on dependencies to container map for _, ctr := range ctrs { + if alreadyRemoved[ctr.ID()] { + continue + } reports, err := ic.Libpod.RemoveDepend(ctx, ctr, options.Force, options.Volumes, options.Timeout) if err != nil { return rmReports, err } - rmReports = append(rmReports, reports...) + addReports(reports) } return rmReports, nil } + + // If --all is set, make sure to remove the infra containers' + // dependencies before doing the parallel removal below. + if options.All { + for _, ctr := range ctrs { + if alreadyRemoved[ctr.ID()] || !ctr.IsInfra() { + continue + } + reports, err := ic.Libpod.RemoveDepend(ctx, ctr, options.Force, options.Volumes, options.Timeout) + if err != nil { + return rmReports, err + } + addReports(reports) + } + } + errMap, err := parallelctr.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { + if alreadyRemoved[c.ID()] { + return nil + } return ic.removeContainer(ctx, c, options) }) if err != nil { return nil, err } for ctr, err := range errMap { + if alreadyRemoved[ctr.ID()] { + continue + } report := new(reports.RmReport) report.Id = ctr.ID() report.Err = err diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 4f72eab96..ae689f1f7 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -188,35 +188,53 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, if opts.Timeout != nil { options = options.WithTimeout(*opts.Timeout) } + + toRemove := []string{} + alreadyRemoved := make(map[string]bool) // Avoids trying to remove already removed containers if opts.All { - ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) + ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, opts.Ignore, nil) if err != nil { return nil, err } - rmReports := make([]*reports.RmReport, 0, len(ctrs)) for _, c := range ctrs { - report, err := containers.Remove(ic.ClientCtx, c.ID, options) + toRemove = append(toRemove, c.ID) + } + } else { + for _, ctr := range namesOrIds { + // NOTE that we set ignore=true here to support + // removing external containers (e.g., Buildah + // containers). If we don't find the container, + // we'll use the raw input provided by the user + // instead of the ID. Since this can only happen + // with external containers, it poses no threat + // to the `alreadyRemoved` checks below. + ctrs, err := getContainersByContext(ic.ClientCtx, false, true, []string{ctr}) if err != nil { - return rmReports, err + return nil, err } - rmReports = append(rmReports, report...) + id := ctr + if len(ctrs) == 1 { + id = ctrs[0].ID + } + toRemove = append(toRemove, id) } - return rmReports, nil } - rmReports := make([]*reports.RmReport, 0, len(namesOrIds)) - for _, name := range namesOrIds { - report, err := containers.Remove(ic.ClientCtx, name, options) + rmReports := make([]*reports.RmReport, 0, len(toRemove)) + for _, nameOrID := range toRemove { + if alreadyRemoved[nameOrID] { + continue + } + newReports, err := containers.Remove(ic.ClientCtx, nameOrID, options) if err != nil { - rmReports = append(rmReports, &reports.RmReport{ - Id: name, - Err: err, - }) + rmReports = append(rmReports, &reports.RmReport{Id: nameOrID, Err: err}) continue } - rmReports = append(rmReports, report...) + for i := range newReports { + alreadyRemoved[newReports[i].Id] = true + rmReports = append(rmReports, newReports[i]) + } } - return rmReports, nil } diff --git a/test/system/200-pod.bats b/test/system/200-pod.bats index 4a3337e57..51fcea104 100644 --- a/test/system/200-pod.bats +++ b/test/system/200-pod.bats @@ -362,4 +362,38 @@ EOF run_podman run --rm --pod "new:$pod_name" $IMAGE hostname is "$output" "$pod_name" "new:POD should have hostname name set to podname" } + +@test "podman rm --force to remove infra container" { + local pod_name="$(random_string 10 | tr A-Z a-z)" + run_podman create --pod "new:$pod_name" $IMAGE + container_ID="$output" + run_podman pod inspect --format "{{.InfraContainerID}}" $pod_name + infra_ID="$output" + + run_podman 125 container rm $infra_ID + is "$output" ".* and cannot be removed without removing the pod" + run_podman 125 container rm --force $infra_ID + is "$output" ".* and cannot be removed without removing the pod" + + run_podman container rm --depend $infra_ID + is "$output" ".*$infra_ID.*" + is "$output" ".*$container_ID.*" + + # Now make sure that --force --all works as well + run_podman create --pod "new:$pod_name" $IMAGE + container_1_ID="$output" + run_podman create --pod "$pod_name" $IMAGE + container_2_ID="$output" + run_podman create $IMAGE + container_3_ID="$output" + run_podman pod inspect --format "{{.InfraContainerID}}" $pod_name + infra_ID="$output" + + run_podman container rm --force --all $infraID + is "$output" ".*$infra_ID.*" + is "$output" ".*$container_1_ID.*" + is "$output" ".*$container_2_ID.*" + is "$output" ".*$container_3_ID.*" +} + # vim: filetype=sh -- cgit v1.2.3-54-g00ecf