Merge pull request #2970 from jwhonce/wip/remote_pod_prune

Refactor of 'podman prune' to better support remote

cmd/podman/cliconfig/config.go

@@ -159,7 +159,7 @@ type PruneContainersValues struct {
 	Force bool
 }
 
-type PrunePodsValues struct {
+type PodPruneValues struct {
 	PodmanCommand
 	Force bool
 }

cmd/podman/pods_prune.go

@@ -1,19 +1,15 @@
 package main
 
 import (
-	"context"
-
 	"github.com/containers/libpod/cmd/podman/cliconfig"
-	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/pkg/adapter"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
 var (
-	prunePodsCommand     cliconfig.PrunePodsValues
-	prunePodsDescription = `
+	podPruneCommand     cliconfig.PodPruneValues
+	podPruneDescription = `
 	podman pod prune
 
 	Removes all exited pods
@@ -22,62 +18,30 @@ var (
 		Use:   "prune",
 		Args:  noSubArgs,
 		Short: "Remove all stopped pods",
-		Long:  prunePodsDescription,
+		Long:  podPruneDescription,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			prunePodsCommand.InputArgs = args
-			prunePodsCommand.GlobalFlags = MainGlobalOpts
-			return prunePodsCmd(&prunePodsCommand)
+			podPruneCommand.InputArgs = args
+			podPruneCommand.GlobalFlags = MainGlobalOpts
+			return podPruneCmd(&podPruneCommand)
 		},
 	}
 )
 
 func init() {
-	prunePodsCommand.Command = _prunePodsCommand
-	prunePodsCommand.SetHelpTemplate(HelpTemplate())
-	prunePodsCommand.SetUsageTemplate(UsageTemplate())
-	flags := prunePodsCommand.Flags()
-	flags.BoolVarP(&prunePodsCommand.Force, "force", "f", false, "Force removal of a running pods.  The default is false")
+	podPruneCommand.Command = _prunePodsCommand
+	podPruneCommand.SetHelpTemplate(HelpTemplate())
+	podPruneCommand.SetUsageTemplate(UsageTemplate())
+	flags := podPruneCommand.Flags()
+	flags.BoolVarP(&podPruneCommand.Force, "force", "f", false, "Force removal of a running pods.  The default is false")
 }
 
-func prunePods(runtime *adapter.LocalRuntime, ctx context.Context, maxWorkers int, force bool) error {
-	var deleteFuncs []shared.ParallelWorkerInput
-
-	states := []string{shared.PodStateStopped, shared.PodStateExited}
-	delPods, err := runtime.GetPodsByStatus(states)
-	if err != nil {
-		return err
-	}
-	if len(delPods) < 1 {
-		return nil
-	}
-	for _, pod := range delPods {
-		p := pod
-		f := func() error {
-			return runtime.RemovePod(ctx, p, force, force)
-		}
-
-		deleteFuncs = append(deleteFuncs, shared.ParallelWorkerInput{
-			ContainerID:  p.ID(),
-			ParallelFunc: f,
-		})
-	}
-	// Run the parallel funcs
-	deleteErrors, errCount := shared.ParallelExecuteWorkerPool(maxWorkers, deleteFuncs)
-	return printParallelOutput(deleteErrors, errCount)
-}
-
-func prunePodsCmd(c *cliconfig.PrunePodsValues) error {
+func podPruneCmd(c *cliconfig.PodPruneValues) error {
 	runtime, err := adapter.GetRuntime(&c.PodmanCommand)
 	if err != nil {
 		return errors.Wrapf(err, "could not get runtime")
 	}
 	defer runtime.Shutdown(false)
 
-	maxWorkers := shared.Parallelize("rm")
-	if c.GlobalIsSet("max-workers") {
-		maxWorkers = c.GlobalFlags.MaxWorks
-	}
-	logrus.Debugf("Setting maximum workers to %d", maxWorkers)
-
-	return prunePods(runtime, getContext(), maxWorkers, c.Bool("force"))
+	ok, failures, err := runtime.PrunePods(getContext(), c)
+	return printCmdResults(ok, failures)
 }

cmd/podman/system_prune.go

@@ -82,13 +82,21 @@ Are you sure you want to continue? [y/N] `, volumeString)
 	ctx := getContext()
 	fmt.Println("Deleted Containers")
 	lasterr := pruneContainers(runtime, ctx, rmWorkers, false, false)
+
 	fmt.Println("Deleted Pods")
-	if err := prunePods(runtime, ctx, rmWorkers, true); err != nil {
+	pruneValues := cliconfig.PodPruneValues{
+		PodmanCommand: c.PodmanCommand,
+		Force:         c.Force,
+	}
+	ok, failures, err := runtime.PrunePods(ctx, &pruneValues)
+	if err != nil {
 		if lasterr != nil {
 			logrus.Errorf("%q", lasterr)
 		}
 		lasterr = err
 	}
+	printCmdResults(ok, failures)
+
 	if c.Bool("volumes") {
 		fmt.Println("Deleted Volumes")
 		err := volumePrune(runtime, getContext())

pkg/adapter/pods.go

@@ -4,20 +4,16 @@ package adapter
 
 import (
 	"context"
-	"github.com/pkg/errors"
 	"strings"
 
 	"github.com/containers/libpod/cmd/podman/cliconfig"
 	"github.com/containers/libpod/cmd/podman/shared"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/adapter/shortcuts"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
-// Pod ...
-type Pod struct {
-	*libpod.Pod
-}
-
 // PodContainerStats is struct containing an adapter Pod and a libpod
 // ContainerStats and is used primarily for outputing pod stats.
 type PodContainerStats struct {
@@ -25,6 +21,49 @@ type PodContainerStats struct {
 	ContainerStats map[string]*libpod.ContainerStats
 }
 
+// PrunePods removes pods
+func (r *LocalRuntime) PrunePods(ctx context.Context, cli *cliconfig.PodPruneValues) ([]string, map[string]error, error) {
+	var (
+		ok       = []string{}
+		failures = map[string]error{}
+	)
+
+	maxWorkers := shared.DefaultPoolSize("rm")
+	if cli.GlobalIsSet("max-workers") {
+		maxWorkers = cli.GlobalFlags.MaxWorks
+	}
+	logrus.Debugf("Setting maximum rm workers to %d", maxWorkers)
+
+	states := []string{shared.PodStateStopped, shared.PodStateExited}
+	if cli.Force {
+		states = append(states, shared.PodStateRunning)
+	}
+
+	pods, err := r.GetPodsByStatus(states)
+	if err != nil {
+		return ok, failures, err
+	}
+	if len(pods) < 1 {
+		return ok, failures, nil
+	}
+
+	pool := shared.NewPool("pod_prune", maxWorkers, len(pods))
+	for _, p := range pods {
+		p := p
+
+		pool.Add(shared.Job{p.ID(),
+			func() error {
+				err := r.Runtime.RemovePod(ctx, p, cli.Force, cli.Force)
+				if err != nil {
+					logrus.Debugf("Failed to remove pod %s: %s", p.ID(), err.Error())
+				}
+				return err
+			},
+		})
+	}
+	return pool.Run()
+}
+
 // RemovePods ...
 func (r *LocalRuntime) RemovePods(ctx context.Context, cli *cliconfig.PodRmValues) ([]string, []error) {
 	var (

pkg/adapter/pods_remote.go

@@ -14,13 +14,9 @@ import (
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/varlinkapi"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
-// Pod ...
-type Pod struct {
-	remotepod
-}
-
 // PodContainerStats is struct containing an adapter Pod and a libpod
 // ContainerStats and is used primarily for outputing pod stats.
 type PodContainerStats struct {
@@ -28,13 +24,6 @@ type PodContainerStats struct {
 	ContainerStats map[string]*libpod.ContainerStats
 }
 
-type remotepod struct {
-	config     *libpod.PodConfig
-	state      *libpod.PodInspectState
-	containers []libpod.PodContainerInfo
-	Runtime    *LocalRuntime
-}
-
 // RemovePods removes one or more based on the cli context.
 func (r *LocalRuntime) RemovePods(ctx context.Context, cli *cliconfig.PodRmValues) ([]string, []error) {
 	var (
@@ -539,3 +528,34 @@ func (r *LocalRuntime) RemovePod(ctx context.Context, p *Pod, removeCtrs, force
 	}
 	return nil
 }
+
+// PrunePods...
+func (r *LocalRuntime) PrunePods(ctx context.Context, cli *cliconfig.PodPruneValues) ([]string, map[string]error, error) {
+	var (
+		ok       = []string{}
+		failures = map[string]error{}
+	)
+	states := []string{shared.PodStateStopped, shared.PodStateExited}
+	if cli.Force {
+		states = append(states, shared.PodStateRunning)
+	}
+
+	ids, err := iopodman.GetPodsByStatus().Call(r.Conn, states)
+	if err != nil {
+		return ok, failures, err
+	}
+	if len(ids) < 1 {
+		return ok, failures, nil
+	}
+
+	for _, id := range ids {
+		_, err := iopodman.RemovePod().Call(r.Conn, id, cli.Force)
+		if err != nil {
+			logrus.Debugf("Failed to remove pod %s: %s", id, err.Error())
+			failures[id] = err
+		} else {
+			ok = append(ok, id)
+		}
+	}
+	return ok, failures, nil
+}

pkg/adapter/runtime.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"io"
 	"io/ioutil"
-	"k8s.io/api/core/v1"
 	"os"
 	"text/template"
 
@@ -25,6 +24,7 @@ import (
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/pkg/errors"
+	"k8s.io/api/core/v1"
 )
 
 // LocalRuntime describes a typical libpod runtime
@@ -43,6 +43,11 @@ type Container struct {
 	*libpod.Container
 }
 
+// Pod encapsulates the libpod.Pod structure, helps with remote vs. local
+type Pod struct {
+	*libpod.Pod
+}
+
 // Volume ...
 type Volume struct {
 	*libpod.Volume
@@ -371,8 +376,7 @@ func (r *LocalRuntime) GenerateKube(c *cliconfig.GenerateKubeValues) (*v1.Pod, *
 }
 
 // GetPodsByStatus returns a slice of pods filtered by a libpod status
-func (r *LocalRuntime) GetPodsByStatus(statuses []string) ([]*Pod, error) {
-	var adapterPods []*Pod
+func (r *LocalRuntime) GetPodsByStatus(statuses []string) ([]*libpod.Pod, error) {
 
 	filterFunc := func(p *libpod.Pod) bool {
 		state, _ := shared.GetPodStatus(p)
@@ -383,25 +387,11 @@ func (r *LocalRuntime) GetPodsByStatus(statuses []string) ([]*Pod, error) {
 		}
 		return false
 	}
+
 	pods, err := r.Runtime.Pods(filterFunc)
 	if err != nil {
 		return nil, err
 	}
-	for _, p := range pods {
-		adapterPod := Pod{
-			p,
-		}
-		adapterPods = append(adapterPods, &adapterPod)
-	}
-	return adapterPods, nil
-}
 
-// RemovePod removes a pod
-// If removeCtrs is specified, containers will be removed
-// Otherwise, a pod that is not empty will return an error and not be removed
-// If force is specified with removeCtrs, all containers will be stopped before
-// being removed
-// Otherwise, the pod will not be removed if any containers are running
-func (r *LocalRuntime) RemovePod(ctx context.Context, p *Pod, removeCtrs, force bool) error {
-	return r.Runtime.RemovePod(ctx, p.Pod, removeCtrs, force)
+	return pods, nil
 }

pkg/adapter/runtime_remote.go

@@ -99,6 +99,18 @@ type remoteContainer struct {
 	state   *libpod.ContainerState
 }
 
+// Pod ...
+type Pod struct {
+	remotepod
+}
+
+type remotepod struct {
+	config     *libpod.PodConfig
+	state      *libpod.PodInspectState
+	containers []libpod.PodContainerInfo
+	Runtime    *LocalRuntime
+}
+
 type VolumeFilter func(*Volume) bool
 
 // Volume is embed for libpod volumes

test/e2e/pod_prune_test.go

@@ -0,0 +1,78 @@
+// +build !remoteclient
+
+package integration
+
+import (
+	"os"
+
+	. "github.com/containers/libpod/test/utils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Podman pod prune", func() {
+	var (
+		tempdir    string
+		err        error
+		podmanTest *PodmanTestIntegration
+	)
+
+	BeforeEach(func() {
+		tempdir, err = CreateTempDirInTempDir()
+		if err != nil {
+			os.Exit(1)
+		}
+		podmanTest = PodmanTestCreate(tempdir)
+		podmanTest.Setup()
+		podmanTest.RestoreAllArtifacts()
+	})
+
+	AfterEach(func() {
+		podmanTest.CleanupPod()
+		f := CurrentGinkgoTestDescription()
+		processTestResult(f)
+
+	})
+
+	It("podman pod prune empty pod", func() {
+		_, ec, _ := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		result := podmanTest.Podman([]string{"pod", "prune"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+	})
+
+	It("podman pod prune doesn't remove a pod with a container", func() {
+		_, ec, podid := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		_, ec2, _ := podmanTest.RunLsContainerInPod("", podid)
+		Expect(ec2).To(Equal(0))
+
+		result := podmanTest.Podman([]string{"pod", "prune"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(125))
+
+		result = podmanTest.Podman([]string{"ps", "-qa"})
+		result.WaitWithDefaultTimeout()
+		Expect(len(result.OutputToStringArray())).To(Equal(1))
+	})
+
+	It("podman pod prune -f does remove a running container", func() {
+		_, ec, podid := podmanTest.CreatePod("")
+		Expect(ec).To(Equal(0))
+
+		session := podmanTest.RunTopContainerInPod("", podid)
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		result := podmanTest.Podman([]string{"pod", "prune", "-f"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.ExitCode()).To(Equal(0))
+
+		result = podmanTest.Podman([]string{"ps", "-q"})
+		result.WaitWithDefaultTimeout()
+		Expect(result.OutputToString()).To(BeEmpty())
+	})
+})