From 65d511c6d859f250b6e45107315436e73f125c3a Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Wed, 25 May 2022 15:34:59 -0400 Subject: 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 --- cmd/podman/common/completion.go | 10 +++++++++ cmd/podman/pods/unpause.go | 6 ++---- pkg/domain/infra/abi/pods.go | 8 +++++++ pkg/domain/infra/tunnel/pods.go | 4 ++++ test/e2e/pod_pause_test.go | 47 +++++++++++++++++++++++++++++++++++------ 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)) }) }) -- cgit v1.2.3-54-g00ecf