Skip to content

Commit

Permalink
Move StartedAt time to before starting the container
Browse files Browse the repository at this point in the history
Signed-off-by: Lars Andringa <l.s.andringa@rug.nl>
Signed-off-by: LarsSven <l.s.andringa@rug.nl>

Replaced boolean parameter by IsZero check

Signed-off-by: LarsSven <l.s.andringa@rug.nl>

Separated SetRunning into two functions

Signed-off-by: LarsSven <l.s.andringa@rug.nl>

Apply suggestions from code review

Documentation fixes

Co-authored-by: Paweł Gronowski <me@woland.xyz>
Signed-off-by: LarsSven <l.s.andringa@rug.nl>
  • Loading branch information
Lars Andringa and vvoland committed Mar 12, 2024
1 parent 341e7b3 commit d4f61f9
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 13 deletions.
20 changes: 15 additions & 5 deletions container/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
}
}

Expand Down
8 changes: 4 additions & 4 deletions container/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion daemon/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"testing"
"time"

"github.com/docker/docker/api/types/backend"
containertypes "github.com/docker/docker/api/types/container"
Expand Down Expand Up @@ -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
},
Expand Down
2 changes: 1 addition & 1 deletion daemon/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion daemon/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion integration/container/daemon_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
Expand Down

0 comments on commit d4f61f9

Please sign in to comment.