From 1fd87e9fdf5da56e33dac97fcb1a8a3caad0114d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 7 Nov 2025 18:18:25 +0100 Subject: [PATCH 1/2] api/types/container: make ContainerState a concrete type Signed-off-by: Sebastiaan van Stijn --- api/types/container/state.go | 14 +++++++++----- api/types/container/state_test.go | 2 +- daemon/container/state.go | 12 ++++++------ daemon/container/view.go | 2 +- daemon/container_operations.go | 2 +- daemon/delete.go | 2 +- daemon/inspect.go | 2 +- daemon/internal/metrics/metrics.go | 1 + daemon/list.go | 4 ++-- daemon/monitor.go | 7 ++++--- integration/container/kill_test.go | 6 +++--- integration/internal/container/states.go | 6 ++++-- .../moby/moby/api/types/container/state.go | 14 +++++++++----- 13 files changed, 43 insertions(+), 31 deletions(-) diff --git a/api/types/container/state.go b/api/types/container/state.go index 6de2df66ab..47c6d12490 100644 --- a/api/types/container/state.go +++ b/api/types/container/state.go @@ -6,9 +6,7 @@ import ( ) // ContainerState is a string representation of the container's current state. -// -// It currently is an alias for string, but may become a distinct type in the future. -type ContainerState = string +type ContainerState string const ( StateCreated ContainerState = "created" // StateCreated indicates the container is created, but not (yet) started. @@ -20,8 +18,14 @@ const ( StateDead ContainerState = "dead" // StateDead indicates that the container failed to be deleted. Containers in this state are attempted to be cleaned up when the daemon restarts. ) -var validStates = []ContainerState{ - StateCreated, StateRunning, StatePaused, StateRestarting, StateRemoving, StateExited, StateDead, +var validStates = []string{ + string(StateCreated), + string(StateRunning), + string(StatePaused), + string(StateRestarting), + string(StateRemoving), + string(StateExited), + string(StateDead), } // ValidateContainerState checks if the provided string is a valid diff --git a/api/types/container/state_test.go b/api/types/container/state_test.go index 79b7984c98..afeb803dbe 100644 --- a/api/types/container/state_test.go +++ b/api/types/container/state_test.go @@ -21,7 +21,7 @@ func TestValidateContainerState(t *testing.T) { {state: "invalid-state-string", expectedErr: `invalid value for state (invalid-state-string): must be one of created, running, paused, restarting, removing, exited, dead`}, } for _, tc := range tests { - t.Run(tc.state, func(t *testing.T) { + t.Run(string(tc.state), func(t *testing.T) { err := ValidateContainerState(tc.state) if tc.expectedErr == "" { assert.NilError(t, err) diff --git a/daemon/container/state.go b/daemon/container/state.go index 1ddc2f93d5..0cec03c258 100644 --- a/daemon/container/state.go +++ b/daemon/container/state.go @@ -31,7 +31,7 @@ type State struct { // // In a similar fashion, [State.Running] and [State.Restarting] can both // be true in a situation where a container is in process of being restarted. - // Refer to [State.StateString] for order of precedence. + // Refer to [State.State] for order of precedence. Running bool Paused bool Restarting bool @@ -114,10 +114,10 @@ func (s *State) String() string { return fmt.Sprintf("Exited (%d) %s ago", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) } -// StateString returns the container's current [ContainerState], based on the +// State returns the container's current [container.ContainerState], based on the // [State.Running], [State.Paused], [State.Restarting], [State.RemovalInProgress], // [State.StartedAt] and [State.Dead] fields. -func (s *State) StateString() container.ContainerState { +func (s *State) State() container.ContainerState { if s.Running { if s.Paused { return container.StatePaused @@ -221,7 +221,7 @@ func (s *State) conditionAlreadyMet(condition container.WaitCondition) bool { // // In a similar fashion, [State.Running] and [State.Restarting] can both // be true in a situation where a container is in process of being restarted. -// Refer to [State.StateString] for order of precedence. +// Refer to [State.State] for order of precedence. func (s *State) IsRunning() bool { s.Lock() defer s.Unlock() @@ -327,7 +327,7 @@ func (s *State) SetError(err error) { // // In a similar fashion, [State.Running] and [State.Restarting] can both // be true in a situation where a container is in process of being restarted. -// Refer to [State.StateString] for order of precedence. +// Refer to [State.State] for order of precedence. func (s *State) IsPaused() bool { s.Lock() defer s.Unlock() @@ -346,7 +346,7 @@ func (s *State) IsPaused() bool { // // In a similar fashion, [State.Running] and [State.Restarting] can both // be true in a situation where a container is in process of being restarted. -// Refer to [State.StateString] for order of precedence. +// Refer to [State.State] for order of precedence. func (s *State) IsRestarting() bool { s.Lock() defer s.Unlock() diff --git a/daemon/container/view.go b/daemon/container/view.go index 41b8cdeff8..c0d23ecff8 100644 --- a/daemon/container/view.go +++ b/daemon/container/view.go @@ -305,7 +305,7 @@ func (v *View) transform(ctr *Container) *Snapshot { ImageID: ctr.ImageID.String(), Ports: []container.PortSummary{}, Mounts: ctr.GetMountPoints(), - State: ctr.State.StateString(), + State: ctr.State.State(), Status: ctr.State.String(), Health: &container.HealthSummary{ Status: health, diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 1efffb8712..1ed84bb04e 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -968,7 +968,7 @@ func (daemon *Daemon) getNetworkedContainer(containerID, connectedContainerPrefi return nil, errdefs.System(errdefs.InvalidParameter(errors.New("cannot join own network namespace"))) } if !nc.State.IsRunning() { - return nil, errdefs.Conflict(fmt.Errorf("cannot join network namespace of a non running container: container %s is %s", strings.TrimPrefix(nc.Name, "/"), nc.State.StateString())) + return nil, errdefs.Conflict(fmt.Errorf("cannot join network namespace of a non running container: container %s is %s", strings.TrimPrefix(nc.Name, "/"), nc.State.State())) } if nc.State.IsRestarting() { return nil, fmt.Errorf("cannot join network namespace of container: %w", errContainerIsRestarting(connectedContainerPrefixOrName)) diff --git a/daemon/delete.go b/daemon/delete.go index c5983ea90c..7bf3eeb843 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -92,7 +92,7 @@ func (daemon *Daemon) cleanupContainer(ctr *container.Container, config backend. if ctr.State.Paused { return errdefs.Conflict(errors.New("container is paused and must be unpaused first")) } else { - return errdefs.Conflict(fmt.Errorf("container is %s: stop the container before removing or force remove", ctr.State.StateString())) + return errdefs.Conflict(fmt.Errorf("container is %s: stop the container before removing or force remove", ctr.State.State())) } } if err := daemon.Kill(ctr); err != nil && !isNotRunning(err) { diff --git a/daemon/inspect.go b/daemon/inspect.go index 41dd00cdc6..284cced10d 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -125,7 +125,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co Path: ctr.Path, Args: ctr.Args, State: &containertypes.State{ - Status: ctr.State.StateString(), + Status: ctr.State.State(), Running: ctr.State.Running, Paused: ctr.State.Paused, Restarting: ctr.State.Restarting, diff --git a/daemon/internal/metrics/metrics.go b/daemon/internal/metrics/metrics.go index 7940586bf2..28b191e56b 100644 --- a/daemon/internal/metrics/metrics.go +++ b/daemon/internal/metrics/metrics.go @@ -91,6 +91,7 @@ func (ctr *StateCounter) Get() (running int, paused int, stopped int) { ctr.mu.RLock() defer ctr.mu.RUnlock() + // FIXME(thaJeztah): there's no "container.StateStopped"; should we align these states with actual states? for _, state := range ctr.states { switch state { case "running": diff --git a/daemon/list.go b/daemon/list.go index a86453eb53..11a6f0fdca 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -281,7 +281,7 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf } err = psFilters.WalkValues("status", func(value string) error { - if err := containertypes.ValidateContainerState(value); err != nil { + if err := containertypes.ValidateContainerState(containertypes.ContainerState(value)); err != nil { return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'status=%s': %w", value, err)) } config.All = true @@ -486,7 +486,7 @@ func includeContainerInList(container *container.Snapshot, filter *listContext) } // Do not include container if its status doesn't match the filter - if !filter.filters.Match("status", container.State) { + if !filter.filters.Match("status", string(container.State)) { return excludeContainer } diff --git a/daemon/monitor.go b/daemon/monitor.go index 91ee2b0469..ec6eccf017 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -8,6 +8,7 @@ import ( cerrdefs "github.com/containerd/errdefs" "github.com/containerd/log" + containertypes "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/events" "github.com/moby/moby/v2/daemon/config" "github.com/moby/moby/v2/daemon/container" @@ -19,10 +20,10 @@ import ( ) func (daemon *Daemon) setStateCounter(c *container.Container) { - switch c.State.StateString() { - case "paused": + switch c.State.State() { + case containertypes.StatePaused: metrics.StateCtr.Set(c.ID, "paused") - case "running": + case containertypes.StateRunning: metrics.StateCtr.Set(c.ID, "running") default: metrics.StateCtr.Set(c.ID, "stopped") diff --git a/integration/container/kill_test.go b/integration/container/kill_test.go index 8dd204ae15..1f01733e32 100644 --- a/integration/container/kill_test.go +++ b/integration/container/kill_test.go @@ -92,17 +92,17 @@ func TestKillWithStopSignalAndRestartPolicies(t *testing.T) { testCases := []struct { doc string stopsignal string - status string + status containertypes.ContainerState }{ { doc: "same-signal-disables-restart-policy", stopsignal: "TERM", - status: "exited", + status: containertypes.StateExited, }, { doc: "different-signal-keep-restart-policy", stopsignal: "CONT", - status: "running", + status: containertypes.StateRunning, }, } diff --git a/integration/internal/container/states.go b/integration/internal/container/states.go index 5fab52a926..3ec372f4c1 100644 --- a/integration/internal/container/states.go +++ b/integration/internal/container/states.go @@ -39,15 +39,17 @@ func IsInState(ctx context.Context, apiClient client.APIClient, containerID stri if err != nil { return poll.Error(err) } + var states []string for _, v := range state { + states = append(states, string(v)) if inspect.Container.State.Status == v { return poll.Success() } } - if len(state) == 1 { + if len(states) == 1 { return poll.Continue("waiting for container State.Status to be '%s', currently '%s'", state[0], inspect.Container.State.Status) } else { - return poll.Continue("waiting for container State.Status to be one of (%s), currently '%s'", strings.Join(state, ", "), inspect.Container.State.Status) + return poll.Continue("waiting for container State.Status to be one of (%s), currently '%s'", strings.Join(states, ", "), inspect.Container.State.Status) } } } diff --git a/vendor/github.com/moby/moby/api/types/container/state.go b/vendor/github.com/moby/moby/api/types/container/state.go index 6de2df66ab..47c6d12490 100644 --- a/vendor/github.com/moby/moby/api/types/container/state.go +++ b/vendor/github.com/moby/moby/api/types/container/state.go @@ -6,9 +6,7 @@ import ( ) // ContainerState is a string representation of the container's current state. -// -// It currently is an alias for string, but may become a distinct type in the future. -type ContainerState = string +type ContainerState string const ( StateCreated ContainerState = "created" // StateCreated indicates the container is created, but not (yet) started. @@ -20,8 +18,14 @@ const ( StateDead ContainerState = "dead" // StateDead indicates that the container failed to be deleted. Containers in this state are attempted to be cleaned up when the daemon restarts. ) -var validStates = []ContainerState{ - StateCreated, StateRunning, StatePaused, StateRestarting, StateRemoving, StateExited, StateDead, +var validStates = []string{ + string(StateCreated), + string(StateRunning), + string(StatePaused), + string(StateRestarting), + string(StateRemoving), + string(StateExited), + string(StateDead), } // ValidateContainerState checks if the provided string is a valid From db71c6a91422c8e9bbdb33afa867e1d41bf7b731 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 7 Nov 2025 18:36:12 +0100 Subject: [PATCH 2/2] api/types/container: make HealthStatus a concrete type Signed-off-by: Sebastiaan van Stijn --- api/types/container/health.go | 9 +++++---- api/types/container/health_test.go | 2 +- daemon/container/health.go | 2 +- daemon/health.go | 2 +- daemon/list.go | 4 ++-- integration-cli/docker_cli_health_test.go | 15 ++++++++------- integration/container/health_test.go | 8 ++++---- .../moby/moby/api/types/container/health.go | 9 +++++---- 8 files changed, 27 insertions(+), 24 deletions(-) diff --git a/api/types/container/health.go b/api/types/container/health.go index b7fe592cc2..1a1ba84b40 100644 --- a/api/types/container/health.go +++ b/api/types/container/health.go @@ -7,9 +7,7 @@ import ( ) // HealthStatus is a string representation of the container's health. -// -// It currently is an alias for string, but may become a distinct type in future. -type HealthStatus = string +type HealthStatus string // Health states const ( @@ -41,7 +39,10 @@ type HealthcheckResult struct { } var validHealths = []string{ - NoHealthcheck, Starting, Healthy, Unhealthy, + string(NoHealthcheck), + string(Starting), + string(Healthy), + string(Unhealthy), } // ValidateHealthStatus checks if the provided string is a valid diff --git a/api/types/container/health_test.go b/api/types/container/health_test.go index dbd1cb96b9..7cef54730e 100644 --- a/api/types/container/health_test.go +++ b/api/types/container/health_test.go @@ -19,7 +19,7 @@ func TestValidateHealthStatus(t *testing.T) { } for _, tc := range tests { - t.Run(tc.health, func(t *testing.T) { + t.Run(string(tc.health), func(t *testing.T) { err := ValidateHealthStatus(tc.health) if tc.expectedErr == "" { assert.NilError(t, err) diff --git a/daemon/container/health.go b/daemon/container/health.go index d510688479..dd2704635d 100644 --- a/daemon/container/health.go +++ b/daemon/container/health.go @@ -23,7 +23,7 @@ func (s *Health) String() string { case container.Starting: return "health: starting" default: // Healthy and Unhealthy are clear on their own - return status + return string(status) } } diff --git a/daemon/health.go b/daemon/health.go index 931f88c236..e2af023c5f 100644 --- a/daemon/health.go +++ b/daemon/health.go @@ -244,7 +244,7 @@ func handleProbeResult(d *Daemon, c *container.Container, result *containertypes current := h.Status() if oldStatus != current { - d.LogContainerEvent(c, events.Action(string(events.ActionHealthStatus)+": "+current)) + d.LogContainerEvent(c, events.Action(string(events.ActionHealthStatus)+": "+string(current))) } } diff --git a/daemon/list.go b/daemon/list.go index 11a6f0fdca..0af18ebd70 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -298,7 +298,7 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf } err = psFilters.WalkValues("health", func(value string) error { - if err := containertypes.ValidateHealthStatus(value); err != nil { + if err := containertypes.ValidateHealthStatus(containertypes.HealthStatus(value)); err != nil { return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'health=%s': %w", value, err)) } return nil @@ -491,7 +491,7 @@ func includeContainerInList(container *container.Snapshot, filter *listContext) } // Do not include container if its health doesn't match the filter - if !filter.filters.ExactMatch("health", container.Health) { + if !filter.filters.ExactMatch("health", string(container.Health)) { return excludeContainer } diff --git a/integration-cli/docker_cli_health_test.go b/integration-cli/docker_cli_health_test.go index 76a3dee0e4..8d0f5eb343 100644 --- a/integration-cli/docker_cli_health_test.go +++ b/integration-cli/docker_cli_health_test.go @@ -26,16 +26,17 @@ func (s *DockerCLIHealthSuite) OnTimeout(t *testing.T) { s.ds.OnTimeout(t) } -func waitForHealthStatus(t *testing.T, name string, prev string, expected string) { - prev = prev + "\n" - expected = expected + "\n" +func waitForHealthStatus(t *testing.T, name string, prev container.HealthStatus, expected container.HealthStatus) { for { out := cli.DockerCmd(t, "inspect", "--format={{.State.Health.Status}}", name).Stdout() - if out == expected { + actual := container.HealthStatus(strings.TrimSpace(out)) + if actual == expected { return } - assert.Equal(t, out, prev) - if out != prev { + + // TODO(thaJeztah): this logic seems broken? assert.Assert would make it fail, so why the "actual != prev"? + assert.Equal(t, actual, prev) + if actual != prev { return } time.Sleep(100 * time.Millisecond) @@ -84,7 +85,7 @@ func (s *DockerCLIHealthSuite) TestHealth(c *testing.T) { // Inspect the status out = cli.DockerCmd(c, "inspect", "--format={{.State.Health.Status}}", name).Stdout() - assert.Equal(c, strings.TrimSpace(out), container.Unhealthy) + assert.Equal(c, container.HealthStatus(strings.TrimSpace(out)), container.Unhealthy) // Make it healthy again cli.DockerCmd(c, "exec", name, "touch", "/status") diff --git a/integration/container/health_test.go b/integration/container/health_test.go index 599d25b492..f5520013f1 100644 --- a/integration/container/health_test.go +++ b/integration/container/health_test.go @@ -140,10 +140,11 @@ func TestHealthStartInterval(t *testing.T) { return poll.Error(err) } if inspect.Container.State.Health.Status != containertypes.Healthy { + var out string if len(inspect.Container.State.Health.Log) > 0 { - t.Log(inspect.Container.State.Health.Log[len(inspect.Container.State.Health.Log)-1]) + out = inspect.Container.State.Health.Log[len(inspect.Container.State.Health.Log)-1].Output } - return poll.Continue("waiting on container to be ready") + return poll.Continue("waiting on container to be ready (%s): %s", inspect.Container.ID, out) } return poll.Success() }, poll.WithTimeout(time.Until(dl))) @@ -169,8 +170,7 @@ func TestHealthStartInterval(t *testing.T) { if h1.Start.Sub(h2.Start) >= inspect.Container.Config.Healthcheck.Interval { return poll.Success() } - t.Log(h1.Start.Sub(h2.Start)) - return poll.Continue("waiting for health check interval to switch from the start interval") + return poll.Continue("waiting for health check interval to switch from the start interval: %s", h1.Start.Sub(h2.Start)) }, poll.WithDelay(time.Second), poll.WithTimeout(time.Until(dl))) } diff --git a/vendor/github.com/moby/moby/api/types/container/health.go b/vendor/github.com/moby/moby/api/types/container/health.go index b7fe592cc2..1a1ba84b40 100644 --- a/vendor/github.com/moby/moby/api/types/container/health.go +++ b/vendor/github.com/moby/moby/api/types/container/health.go @@ -7,9 +7,7 @@ import ( ) // HealthStatus is a string representation of the container's health. -// -// It currently is an alias for string, but may become a distinct type in future. -type HealthStatus = string +type HealthStatus string // Health states const ( @@ -41,7 +39,10 @@ type HealthcheckResult struct { } var validHealths = []string{ - NoHealthcheck, Starting, Healthy, Unhealthy, + string(NoHealthcheck), + string(Starting), + string(Healthy), + string(Unhealthy), } // ValidateHealthStatus checks if the provided string is a valid