Fix podman pod unpaue TODO

Update the podman pod unpause to only show the paused
containers with autocomplete.
Fix a typo in the help command.
Update the unpause function to only attempt an unpause
on pasued pods instead of all the pods.
Update the tests accordingly.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>

5 files changed, 65 insertions, 10 deletions

diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go
index 89e53c180..ae30f2875 100644
--- a/cmd/podman/common/completion.go
+++ b/cmd/podman/common/completion.go
@@ -520,6 +520,16 @@ func AutocompletePodsRunning(cmd *cobra.Command, args []string, toComplete strin
 	return getPods(cmd, toComplete, completeDefault, "running", "degraded")
 }
 
+// AutoCompletePodsPause - Autocomplete only paused pod names
+// When a pod has a few containers paused, that ends up in degraded state
+// So autocomplete degraded pod names as well
+func AutoCompletePodsPause(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+	if !validCurrentCmdLine(cmd, args, toComplete) {
+		return nil, cobra.ShellCompDirectiveNoFileComp
+	}
+	return getPods(cmd, toComplete, completeDefault, "paused", "degraded")
+}
+
 // AutocompleteForKube - Autocomplete all Podman objects supported by kube generate.
 func AutocompleteForKube(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	if !validCurrentCmdLine(cmd, args, toComplete) {
diff --git a/cmd/podman/pods/unpause.go b/cmd/podman/pods/unpause.go
index 8a0a24e98..47b29458b 100644
--- a/cmd/podman/pods/unpause.go
+++ b/cmd/podman/pods/unpause.go
@@ -24,9 +24,7 @@ var (
 		Args: func(cmd *cobra.Command, args []string) error {
 			return validate.CheckAllLatestAndIDFile(cmd, args, false, "")
 		},
-		// TODO have a function which shows only pods which could be unpaused
-		// for now show all
-		ValidArgsFunction: common.AutocompletePods,
+		ValidArgsFunction: common.AutoCompletePodsPause,
 		Example: `podman pod unpause podID1 podID2
   podman pod unpause --all
   podman pod unpause --latest`,
@@ -43,7 +41,7 @@ func init() {
 		Parent:  podCmd,
 	})
 	flags := unpauseCommand.Flags()
-	flags.BoolVarP(&unpauseOptions.All, "all", "a", false, "Pause all running pods")
+	flags.BoolVarP(&unpauseOptions.All, "all", "a", false, "Unpause all running pods")
 	validate.AddLatestFlag(unpauseCommand, &unpauseOptions.Latest)
 }
 
diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go
index 1dca8c580..d287ebe08 100644
--- a/pkg/domain/infra/abi/pods.go
+++ b/pkg/domain/infra/abi/pods.go
@@ -158,6 +158,14 @@ func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string,
 		return nil, err
 	}
 	for _, p := range pods {
+		status, err := p.GetPodStatus()
+		if err != nil {
+			return nil, err
+		}
+		// If the pod is not paused or degraded, there is no need to attempt an unpause on it
+		if status != define.PodStatePaused && status != define.PodStateDegraded {
+			continue
+		}
 		report := entities.PodUnpauseReport{Id: p.ID()}
 		errs, err := p.Unpause(ctx)
 		if err != nil && errors.Cause(err) != define.ErrPodPartialFail {
diff --git a/pkg/domain/infra/tunnel/pods.go b/pkg/domain/infra/tunnel/pods.go
index 7b1fa231f..d52060347 100644
--- a/pkg/domain/infra/tunnel/pods.go
+++ b/pkg/domain/infra/tunnel/pods.go
@@ -80,6 +80,10 @@ func (ic *ContainerEngine) PodUnpause(ctx context.Context, namesOrIds []string,
 	}
 	reports := make([]*entities.PodUnpauseReport, 0, len(foundPods))
 	for _, p := range foundPods {
+		// If the pod is not paused or degraded, there is no need to attempt an unpause on it
+		if p.Status != define.PodStatePaused && p.Status != define.PodStateDegraded {
+			continue
+		}
 		response, err := pods.Unpause(ic.ClientCtx, p.Id, nil)
 		if err != nil {
 			report := entities.PodUnpauseReport{
diff --git a/test/e2e/pod_pause_test.go b/test/e2e/pod_pause_test.go
index d78890347..39e5696fa 100644
--- a/test/e2e/pod_pause_test.go
+++ b/test/e2e/pod_pause_test.go
@@ -56,7 +56,7 @@ var _ = Describe("Podman pod pause", func() {
 		Expect(result).Should(Exit(0))
 	})
 
-	It("podman pod pause a running pod by id", func() {
+	It("pause a running pod by id", func() {
 		_, ec, podid := podmanTest.CreatePod(nil)
 		Expect(ec).To(Equal(0))
 
@@ -73,11 +73,10 @@ var _ = Describe("Podman pod pause", func() {
 
 		result = podmanTest.Podman([]string{"pod", "unpause", podid})
 		result.WaitWithDefaultTimeout()
-
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
 	})
 
-	It("podman unpause a running pod by id", func() {
+	It("unpause a paused pod by id", func() {
 		_, ec, podid := podmanTest.CreatePod(nil)
 		Expect(ec).To(Equal(0))
 
@@ -85,14 +84,21 @@ var _ = Describe("Podman pod pause", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
-		result := podmanTest.Podman([]string{"pod", "unpause", podid})
+		result := podmanTest.Podman([]string{"pod", "pause", podid})
 		result.WaitWithDefaultTimeout()
+		Expect(result).Should(Exit(0))
+		Expect(result.OutputToStringArray()).Should(HaveLen(1))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
+		Expect(podmanTest.GetContainerStatus()).To(Equal(pausedState))
 
+		result = podmanTest.Podman([]string{"pod", "unpause", podid})
+		result.WaitWithDefaultTimeout()
 		Expect(result).Should(Exit(0))
+		Expect(result.OutputToStringArray()).Should(HaveLen(1))
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
 	})
 
-	It("podman pod pause a running pod by name", func() {
+	It("unpause a paused pod by name", func() {
 		_, ec, _ := podmanTest.CreatePod(map[string][]string{"--name": {"test1"}})
 		Expect(ec).To(Equal(0))
 
@@ -102,13 +108,42 @@ var _ = Describe("Podman pod pause", func() {
 
 		result := podmanTest.Podman([]string{"pod", "pause", "test1"})
 		result.WaitWithDefaultTimeout()
-
 		Expect(result).Should(Exit(0))
+		Expect(result.OutputToStringArray()).Should(HaveLen(1))
 		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0))
 		Expect(podmanTest.GetContainerStatus()).To(Equal(pausedState))
 
 		result = podmanTest.Podman([]string{"pod", "unpause", "test1"})
 		result.WaitWithDefaultTimeout()
 		Expect(result).Should(Exit(0))
+		Expect(result.OutputToStringArray()).Should(HaveLen(1))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
+	})
+
+	It("unpause --all", func() {
+		_, ec, podid1 := podmanTest.CreatePod(nil)
+		Expect(ec).To(Equal(0))
+		_, ec, podid2 := podmanTest.CreatePod(nil)
+		Expect(ec).To(Equal(0))
+
+		session := podmanTest.RunTopContainerInPod("", podid1)
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		session = podmanTest.RunTopContainerInPod("", podid2)
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		result := podmanTest.Podman([]string{"pod", "pause", podid1})
+		result.WaitWithDefaultTimeout()
+		Expect(result).Should(Exit(0))
+		Expect(result.OutputToStringArray()).Should(HaveLen(1))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(1))
+		Expect(podmanTest.GetContainerStatus()).To(ContainSubstring(pausedState))
+
+		result = podmanTest.Podman([]string{"pod", "unpause", "--all"})
+		result.WaitWithDefaultTimeout()
+		Expect(result).Should(Exit(0))
+		Expect(result.OutputToStringArray()).Should(HaveLen(1))
+		Expect(podmanTest.NumberOfContainersRunning()).To(Equal(2))
 	})
 })