From 69962682e990ddb9437291b98bd335e74c090fc8 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 18 Apr 2019 10:34:27 -0700 Subject: Refactor of 'podman prune' to better support remote * Push iterations into the service not the client * Add e2e tests * Refactor to use new frameworks Signed-off-by: Jhon Honce --- cmd/podman/cliconfig/config.go | 2 +- cmd/podman/pods_prune.go | 64 ++++++++-------------------------- cmd/podman/system_prune.go | 10 +++++- pkg/adapter/pods.go | 51 +++++++++++++++++++++++---- pkg/adapter/pods_remote.go | 44 +++++++++++++++++------- pkg/adapter/runtime.go | 28 +++++---------- pkg/adapter/runtime_remote.go | 12 +++++++ test/e2e/pod_prune_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 200 insertions(+), 89 deletions(-) create mode 100644 test/e2e/pod_prune_test.go diff --git a/cmd/podman/cliconfig/config.go b/cmd/podman/cliconfig/config.go index 16c98a13e..640a4bff4 100644 --- a/cmd/podman/cliconfig/config.go +++ b/cmd/podman/cliconfig/config.go @@ -159,7 +159,7 @@ type PruneContainersValues struct { Force bool } -type PrunePodsValues struct { +type PodPruneValues struct { PodmanCommand Force bool } diff --git a/cmd/podman/pods_prune.go b/cmd/podman/pods_prune.go index 89401a98a..e6946f068 100644 --- a/cmd/podman/pods_prune.go +++ b/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) } diff --git a/cmd/podman/system_prune.go b/cmd/podman/system_prune.go index 14cb96941..8900e2644 100644 --- a/cmd/podman/system_prune.go +++ b/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()) diff --git a/pkg/adapter/pods.go b/pkg/adapter/pods.go index 901c1857b..bb7d9cce6 100644 --- a/pkg/adapter/pods.go +++ b/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 ( diff --git a/pkg/adapter/pods_remote.go b/pkg/adapter/pods_remote.go index 00a5d9a32..7cf38aac0 100644 --- a/pkg/adapter/pods_remote.go +++ b/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 +} diff --git a/pkg/adapter/runtime.go b/pkg/adapter/runtime.go index 6ed9cee77..753f7c944 100644 --- a/pkg/adapter/runtime.go +++ b/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 } diff --git a/pkg/adapter/runtime_remote.go b/pkg/adapter/runtime_remote.go index 71f7380db..dcb0924ce 100644 --- a/pkg/adapter/runtime_remote.go +++ b/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 diff --git a/test/e2e/pod_prune_test.go b/test/e2e/pod_prune_test.go new file mode 100644 index 000000000..c20f602ad --- /dev/null +++ b/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()) + }) +}) -- cgit v1.2.3-54-g00ecf