Browse code

Merge pull request #51439 from thaJeztah/concrete_enums

api/types/container: make ContainerState, HealthStatus concrete types

Paweł Gronowski authored on 2025/11/11 00:21:22
Showing 20 changed files
... ...
@@ -7,9 +7,7 @@ import (
7 7
 )
8 8
 
9 9
 // HealthStatus is a string representation of the container's health.
10
-//
11
-// It currently is an alias for string, but may become a distinct type in future.
12
-type HealthStatus = string
10
+type HealthStatus string
13 11
 
14 12
 // Health states
15 13
 const (
... ...
@@ -41,7 +39,10 @@ type HealthcheckResult struct {
41 41
 }
42 42
 
43 43
 var validHealths = []string{
44
-	NoHealthcheck, Starting, Healthy, Unhealthy,
44
+	string(NoHealthcheck),
45
+	string(Starting),
46
+	string(Healthy),
47
+	string(Unhealthy),
45 48
 }
46 49
 
47 50
 // ValidateHealthStatus checks if the provided string is a valid
... ...
@@ -19,7 +19,7 @@ func TestValidateHealthStatus(t *testing.T) {
19 19
 	}
20 20
 
21 21
 	for _, tc := range tests {
22
-		t.Run(tc.health, func(t *testing.T) {
22
+		t.Run(string(tc.health), func(t *testing.T) {
23 23
 			err := ValidateHealthStatus(tc.health)
24 24
 			if tc.expectedErr == "" {
25 25
 				assert.NilError(t, err)
... ...
@@ -6,9 +6,7 @@ import (
6 6
 )
7 7
 
8 8
 // ContainerState is a string representation of the container's current state.
9
-//
10
-// It currently is an alias for string, but may become a distinct type in the future.
11
-type ContainerState = string
9
+type ContainerState string
12 10
 
13 11
 const (
14 12
 	StateCreated    ContainerState = "created"    // StateCreated indicates the container is created, but not (yet) started.
... ...
@@ -20,8 +18,14 @@ const (
20 20
 	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.
21 21
 )
22 22
 
23
-var validStates = []ContainerState{
24
-	StateCreated, StateRunning, StatePaused, StateRestarting, StateRemoving, StateExited, StateDead,
23
+var validStates = []string{
24
+	string(StateCreated),
25
+	string(StateRunning),
26
+	string(StatePaused),
27
+	string(StateRestarting),
28
+	string(StateRemoving),
29
+	string(StateExited),
30
+	string(StateDead),
25 31
 }
26 32
 
27 33
 // ValidateContainerState checks if the provided string is a valid
... ...
@@ -21,7 +21,7 @@ func TestValidateContainerState(t *testing.T) {
21 21
 		{state: "invalid-state-string", expectedErr: `invalid value for state (invalid-state-string): must be one of created, running, paused, restarting, removing, exited, dead`},
22 22
 	}
23 23
 	for _, tc := range tests {
24
-		t.Run(tc.state, func(t *testing.T) {
24
+		t.Run(string(tc.state), func(t *testing.T) {
25 25
 			err := ValidateContainerState(tc.state)
26 26
 			if tc.expectedErr == "" {
27 27
 				assert.NilError(t, err)
... ...
@@ -23,7 +23,7 @@ func (s *Health) String() string {
23 23
 	case container.Starting:
24 24
 		return "health: starting"
25 25
 	default: // Healthy and Unhealthy are clear on their own
26
-		return status
26
+		return string(status)
27 27
 	}
28 28
 }
29 29
 
... ...
@@ -31,7 +31,7 @@ type State struct {
31 31
 	//
32 32
 	// In a similar fashion, [State.Running] and [State.Restarting] can both
33 33
 	// be true in a situation where a container is in process of being restarted.
34
-	// Refer to [State.StateString] for order of precedence.
34
+	// Refer to [State.State] for order of precedence.
35 35
 	Running           bool
36 36
 	Paused            bool
37 37
 	Restarting        bool
... ...
@@ -114,10 +114,10 @@ func (s *State) String() string {
114 114
 	return fmt.Sprintf("Exited (%d) %s ago", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt)))
115 115
 }
116 116
 
117
-// StateString returns the container's current [ContainerState], based on the
117
+// State returns the container's current [container.ContainerState], based on the
118 118
 // [State.Running], [State.Paused], [State.Restarting], [State.RemovalInProgress],
119 119
 // [State.StartedAt] and [State.Dead] fields.
120
-func (s *State) StateString() container.ContainerState {
120
+func (s *State) State() container.ContainerState {
121 121
 	if s.Running {
122 122
 		if s.Paused {
123 123
 			return container.StatePaused
... ...
@@ -221,7 +221,7 @@ func (s *State) conditionAlreadyMet(condition container.WaitCondition) bool {
221 221
 //
222 222
 // In a similar fashion, [State.Running] and [State.Restarting] can both
223 223
 // be true in a situation where a container is in process of being restarted.
224
-// Refer to [State.StateString] for order of precedence.
224
+// Refer to [State.State] for order of precedence.
225 225
 func (s *State) IsRunning() bool {
226 226
 	s.Lock()
227 227
 	defer s.Unlock()
... ...
@@ -327,7 +327,7 @@ func (s *State) SetError(err error) {
327 327
 //
328 328
 // In a similar fashion, [State.Running] and [State.Restarting] can both
329 329
 // be true in a situation where a container is in process of being restarted.
330
-// Refer to [State.StateString] for order of precedence.
330
+// Refer to [State.State] for order of precedence.
331 331
 func (s *State) IsPaused() bool {
332 332
 	s.Lock()
333 333
 	defer s.Unlock()
... ...
@@ -346,7 +346,7 @@ func (s *State) IsPaused() bool {
346 346
 //
347 347
 // In a similar fashion, [State.Running] and [State.Restarting] can both
348 348
 // be true in a situation where a container is in process of being restarted.
349
-// Refer to [State.StateString] for order of precedence.
349
+// Refer to [State.State] for order of precedence.
350 350
 func (s *State) IsRestarting() bool {
351 351
 	s.Lock()
352 352
 	defer s.Unlock()
... ...
@@ -305,7 +305,7 @@ func (v *View) transform(ctr *Container) *Snapshot {
305 305
 			ImageID: ctr.ImageID.String(),
306 306
 			Ports:   []container.PortSummary{},
307 307
 			Mounts:  ctr.GetMountPoints(),
308
-			State:   ctr.State.StateString(),
308
+			State:   ctr.State.State(),
309 309
 			Status:  ctr.State.String(),
310 310
 			Health: &container.HealthSummary{
311 311
 				Status:        health,
... ...
@@ -968,7 +968,7 @@ func (daemon *Daemon) getNetworkedContainer(containerID, connectedContainerPrefi
968 968
 		return nil, errdefs.System(errdefs.InvalidParameter(errors.New("cannot join own network namespace")))
969 969
 	}
970 970
 	if !nc.State.IsRunning() {
971
-		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()))
971
+		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()))
972 972
 	}
973 973
 	if nc.State.IsRestarting() {
974 974
 		return nil, fmt.Errorf("cannot join network namespace of container: %w", errContainerIsRestarting(connectedContainerPrefixOrName))
... ...
@@ -92,7 +92,7 @@ func (daemon *Daemon) cleanupContainer(ctr *container.Container, config backend.
92 92
 			if ctr.State.Paused {
93 93
 				return errdefs.Conflict(errors.New("container is paused and must be unpaused first"))
94 94
 			} else {
95
-				return errdefs.Conflict(fmt.Errorf("container is %s: stop the container before removing or force remove", ctr.State.StateString()))
95
+				return errdefs.Conflict(fmt.Errorf("container is %s: stop the container before removing or force remove", ctr.State.State()))
96 96
 			}
97 97
 		}
98 98
 		if err := daemon.Kill(ctr); err != nil && !isNotRunning(err) {
... ...
@@ -244,7 +244,7 @@ func handleProbeResult(d *Daemon, c *container.Container, result *containertypes
244 244
 
245 245
 	current := h.Status()
246 246
 	if oldStatus != current {
247
-		d.LogContainerEvent(c, events.Action(string(events.ActionHealthStatus)+": "+current))
247
+		d.LogContainerEvent(c, events.Action(string(events.ActionHealthStatus)+": "+string(current)))
248 248
 	}
249 249
 }
250 250
 
... ...
@@ -125,7 +125,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
125 125
 		Path:    ctr.Path,
126 126
 		Args:    ctr.Args,
127 127
 		State: &containertypes.State{
128
-			Status:     ctr.State.StateString(),
128
+			Status:     ctr.State.State(),
129 129
 			Running:    ctr.State.Running,
130 130
 			Paused:     ctr.State.Paused,
131 131
 			Restarting: ctr.State.Restarting,
... ...
@@ -91,6 +91,7 @@ func (ctr *StateCounter) Get() (running int, paused int, stopped int) {
91 91
 	ctr.mu.RLock()
92 92
 	defer ctr.mu.RUnlock()
93 93
 
94
+	// FIXME(thaJeztah): there's no "container.StateStopped"; should we align these states with actual states?
94 95
 	for _, state := range ctr.states {
95 96
 		switch state {
96 97
 		case "running":
... ...
@@ -281,7 +281,7 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf
281 281
 	}
282 282
 
283 283
 	err = psFilters.WalkValues("status", func(value string) error {
284
-		if err := containertypes.ValidateContainerState(value); err != nil {
284
+		if err := containertypes.ValidateContainerState(containertypes.ContainerState(value)); err != nil {
285 285
 			return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'status=%s': %w", value, err))
286 286
 		}
287 287
 		config.All = true
... ...
@@ -298,7 +298,7 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf
298 298
 	}
299 299
 
300 300
 	err = psFilters.WalkValues("health", func(value string) error {
301
-		if err := containertypes.ValidateHealthStatus(value); err != nil {
301
+		if err := containertypes.ValidateHealthStatus(containertypes.HealthStatus(value)); err != nil {
302 302
 			return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'health=%s': %w", value, err))
303 303
 		}
304 304
 		return nil
... ...
@@ -486,12 +486,12 @@ func includeContainerInList(container *container.Snapshot, filter *listContext)
486 486
 	}
487 487
 
488 488
 	// Do not include container if its status doesn't match the filter
489
-	if !filter.filters.Match("status", container.State) {
489
+	if !filter.filters.Match("status", string(container.State)) {
490 490
 		return excludeContainer
491 491
 	}
492 492
 
493 493
 	// Do not include container if its health doesn't match the filter
494
-	if !filter.filters.ExactMatch("health", container.Health) {
494
+	if !filter.filters.ExactMatch("health", string(container.Health)) {
495 495
 		return excludeContainer
496 496
 	}
497 497
 
... ...
@@ -8,6 +8,7 @@ import (
8 8
 
9 9
 	cerrdefs "github.com/containerd/errdefs"
10 10
 	"github.com/containerd/log"
11
+	containertypes "github.com/moby/moby/api/types/container"
11 12
 	"github.com/moby/moby/api/types/events"
12 13
 	"github.com/moby/moby/v2/daemon/config"
13 14
 	"github.com/moby/moby/v2/daemon/container"
... ...
@@ -19,10 +20,10 @@ import (
19 19
 )
20 20
 
21 21
 func (daemon *Daemon) setStateCounter(c *container.Container) {
22
-	switch c.State.StateString() {
23
-	case "paused":
22
+	switch c.State.State() {
23
+	case containertypes.StatePaused:
24 24
 		metrics.StateCtr.Set(c.ID, "paused")
25
-	case "running":
25
+	case containertypes.StateRunning:
26 26
 		metrics.StateCtr.Set(c.ID, "running")
27 27
 	default:
28 28
 		metrics.StateCtr.Set(c.ID, "stopped")
... ...
@@ -26,16 +26,17 @@ func (s *DockerCLIHealthSuite) OnTimeout(t *testing.T) {
26 26
 	s.ds.OnTimeout(t)
27 27
 }
28 28
 
29
-func waitForHealthStatus(t *testing.T, name string, prev string, expected string) {
30
-	prev = prev + "\n"
31
-	expected = expected + "\n"
29
+func waitForHealthStatus(t *testing.T, name string, prev container.HealthStatus, expected container.HealthStatus) {
32 30
 	for {
33 31
 		out := cli.DockerCmd(t, "inspect", "--format={{.State.Health.Status}}", name).Stdout()
34
-		if out == expected {
32
+		actual := container.HealthStatus(strings.TrimSpace(out))
33
+		if actual == expected {
35 34
 			return
36 35
 		}
37
-		assert.Equal(t, out, prev)
38
-		if out != prev {
36
+
37
+		// TODO(thaJeztah): this logic seems broken? assert.Assert would make it fail, so why the "actual != prev"?
38
+		assert.Equal(t, actual, prev)
39
+		if actual != prev {
39 40
 			return
40 41
 		}
41 42
 		time.Sleep(100 * time.Millisecond)
... ...
@@ -84,7 +85,7 @@ func (s *DockerCLIHealthSuite) TestHealth(c *testing.T) {
84 84
 
85 85
 	// Inspect the status
86 86
 	out = cli.DockerCmd(c, "inspect", "--format={{.State.Health.Status}}", name).Stdout()
87
-	assert.Equal(c, strings.TrimSpace(out), container.Unhealthy)
87
+	assert.Equal(c, container.HealthStatus(strings.TrimSpace(out)), container.Unhealthy)
88 88
 
89 89
 	// Make it healthy again
90 90
 	cli.DockerCmd(c, "exec", name, "touch", "/status")
... ...
@@ -140,10 +140,11 @@ func TestHealthStartInterval(t *testing.T) {
140 140
 			return poll.Error(err)
141 141
 		}
142 142
 		if inspect.Container.State.Health.Status != containertypes.Healthy {
143
+			var out string
143 144
 			if len(inspect.Container.State.Health.Log) > 0 {
144
-				t.Log(inspect.Container.State.Health.Log[len(inspect.Container.State.Health.Log)-1])
145
+				out = inspect.Container.State.Health.Log[len(inspect.Container.State.Health.Log)-1].Output
145 146
 			}
146
-			return poll.Continue("waiting on container to be ready")
147
+			return poll.Continue("waiting on container to be ready (%s): %s", inspect.Container.ID, out)
147 148
 		}
148 149
 		return poll.Success()
149 150
 	}, poll.WithTimeout(time.Until(dl)))
... ...
@@ -169,8 +170,7 @@ func TestHealthStartInterval(t *testing.T) {
169 169
 		if h1.Start.Sub(h2.Start) >= inspect.Container.Config.Healthcheck.Interval {
170 170
 			return poll.Success()
171 171
 		}
172
-		t.Log(h1.Start.Sub(h2.Start))
173
-		return poll.Continue("waiting for health check interval to switch from the start interval")
172
+		return poll.Continue("waiting for health check interval to switch from the start interval: %s", h1.Start.Sub(h2.Start))
174 173
 	}, poll.WithDelay(time.Second), poll.WithTimeout(time.Until(dl)))
175 174
 }
176 175
 
... ...
@@ -92,17 +92,17 @@ func TestKillWithStopSignalAndRestartPolicies(t *testing.T) {
92 92
 	testCases := []struct {
93 93
 		doc        string
94 94
 		stopsignal string
95
-		status     string
95
+		status     containertypes.ContainerState
96 96
 	}{
97 97
 		{
98 98
 			doc:        "same-signal-disables-restart-policy",
99 99
 			stopsignal: "TERM",
100
-			status:     "exited",
100
+			status:     containertypes.StateExited,
101 101
 		},
102 102
 		{
103 103
 			doc:        "different-signal-keep-restart-policy",
104 104
 			stopsignal: "CONT",
105
-			status:     "running",
105
+			status:     containertypes.StateRunning,
106 106
 		},
107 107
 	}
108 108
 
... ...
@@ -39,15 +39,17 @@ func IsInState(ctx context.Context, apiClient client.APIClient, containerID stri
39 39
 		if err != nil {
40 40
 			return poll.Error(err)
41 41
 		}
42
+		var states []string
42 43
 		for _, v := range state {
44
+			states = append(states, string(v))
43 45
 			if inspect.Container.State.Status == v {
44 46
 				return poll.Success()
45 47
 			}
46 48
 		}
47
-		if len(state) == 1 {
49
+		if len(states) == 1 {
48 50
 			return poll.Continue("waiting for container State.Status to be '%s', currently '%s'", state[0], inspect.Container.State.Status)
49 51
 		} else {
50
-			return poll.Continue("waiting for container State.Status to be one of (%s), currently '%s'", strings.Join(state, ", "), inspect.Container.State.Status)
52
+			return poll.Continue("waiting for container State.Status to be one of (%s), currently '%s'", strings.Join(states, ", "), inspect.Container.State.Status)
51 53
 		}
52 54
 	}
53 55
 }
... ...
@@ -7,9 +7,7 @@ import (
7 7
 )
8 8
 
9 9
 // HealthStatus is a string representation of the container's health.
10
-//
11
-// It currently is an alias for string, but may become a distinct type in future.
12
-type HealthStatus = string
10
+type HealthStatus string
13 11
 
14 12
 // Health states
15 13
 const (
... ...
@@ -41,7 +39,10 @@ type HealthcheckResult struct {
41 41
 }
42 42
 
43 43
 var validHealths = []string{
44
-	NoHealthcheck, Starting, Healthy, Unhealthy,
44
+	string(NoHealthcheck),
45
+	string(Starting),
46
+	string(Healthy),
47
+	string(Unhealthy),
45 48
 }
46 49
 
47 50
 // ValidateHealthStatus checks if the provided string is a valid
... ...
@@ -6,9 +6,7 @@ import (
6 6
 )
7 7
 
8 8
 // ContainerState is a string representation of the container's current state.
9
-//
10
-// It currently is an alias for string, but may become a distinct type in the future.
11
-type ContainerState = string
9
+type ContainerState string
12 10
 
13 11
 const (
14 12
 	StateCreated    ContainerState = "created"    // StateCreated indicates the container is created, but not (yet) started.
... ...
@@ -20,8 +18,14 @@ const (
20 20
 	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.
21 21
 )
22 22
 
23
-var validStates = []ContainerState{
24
-	StateCreated, StateRunning, StatePaused, StateRestarting, StateRemoving, StateExited, StateDead,
23
+var validStates = []string{
24
+	string(StateCreated),
25
+	string(StateRunning),
26
+	string(StatePaused),
27
+	string(StateRestarting),
28
+	string(StateRemoving),
29
+	string(StateExited),
30
+	string(StateDead),
25 31
 }
26 32
 
27 33
 // ValidateContainerState checks if the provided string is a valid