From d4f61f92fd98e5c738bb55780742c0fd7f39dcf8 Mon Sep 17 00:00:00 2001 From: Lars Andringa Date: Sat, 30 Dec 2023 15:32:53 +0100 Subject: [PATCH] Move StartedAt time to before starting the container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lars Andringa Signed-off-by: LarsSven Replaced boolean parameter by IsZero check Signed-off-by: LarsSven Separated SetRunning into two functions Signed-off-by: LarsSven Apply suggestions from code review Documentation fixes Co-authored-by: Paweł Gronowski Signed-off-by: LarsSven --- container/state.go | 20 +++++++++++++++----- container/state_test.go | 8 ++++---- daemon/delete_test.go | 3 ++- daemon/monitor.go | 2 +- daemon/start.go | 3 ++- integration/container/daemon_linux_test.go | 2 +- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/container/state.go b/container/state.go index cdf88fe371..b3bba4ba2b 100644 --- a/container/state.go +++ b/container/state.go @@ -268,13 +268,23 @@ func (s *State) SetExitCode(ec int) { s.ExitCodeValue = ec } -// SetRunning sets the state of the container to "running". -func (s *State) SetRunning(ctr libcontainerdtypes.Container, tsk libcontainerdtypes.Task, initial bool) { +// SetRunning sets the running state along with StartedAt time. +func (s *State) SetRunning(ctr libcontainerdtypes.Container, tsk libcontainerdtypes.Task, start time.Time) { + s.setRunning(ctr, tsk, &start) +} + +// SetRunningExternal sets the running state without setting the `StartedAt` time (used for containers not started by Docker instead of SetRunning). +func (s *State) SetRunningExternal(ctr libcontainerdtypes.Container, tsk libcontainerdtypes.Task) { + s.setRunning(ctr, tsk, nil) +} + +// setRunning sets the state of the container to "running". +func (s *State) setRunning(ctr libcontainerdtypes.Container, tsk libcontainerdtypes.Task, start *time.Time) { s.ErrorMsg = "" s.Paused = false s.Running = true s.Restarting = false - if initial { + if start != nil { s.Paused = false } s.ExitCodeValue = 0 @@ -286,8 +296,8 @@ func (s *State) SetRunning(ctr libcontainerdtypes.Container, tsk libcontainerdty s.Pid = 0 } s.OOMKilled = false - if initial { - s.StartedAt = time.Now().UTC() + if start != nil { + s.StartedAt = start.UTC() } } diff --git a/container/state_test.go b/container/state_test.go index f4f22f70d6..2043a66053 100644 --- a/container/state_test.go +++ b/container/state_test.go @@ -68,7 +68,7 @@ func TestStateRunStop(t *testing.T) { // Set the state to "Running". s.Lock() - s.SetRunning(nil, &mockTask{pid: uint32(i)}, true) + s.SetRunning(nil, &mockTask{pid: uint32(i)}, time.Now()) s.Unlock() // Assert desired state. @@ -133,7 +133,7 @@ func TestStateTimeoutWait(t *testing.T) { s := NewState() s.Lock() - s.SetRunning(nil, nil, true) + s.SetRunning(nil, nil, time.Now()) s.Unlock() // Start a wait with a timeout. @@ -182,7 +182,7 @@ func TestCorrectStateWaitResultAfterRestart(t *testing.T) { s := NewState() s.Lock() - s.SetRunning(nil, nil, true) + s.SetRunning(nil, nil, time.Now()) s.Unlock() waitC := s.Wait(context.Background(), WaitConditionNotRunning) @@ -193,7 +193,7 @@ func TestCorrectStateWaitResultAfterRestart(t *testing.T) { s.Unlock() s.Lock() - s.SetRunning(nil, nil, true) + s.SetRunning(nil, nil, time.Now()) s.Unlock() got := <-waitC diff --git a/daemon/delete_test.go b/daemon/delete_test.go index 9733498cac..fe727edcdf 100644 --- a/daemon/delete_test.go +++ b/daemon/delete_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "testing" + "time" "github.com/docker/docker/api/types/backend" containertypes "github.com/docker/docker/api/types/container" @@ -52,7 +53,7 @@ func TestContainerDelete(t *testing.T) { errMsg: "container is restarting: stop the container before removing or force remove", initContainer: func() *container.Container { c := newContainerWithState(container.NewState()) - c.SetRunning(nil, nil, true) + c.SetRunning(nil, nil, time.Now()) c.SetRestarting(&container.ExitStatus{}) return c }, diff --git a/daemon/monitor.go b/daemon/monitor.go index 4734fe45e5..3cad773e14 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -254,7 +254,7 @@ func (daemon *Daemon) ProcessEvent(id string, e libcontainerdtypes.EventType, ei } return err } - c.SetRunning(ctr, tsk, false) + c.SetRunningExternal(ctr, tsk) c.HasBeenManuallyStopped = false c.HasBeenStartedBefore = true daemon.setStateCounter(c) diff --git a/daemon/start.go b/daemon/start.go index 948f458c0e..b55062c619 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -199,6 +199,7 @@ func (daemon *Daemon) containerStart(ctx context.Context, daemonCfg *configStore return setExitCodeFromError(container.SetExitCode, err) } + startupTime := time.Now() // TODO(mlaventure): we need to specify checkpoint options here tsk, err := ctr.Start(context.TODO(), // Passing ctx to ctr.Start caused integration tests to be stuck in the cleanup phase checkpointDir, container.StreamConfig.Stdin() != nil || container.Config.Tty, @@ -212,7 +213,7 @@ func (daemon *Daemon) containerStart(ctx context.Context, daemonCfg *configStore } container.HasBeenManuallyRestarted = false - container.SetRunning(ctr, tsk, true) + container.SetRunning(ctr, tsk, startupTime) container.HasBeenStartedBefore = true daemon.setStateCounter(container) diff --git a/integration/container/daemon_linux_test.go b/integration/container/daemon_linux_test.go index 41bddd96f9..3b60a39d68 100644 --- a/integration/container/daemon_linux_test.go +++ b/integration/container/daemon_linux_test.go @@ -256,7 +256,7 @@ func TestHardRestartWhenContainerIsRunning(t *testing.T) { for _, id := range []string{noPolicy, onFailure} { d.TamperWithContainerConfig(t, id, func(c *realcontainer.Container) { - c.SetRunning(nil, nil, true) + c.SetRunning(nil, nil, time.Now()) c.HasBeenStartedBefore = true }) }