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 <rothberg@redhat.com>

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)
 }
 

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
 }
 

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

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))
+	rmReports := make([]*reports.RmReport, 0, len(toRemove))
-	for _, name := range namesOrIds {
+	for _, nameOrID := range toRemove {
-		report, err := containers.Remove(ic.ClientCtx, name, options)
+		if alreadyRemoved[nameOrID] {
-		if err != nil {
-			rmReports = append(rmReports, &reports.RmReport{
-				Id:  name,
-				Err: err,
-			})
 			continue
 		}
-		rmReports = append(rmReports, report...)
+		newReports, err := containers.Remove(ic.ClientCtx, nameOrID, options)
+		if err != nil {
+			rmReports = append(rmReports, &reports.RmReport{Id: nameOrID, Err: err})
+			continue
+		}
+		for i := range newReports {
+			alreadyRemoved[newReports[i].Id] = true
+			rmReports = append(rmReports, newReports[i])
+		}
 	}
-
 	return rmReports, nil
 }
 

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