From 141de86862898a4b9e35c15f51031952c63c7114 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 22 Dec 2021 09:35:00 -0500 Subject: Revamp Libpod state strings for Docker compat Improve our compatibility with Docker by better handling the state strings that we print in `podman ps`. Docker capitalizes all states in `ps` (we do not) - fix this in our PS code. Also, stop normalizing ContainerStateConfigured to the "Created" state, and instead make it always be Created, with the existing Created state becoming Initialized. I didn't rename the actual states because I'm somewhat reticent to make such a large change a day before we leave for break. It's somewhat confusing that ContainerStateConfigured now returns Created, but internally and externally we're still consistent. [NO NEW TESTS NEEDED] existing tests should catch anything that broke. I also consider this a breaking change. I will flag appropriately on Github. Fixes RHBZ#2010432 and RHBZ#2032561 Signed-off-by: Matthew Heon --- cmd/podman/containers/ps.go | 5 ++--- libpod/define/containerstate.go | 15 +++++++++++---- test/apiv2/20-containers.at | 4 ++-- test/e2e/init_test.go | 10 +++++----- test/e2e/pod_pause_test.go | 2 +- test/system/040-ps.bats | 2 +- test/system/080-pause.bats | 2 +- test/upgrade/test-upgrade.bats | 2 +- 8 files changed, 24 insertions(+), 18 deletions(-) diff --git a/cmd/podman/containers/ps.go b/cmd/podman/containers/ps.go index c8a03ab4f..f6ba3c1f3 100644 --- a/cmd/podman/containers/ps.go +++ b/cmd/podman/containers/ps.go @@ -358,13 +358,12 @@ func (l psReporter) State() string { case "running": t := units.HumanDuration(time.Since(time.Unix(l.StartedAt, 0))) state = "Up " + t + " ago" - case "configured": - state = "Created" case "exited", "stopped": t := units.HumanDuration(time.Since(time.Unix(l.ExitedAt, 0))) state = fmt.Sprintf("Exited (%d) %s ago", l.ExitCode, t) default: - state = l.ListContainer.State + // Need to capitalize the first letter to match Docker. + state = strings.Title(l.ListContainer.State) } return state } diff --git a/libpod/define/containerstate.go b/libpod/define/containerstate.go index fc272beaa..23ba1f451 100644 --- a/libpod/define/containerstate.go +++ b/libpod/define/containerstate.go @@ -37,16 +37,23 @@ const ( ContainerStateStopping ContainerStatus = iota ) -// ContainerStatus returns a string representation for users -// of a container state +// ContainerStatus returns a string representation for users of a container +// state. All results should match Docker's versions (from `docker ps`) as +// closely as possible, given the different set of states we support. func (t ContainerStatus) String() string { switch t { case ContainerStateUnknown: return "unknown" case ContainerStateConfigured: - return "configured" - case ContainerStateCreated: + // The naming here is confusing, but it's necessary for Docker + // compatibility - their Created state is our Configured state. return "created" + case ContainerStateCreated: + // Docker does not have an equivalent to this state, so give it + // a clear name. Most of the time this is a purely transitory + // state between Configured and Running so we don't expect to + // see it much anyways. + return "initialized" case ContainerStateRunning: return "running" case ContainerStateStopped: diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 3d67650d2..72003984f 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -156,7 +156,7 @@ podman create -t -i --name myctr $IMAGE ls # Check configuration before initializing t GET libpod/containers/myctr/json 200 \ .Id~[0-9a-f]\\{64\\} \ - .State.Status="configured" \ + .State.Status="created" \ .State.Pid=0 \ .ResolvConfPath="" \ .HostnamePath="" \ @@ -172,7 +172,7 @@ t POST libpod/containers/myctr/init 204 # Check configuration after initializing t GET libpod/containers/myctr/json 200 \ .Id~[0-9a-f]\\{64\\} \ - .State.Status="created" \ + .State.Status="initialized" \ .State.Pid~[0-9]\\{1\,8\\} \ .ResolvConfPath=$userdata_path/resolv.conf \ .HostnamePath=$userdata_path/hostname \ diff --git a/test/e2e/init_test.go b/test/e2e/init_test.go index 788b67154..729cc7a5a 100644 --- a/test/e2e/init_test.go +++ b/test/e2e/init_test.go @@ -57,7 +57,7 @@ var _ = Describe("Podman init", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() - Expect(conData[0].State.Status).To(Equal("created")) + Expect(conData[0].State.Status).To(Equal("initialized")) }) It("podman init single container by name", func() { @@ -72,7 +72,7 @@ var _ = Describe("Podman init", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() - Expect(conData[0].State.Status).To(Equal("created")) + Expect(conData[0].State.Status).To(Equal("initialized")) }) It("podman init latest container", func() { @@ -87,7 +87,7 @@ var _ = Describe("Podman init", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() - Expect(conData[0].State.Status).To(Equal("created")) + Expect(conData[0].State.Status).To(Equal("initialized")) }) It("podman init all three containers, one running", func() { @@ -107,12 +107,12 @@ var _ = Describe("Podman init", func() { result.WaitWithDefaultTimeout() Expect(result).Should(Exit(0)) conData := result.InspectContainerToJSON() - Expect(conData[0].State.Status).To(Equal("created")) + Expect(conData[0].State.Status).To(Equal("initialized")) result2 := podmanTest.Podman([]string{"inspect", "test2"}) result2.WaitWithDefaultTimeout() Expect(result2).Should(Exit(0)) conData2 := result2.InspectContainerToJSON() - Expect(conData2[0].State.Status).To(Equal("created")) + Expect(conData2[0].State.Status).To(Equal("initialized")) result3 := podmanTest.Podman([]string{"inspect", "test3"}) result3.WaitWithDefaultTimeout() Expect(result3).Should(Exit(0)) diff --git a/test/e2e/pod_pause_test.go b/test/e2e/pod_pause_test.go index 7c092ba8e..daf706ec4 100644 --- a/test/e2e/pod_pause_test.go +++ b/test/e2e/pod_pause_test.go @@ -16,7 +16,7 @@ var _ = Describe("Podman pod pause", func() { podmanTest *PodmanTestIntegration ) - pausedState := "paused" + pausedState := "Paused" BeforeEach(func() { SkipIfRootlessCgroupsV1("Pause is not supported in cgroups v1") diff --git a/test/system/040-ps.bats b/test/system/040-ps.bats index 61b290415..8d0a405d2 100644 --- a/test/system/040-ps.bats +++ b/test/system/040-ps.bats @@ -110,7 +110,7 @@ EOF run_podman ps --external is "${#lines[@]}" "3" "podman ps -a --external sees buildah containers" is "${lines[1]}" \ - "[0-9a-f]\{12\} \+$IMAGE *buildah .* seconds ago .* storage .* ${PODMAN_TEST_IMAGE_NAME}-working-container" \ + "[0-9a-f]\{12\} \+$IMAGE *buildah .* seconds ago .* Storage .* ${PODMAN_TEST_IMAGE_NAME}-working-container" \ "podman ps --external" # 'rm -a' should be a NOP diff --git a/test/system/080-pause.bats b/test/system/080-pause.bats index 57f390a74..deaf8d831 100644 --- a/test/system/080-pause.bats +++ b/test/system/080-pause.bats @@ -27,7 +27,7 @@ load helpers is "$output" "paused" "podman inspect .State.Status" sleep 3 run_podman ps -a --format '{{.ID}} {{.Names}} {{.Status}}' - is "$output" "${cid:0:12} $cname paused" "podman ps on paused container" + is "$output" "${cid:0:12} $cname Paused" "podman ps on paused container" run_podman unpause $cname run_podman ps -a --format '{{.ID}} {{.Names}} {{.Status}}' is "$output" "${cid:0:12} $cname Up .*" "podman ps on resumed container" diff --git a/test/upgrade/test-upgrade.bats b/test/upgrade/test-upgrade.bats index 5d2435ab9..58e4fe0a3 100644 --- a/test/upgrade/test-upgrade.bats +++ b/test/upgrade/test-upgrade.bats @@ -223,7 +223,7 @@ EOF @test "inspect - all container status" { tests=" running | running | 0 -created | configured | 0 +created | created | 0 done | exited | 0 failed | exited | 17 " -- cgit v1.2.3-54-g00ecf