From 97b16aecf9275f4103c2737b79d0c5e81583aa58 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Fri, 5 Jan 2018 04:33:42 +0000 Subject: [PATCH 1/2] Fix issue of filter in `docker ps` where `health=starting` returns nothing This fix tries to address the issue raised in 35920 where the filter of `docker ps` with `health=starting` always returns nothing. The issue was that in container view, the human readable string (`HealthString()` => `Health.String()`) of health status was used. In case of starting it is `"health: starting"`. However, the filter still uses `starting` so no match returned. This fix fixes the issue by using `container.Health.Status()` instead so that it matches the string (`starting`) passed by filter. This fix fixes 35920. Signed-off-by: Yong Tang --- container/view.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/container/view.go b/container/view.go index 164827c550..913d9e7470 100644 --- a/container/view.go +++ b/container/view.go @@ -295,6 +295,10 @@ func (v *memdbView) GetAllNames() map[string][]string { // transform maps a (deep) copied Container object to what queries need. // A lock on the Container is not held because these are immutable deep copies. func (v *memdbView) transform(container *Container) *Snapshot { + health := types.NoHealthcheck + if container.Health != nil { + health = container.Health.Status() + } snapshot := &Snapshot{ Container: types.Container{ ID: container.ID, @@ -313,7 +317,7 @@ func (v *memdbView) transform(container *Container) *Snapshot { Managed: container.Managed, ExposedPorts: make(nat.PortSet), PortBindings: make(nat.PortSet), - Health: container.HealthString(), + Health: health, Running: container.Running, Paused: container.Paused, ExitCode: container.ExitCode(), From f509a54bdd29b8f65c17097fde3664c6fad36c21 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sun, 7 Jan 2018 05:10:36 +0000 Subject: [PATCH 2/2] Add test case for `docker ps -f health=starting` Signed-off-by: Yong Tang --- container/state.go | 9 --------- container/view_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/container/state.go b/container/state.go index 3af4015ed9..81c3b45948 100644 --- a/container/state.go +++ b/container/state.go @@ -102,15 +102,6 @@ func (s *State) String() string { return fmt.Sprintf("Exited (%d) %s ago", s.ExitCodeValue, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) } -// HealthString returns a single string to describe health status. -func (s *State) HealthString() string { - if s.Health == nil { - return types.NoHealthcheck - } - - return s.Health.String() -} - // IsValidHealthString checks if the provided string is a valid container health status or not. func IsValidHealthString(s string) bool { return s == types.Starting || diff --git a/container/view_test.go b/container/view_test.go index 4535a8fade..c425aaf731 100644 --- a/container/view_test.go +++ b/container/view_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "testing" + "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" "github.com/pborman/uuid" "github.com/stretchr/testify/assert" @@ -159,3 +160,26 @@ func TestNames(t *testing.T) { view = db.Snapshot() assert.Equal(t, map[string][]string{"containerid4": {"name1", "name2"}}, view.GetAllNames()) } + +// Test case for GitHub issue 35920 +func TestViewWithHealthCheck(t *testing.T) { + var ( + db, _ = NewViewDB() + one = newContainer(t) + ) + one.Health = &Health{ + Health: types.Health{ + Status: "starting", + }, + } + if err := one.CheckpointTo(db); err != nil { + t.Fatal(err) + } + s, err := db.Snapshot().Get(one.ID) + if err != nil { + t.Fatal(err) + } + if s == nil || s.Health != "starting" { + t.Fatalf("expected Health=starting. Got: %+v", s) + } +}