Refactored method of getting pods

Now, for commands that have --latest and --all, the context flags are checked, and pods are grabbed in a single function

Signed-off-by: haircommander <pehunt@redhat.com>

Closes: #1161
Approved by: rhatdan

6 files changed, 65 insertions, 153 deletions

diff --git a/cmd/podman/pod_kill.go b/cmd/podman/pod_kill.go
index 43924f8a0..48f4eaa7a 100644
--- a/cmd/podman/pod_kill.go
+++ b/cmd/podman/pod_kill.go
@@ -7,7 +7,6 @@ import (
 	"github.com/docker/docker/pkg/signal"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/cmd/podman/libpodruntime"
-	"github.com/projectatomic/libpod/libpod"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -49,10 +48,7 @@ func podKillCmd(c *cli.Context) error {
 	}
 	defer runtime.Shutdown(false)
 
-	args := c.Args()
 	var killSignal uint = uint(syscall.SIGTERM)
-	var lastError error
-	var pods []*libpod.Pod
 
 	if c.String("signal") != "" {
 		// Check if the signalString provided by the user is valid
@@ -64,31 +60,10 @@ func podKillCmd(c *cli.Context) error {
 		killSignal = uint(sysSignal)
 	}
 
-	if c.Bool("all") {
-		pods, err = runtime.Pods()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get pods")
-		}
-	}
-	if c.Bool("latest") {
-		pod, err := runtime.GetLatestPod()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get latest pod")
-		}
-		pods = append(pods, pod)
-	}
-	for _, i := range args {
-		pod, err := runtime.LookupPod(i)
-		if err != nil {
-			logrus.Errorf("%q", lastError)
-			if lastError != nil {
-				logrus.Errorf("%q", lastError)
-			}
-			lastError = errors.Wrapf(err, "unable to find pods %s", i)
-			continue
-		}
-		pods = append(pods, pod)
-	}
+	// getPodsFromContext returns an error when a requested pod
+	// isn't found. The only fatal error scenerio is when there are no pods
+	// in which case the following loop will be skipped.
+	pods, lastError := getPodsFromContext(c, runtime)
 
 	for _, pod := range pods {
 		ctr_errs, err := pod.Kill(killSignal)
diff --git a/cmd/podman/pod_restart.go b/cmd/podman/pod_restart.go
index a4b121009..fe802fc9d 100644
--- a/cmd/podman/pod_restart.go
+++ b/cmd/podman/pod_restart.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/cmd/podman/libpodruntime"
-	"github.com/projectatomic/libpod/libpod"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -42,36 +41,10 @@ func podRestartCmd(c *cli.Context) error {
 	}
 	defer runtime.Shutdown(false)
 
-	args := c.Args()
-	var pods []*libpod.Pod
-	var lastError error
-
-	if c.Bool("all") {
-		pods, err = runtime.Pods()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get running pods")
-		}
-	}
-
-	if c.Bool("latest") {
-		pod, err := runtime.GetLatestPod()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get latest pod")
-		}
-		pods = append(pods, pod)
-	}
-
-	for _, i := range args {
-		pod, err := runtime.LookupPod(i)
-		if err != nil {
-			if lastError != nil {
-				logrus.Errorf("%q", lastError)
-			}
-			lastError = errors.Wrapf(err, "unable to find pod %s", i)
-			continue
-		}
-		pods = append(pods, pod)
-	}
+	// getPodsFromContext returns an error when a requested pod
+	// isn't found. The only fatal error scenerio is when there are no pods
+	// in which case the following loop will be skipped.
+	pods, lastError := getPodsFromContext(c, runtime)
 
 	ctx := getContext()
 	for _, pod := range pods {
diff --git a/cmd/podman/pod_rm.go b/cmd/podman/pod_rm.go
index 362e23368..983203374 100644
--- a/cmd/podman/pod_rm.go
+++ b/cmd/podman/pod_rm.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/cmd/podman/libpodruntime"
-	"github.com/projectatomic/libpod/libpod"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -43,46 +42,22 @@ func podRmCmd(c *cli.Context) error {
 	if err := checkMutuallyExclusiveFlags(c); err != nil {
 		return err
 	}
+
 	runtime, err := libpodruntime.GetRuntime(c)
 	if err != nil {
 		return errors.Wrapf(err, "could not get runtime")
 	}
 	defer runtime.Shutdown(false)
 
-	args := c.Args()
 	ctx := getContext()
-	var delPods []*libpod.Pod
-	var lastError error
-	if c.Bool("all") {
-		delPods, err = runtime.GetAllPods()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get pod list")
-		}
-	}
-
-	if c.Bool("latest") {
-		delPod, err := runtime.GetLatestPod()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get latest pod")
-		}
-		delPods = append(delPods, delPod)
-	}
-
-	for _, i := range args {
-		pod, err := runtime.LookupPod(i)
-		if err != nil {
-			logrus.Errorf("%q", lastError)
-			if lastError != nil {
-				logrus.Errorf("%q", lastError)
-			}
-			lastError = errors.Wrapf(err, "unable to find pods %s", i)
-			continue
-		}
-		delPods = append(delPods, pod)
-	}
 	force := c.Bool("force")
 
-	for _, pod := range delPods {
+	// getPodsFromContext returns an error when a requested pod
+	// isn't found. The only fatal error scenerio is when there are no pods
+	// in which case the following loop will be skipped.
+	pods, lastError := getPodsFromContext(c, runtime)
+
+	for _, pod := range pods {
 		err = runtime.RemovePod(ctx, pod, force, force)
 		if err != nil {
 			if lastError != nil {
diff --git a/cmd/podman/pod_start.go b/cmd/podman/pod_start.go
index 0ca695a21..e35ec7993 100644
--- a/cmd/podman/pod_start.go
+++ b/cmd/podman/pod_start.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/cmd/podman/libpodruntime"
-	"github.com/projectatomic/libpod/libpod"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -42,38 +41,14 @@ func podStartCmd(c *cli.Context) error {
 
 	runtime, err := libpodruntime.GetRuntime(c)
 	if err != nil {
-		return errors.Wrapf(err, "error creating libpod runtime")
+		return errors.Wrapf(err, "could not get runtime")
 	}
 	defer runtime.Shutdown(false)
 
-	args := c.Args()
-	var pods []*libpod.Pod
-	var lastError error
-
-	if c.Bool("all") {
-		pods, err = runtime.Pods()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get running pods")
-		}
-	}
-	if c.Bool("latest") {
-		pod, err := runtime.GetLatestPod()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get latest pod")
-		}
-		pods = append(pods, pod)
-	}
-	for _, i := range args {
-		pod, err := runtime.LookupPod(i)
-		if err != nil {
-			if lastError != nil {
-				logrus.Errorf("%q", lastError)
-			}
-			lastError = errors.Wrapf(err, "unable to find pod %s", i)
-			continue
-		}
-		pods = append(pods, pod)
-	}
+	// getPodsFromContext returns an error when a requested pod
+	// isn't found. The only fatal error scenerio is when there are no pods
+	// in which case the following loop will be skipped.
+	pods, lastError := getPodsFromContext(c, runtime)
 
 	ctx := getContext()
 	for _, pod := range pods {
diff --git a/cmd/podman/pod_stop.go b/cmd/podman/pod_stop.go
index 85904ebf3..97d248b30 100644
--- a/cmd/podman/pod_stop.go
+++ b/cmd/podman/pod_stop.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/cmd/podman/libpodruntime"
-	"github.com/projectatomic/libpod/libpod"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 )
@@ -45,36 +44,10 @@ func podStopCmd(c *cli.Context) error {
 	}
 	defer runtime.Shutdown(false)
 
-	args := c.Args()
-	var pods []*libpod.Pod
-	var lastError error
-
-	if c.Bool("all") {
-		pods, err = runtime.Pods()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get running pods")
-		}
-	}
-
-	if c.Bool("latest") {
-		pod, err := runtime.GetLatestPod()
-		if err != nil {
-			return errors.Wrapf(err, "unable to get latest pod")
-		}
-		pods = append(pods, pod)
-	}
-
-	for _, i := range args {
-		pod, err := runtime.LookupPod(i)
-		if err != nil {
-			if lastError != nil {
-				logrus.Errorf("%q", lastError)
-			}
-			lastError = errors.Wrapf(err, "unable to find pod %s", i)
-			continue
-		}
-		pods = append(pods, pod)
-	}
+	// getPodsFromContext returns an error when a requested pod
+	// isn't found. The only fatal error scenerio is when there are no pods
+	// in which case the following loop will be skipped.
+	pods, lastError := getPodsFromContext(c, runtime)
 
 	for _, pod := range pods {
 		// set cleanup to true to clean mounts and namespaces
diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go
index 9d9e760d6..2d19e312c 100644
--- a/cmd/podman/utils.go
+++ b/cmd/podman/utils.go
@@ -174,3 +174,44 @@ func checkMutuallyExclusiveFlags(c *cli.Context) error {
 	}
 	return nil
 }
+
+// For pod commands that have a latest and all flag, getPodsFromContext gets
+// pods the user specifies. If there's an error before getting pods, the pods slice
+// will be empty and error will be not nil. If an error occured after, the pod slice
+// will hold all of the successful pods, and error will hold the last error.
+// The remaining errors will be logged. On success, pods will hold all pods and
+// error will be nil.
+func getPodsFromContext(c *cli.Context, r *libpod.Runtime) ([]*libpod.Pod, error) {
+	args := c.Args()
+	var pods []*libpod.Pod
+	var lastError error
+	var err error
+
+	if c.Bool("all") {
+		pods, err = r.Pods()
+		if err != nil {
+			return nil, errors.Wrapf(err, "unable to get running pods")
+		}
+	}
+
+	if c.Bool("latest") {
+		pod, err := r.GetLatestPod()
+		if err != nil {
+			return nil, errors.Wrapf(err, "unable to get latest pod")
+		}
+		pods = append(pods, pod)
+	}
+
+	for _, i := range args {
+		pod, err := r.LookupPod(i)
+		if err != nil {
+			if lastError != nil {
+				logrus.Errorf("%q", lastError)
+			}
+			lastError = errors.Wrapf(err, "unable to find pod %s", i)
+			continue
+		}
+		pods = append(pods, pod)
+	}
+	return pods, lastError
+}