From d4d93bf55897bad16111746d43fdd13d6d21832a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 11 Aug 2025 09:08:34 +0200 Subject: [PATCH] daemon/container: remove State.ExitCode() method This method did not provide any special handling for accessing the field, and did not handle locking. Let's remove it for now to not pretend we're doing anything more safe than directly accessing the field. Signed-off-by: Sebastiaan van Stijn --- daemon/container/container.go | 2 +- daemon/container/state.go | 24 +++++++++--------------- daemon/container/state_test.go | 8 ++++---- daemon/container/view.go | 2 +- daemon/inspect.go | 2 +- daemon/start.go | 2 +- 6 files changed, 17 insertions(+), 23 deletions(-) diff --git a/daemon/container/container.go b/daemon/container/container.go index c178fb8f33..a5b3f6a6da 100644 --- a/daemon/container/container.go +++ b/daemon/container/container.go @@ -536,7 +536,7 @@ func (container *Container) GetExecIDs() []string { // This is based on the container's restart policy. func (container *Container) ShouldRestart() bool { shouldRestart, _, _ := container.RestartManager().ShouldRestart( - uint32(container.State.ExitCode()), + uint32(container.State.ExitCode), container.HasBeenManuallyStopped, container.State.FinishedAt.Sub(container.State.StartedAt), ) diff --git a/daemon/container/state.go b/daemon/container/state.go index 3825062c7f..1ddc2f93d5 100644 --- a/daemon/container/state.go +++ b/daemon/container/state.go @@ -39,7 +39,7 @@ type State struct { RemovalInProgress bool `json:"-"` // No need for this to be persistent on disk. Dead bool Pid int - ExitCodeValue int `json:"ExitCode"` + ExitCode int `json:"ExitCode"` ErrorMsg string `json:"Error"` // contains last known error during container start, stop, or remove StartedAt time.Time FinishedAt time.Time @@ -85,7 +85,7 @@ func (s *State) String() string { return fmt.Sprintf("Up %s (Paused)", units.HumanDuration(time.Now().UTC().Sub(s.StartedAt))) } if s.Restarting { - return fmt.Sprintf("Restarting (%d) %s ago", s.ExitCodeValue, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) + return fmt.Sprintf("Restarting (%d) %s ago", s.ExitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) } if h := s.Health; h != nil { @@ -111,7 +111,7 @@ func (s *State) String() string { return "" } - return fmt.Sprintf("Exited (%d) %s ago", s.ExitCodeValue, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) + 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 @@ -162,7 +162,7 @@ func (s *State) Wait(ctx context.Context, condition container.WaitCondition) <-c if s.conditionAlreadyMet(condition) { resultC <- StateStatus{ - exitCode: s.ExitCodeValue, + exitCode: s.ExitCode, err: s.Err(), } @@ -235,16 +235,10 @@ func (s *State) GetPID() int { return s.Pid } -// ExitCode returns current exitcode for the state. Take lock before if state -// may be shared. -func (s *State) ExitCode() int { - return s.ExitCodeValue -} - // SetExitCode sets current exitcode for the state. Take lock before if state // may be shared. func (s *State) SetExitCode(ec int) { - s.ExitCodeValue = ec + s.ExitCode = ec } // SetRunning sets the running state along with StartedAt time. @@ -266,7 +260,7 @@ func (s *State) setRunning(ctr libcontainerdtypes.Container, tsk libcontainerdty if start != nil { s.Paused = false } - s.ExitCodeValue = 0 + s.ExitCode = 0 s.ctr = ctr s.task = tsk if tsk != nil { @@ -291,7 +285,7 @@ func (s *State) SetStopped(exitStatus *ExitStatus) { } else { s.FinishedAt = exitStatus.ExitedAt } - s.ExitCodeValue = exitStatus.ExitCode + s.ExitCode = exitStatus.ExitCode s.notifyAndClear(&s.stopWaiters) } @@ -306,7 +300,7 @@ func (s *State) SetRestarting(exitStatus *ExitStatus) { s.Paused = false s.Pid = 0 s.FinishedAt = time.Now().UTC() - s.ExitCodeValue = exitStatus.ExitCode + s.ExitCode = exitStatus.ExitCode s.notifyAndClear(&s.stopWaiters) } @@ -419,7 +413,7 @@ func (s *State) Err() error { func (s *State) notifyAndClear(waiters *[]chan<- StateStatus) { result := StateStatus{ - exitCode: s.ExitCodeValue, + exitCode: s.ExitCode, err: s.Err(), } diff --git a/daemon/container/state_test.go b/daemon/container/state_test.go index 9c1887f346..382e45f31c 100644 --- a/daemon/container/state_test.go +++ b/daemon/container/state_test.go @@ -58,8 +58,8 @@ func TestStateRunStop(t *testing.T) { if s.Pid != i { t.Fatalf("Pid %v, expected %v", s.Pid, i) } - if s.ExitCode() != 0 { - t.Fatalf("ExitCode %v, expected 0", s.ExitCode()) + if s.ExitCode != 0 { + t.Fatalf("ExitCode %v, expected 0", s.ExitCode) } // Now that it's running, a wait with WaitConditionNotRunning @@ -78,8 +78,8 @@ func TestStateRunStop(t *testing.T) { if s.IsRunning() { t.Fatal("State is running") } - if s.ExitCode() != i { - t.Fatalf("ExitCode %v, expected %v", s.ExitCode(), i) + if s.ExitCode != i { + t.Fatalf("ExitCode %v, expected %v", s.ExitCode, i) } if s.Pid != 0 { t.Fatalf("Pid %v, expected 0", s.Pid) diff --git a/daemon/container/view.go b/daemon/container/view.go index 5af322e338..bcb792a559 100644 --- a/daemon/container/view.go +++ b/daemon/container/view.go @@ -324,7 +324,7 @@ func (v *View) transform(ctr *Container) *Snapshot { Health: health, Running: ctr.State.Running, Paused: ctr.State.Paused, - ExitCode: ctr.State.ExitCode(), + ExitCode: ctr.State.ExitCode, } if snapshot.Names == nil { diff --git a/daemon/inspect.go b/daemon/inspect.go index fa7d6d154c..d897373603 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -131,7 +131,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co OOMKilled: ctr.State.OOMKilled, Dead: ctr.State.Dead, Pid: ctr.State.Pid, - ExitCode: ctr.State.ExitCode(), + ExitCode: ctr.State.ExitCode, Error: ctr.State.ErrorMsg, StartedAt: ctr.State.StartedAt.Format(time.RFC3339Nano), FinishedAt: ctr.State.FinishedAt.Format(time.RFC3339Nano), diff --git a/daemon/start.go b/daemon/start.go index cf220f7ef8..f09bca316a 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -107,7 +107,7 @@ func (daemon *Daemon) containerStart(ctx context.Context, daemonCfg *configStore if retErr != nil { container.State.SetError(retErr) // if no one else has set it, make sure we don't leave it at zero - if container.State.ExitCode() == 0 { + if container.State.ExitCode == 0 { container.State.SetExitCode(exitUnknown) } if err := container.CheckpointTo(context.WithoutCancel(ctx), daemon.containersReplica); err != nil {