From ba213d09565f15bab55146e9fb18bfa7879f601b Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Tue, 30 Aug 2016 09:44:06 -0700 Subject: [PATCH 1/2] Re-export container state's ExitCode and Error fields Those are needed in order to reload their value upon docker daemon restart. Signed-off-by: Kenfe-Mickael Laventure Upstream-commit: 2998945a54577e24a6414d576bc861e58fa87359 Component: engine --- components/engine/container/state.go | 61 ++++++++++++++------ components/engine/container/state_solaris.go | 2 +- components/engine/container/state_unix.go | 2 +- components/engine/container/state_windows.go | 2 +- 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/components/engine/container/state.go b/components/engine/container/state.go index 4146947bcb..12b22a4026 100644 --- a/components/engine/container/state.go +++ b/components/engine/container/state.go @@ -24,14 +24,40 @@ type State struct { RemovalInProgress bool // Not need for this to be persistent on disk. Dead bool Pid int - exitCode int - error string // contains last known error when starting the container + ExitCodeValue int `json:"ExitCode"` + ErrorMsg string `json:"Error"` // contains last known error when starting the container StartedAt time.Time FinishedAt time.Time waitChan chan struct{} Health *Health } +// StateStatus is used to return an error type implementing both +// exec.ExitCode and error. +// This type is needed as State include a sync.Mutex field which make +// copying it unsafe. +type StateStatus struct { + exitCode int + error string +} + +func newStateStatus(ec int, err string) *StateStatus { + return &StateStatus{ + exitCode: ec, + error: err, + } +} + +// ExitCode returns current exitcode for the state. +func (ss *StateStatus) ExitCode() int { + return ss.exitCode +} + +// Error returns current error for the state. +func (ss *StateStatus) Error() string { + return ss.error +} + // NewState creates a default state object with a fresh channel for state changes. func NewState() *State { return &State{ @@ -46,7 +72,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.exitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) + return fmt.Sprintf("Restarting (%d) %s ago", s.ExitCodeValue, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) } if h := s.Health; h != nil { @@ -71,7 +97,7 @@ func (s *State) String() string { return "" } - return fmt.Sprintf("Exited (%d) %s ago", s.exitCode, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) + return fmt.Sprintf("Exited (%d) %s ago", s.ExitCodeValue, units.HumanDuration(time.Now().UTC().Sub(s.FinishedAt))) } // StateString returns a single string to describe state @@ -134,7 +160,7 @@ func wait(waitChan <-chan struct{}, timeout time.Duration) error { func (s *State) WaitStop(timeout time.Duration) (int, error) { s.Lock() if !s.Running { - exitCode := s.exitCode + exitCode := s.ExitCodeValue s.Unlock() return exitCode, nil } @@ -154,24 +180,24 @@ func (s *State) WaitWithContext(ctx context.Context) error { // todo(tonistiigi): make other wait functions use this s.Lock() if !s.Running { - state := *s + state := newStateStatus(s.ExitCode(), s.Error()) defer s.Unlock() - if state.exitCode == 0 { + if state.ExitCode() == 0 { return nil } - return &state + return state } waitChan := s.waitChan s.Unlock() select { case <-waitChan: s.Lock() - state := *s + state := newStateStatus(s.ExitCode(), s.Error()) s.Unlock() - if state.exitCode == 0 { + if state.ExitCode() == 0 { return nil } - return &state + return state case <-ctx.Done(): return ctx.Err() } @@ -196,22 +222,21 @@ func (s *State) GetPID() int { // ExitCode returns current exitcode for the state. Take lock before if state // may be shared. func (s *State) ExitCode() int { - res := s.exitCode - return res + 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.exitCode = ec + s.ExitCodeValue = ec } // SetRunning sets the state of the container to "running". func (s *State) SetRunning(pid int, initial bool) { - s.error = "" + s.ErrorMsg = "" s.Running = true s.Restarting = false - s.exitCode = 0 + s.ExitCodeValue = 0 s.Pid = pid if initial { s.StartedAt = time.Now().UTC() @@ -263,7 +288,7 @@ func (s *State) SetRestarting(exitStatus *ExitStatus) { // know the error that occurred when container transits to another state // when inspecting it func (s *State) SetError(err error) { - s.error = err.Error() + s.ErrorMsg = err.Error() } // IsPaused returns whether the container is paused or not. @@ -310,5 +335,5 @@ func (s *State) SetDead() { // Error returns current error for the state. func (s *State) Error() string { - return s.error + return s.ErrorMsg } diff --git a/components/engine/container/state_solaris.go b/components/engine/container/state_solaris.go index 9aef1d518e..1229650efa 100644 --- a/components/engine/container/state_solaris.go +++ b/components/engine/container/state_solaris.go @@ -3,5 +3,5 @@ package container // setFromExitStatus is a platform specific helper function to set the state // based on the ExitStatus structure. func (s *State) setFromExitStatus(exitStatus *ExitStatus) { - s.exitCode = exitStatus.ExitCode + s.ExitCodeValue = exitStatus.ExitCode } diff --git a/components/engine/container/state_unix.go b/components/engine/container/state_unix.go index f09d015e0b..a2fa5afc28 100644 --- a/components/engine/container/state_unix.go +++ b/components/engine/container/state_unix.go @@ -5,6 +5,6 @@ package container // setFromExitStatus is a platform specific helper function to set the state // based on the ExitStatus structure. func (s *State) setFromExitStatus(exitStatus *ExitStatus) { - s.exitCode = exitStatus.ExitCode + s.ExitCodeValue = exitStatus.ExitCode s.OOMKilled = exitStatus.OOMKilled } diff --git a/components/engine/container/state_windows.go b/components/engine/container/state_windows.go index 9aef1d518e..1229650efa 100644 --- a/components/engine/container/state_windows.go +++ b/components/engine/container/state_windows.go @@ -3,5 +3,5 @@ package container // setFromExitStatus is a platform specific helper function to set the state // based on the ExitStatus structure. func (s *State) setFromExitStatus(exitStatus *ExitStatus) { - s.exitCode = exitStatus.ExitCode + s.ExitCodeValue = exitStatus.ExitCode } From d7713e33ac31561d3eb53e7321b044ab7c3eacfc Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Tue, 30 Aug 2016 12:00:27 -0700 Subject: [PATCH 2/2] Add integration test to check persistence of exitcode and error message Signed-off-by: Kenfe-Mickael Laventure Upstream-commit: 88bfa6ede8fd1536d8248cc56882bc4b826d090d Component: engine --- .../integration-cli/docker_cli_daemon_test.go | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/components/engine/integration-cli/docker_cli_daemon_test.go b/components/engine/integration-cli/docker_cli_daemon_test.go index 0febe800c1..45c220a884 100644 --- a/components/engine/integration-cli/docker_cli_daemon_test.go +++ b/components/engine/integration-cli/docker_cli_daemon_test.go @@ -2764,3 +2764,41 @@ func (s *DockerDaemonSuite) TestDaemonRestartWithAutoRemoveContainer(c *check.C) c.Assert(out, checker.Contains, "top1", check.Commentf("top1 should exist after daemon restarts")) c.Assert(out, checker.Not(checker.Contains), "top2", check.Commentf("top2 should be removed after daemon restarts")) } + +func (s *DockerDaemonSuite) TestDaemonRestartSaveContainerExitCode(c *check.C) { + err := s.d.StartWithBusybox() + c.Assert(err, checker.IsNil) + + containerName := "error-values" + runError := "oci runtime error: exec: \"toto\": executable file not found in $PATH" + // Make a container with both a non 0 exit code and an error message + out, err := s.d.Cmd("run", "--name", containerName, "busybox", "toto") + c.Assert(err, checker.NotNil) + c.Assert(out, checker.Contains, runError) + + // Check that those values were saved on disk + out, err = s.d.Cmd("inspect", "-f", "{{.State.ExitCode}}", containerName) + out = strings.TrimSpace(out) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Equals, "127") + + out, err = s.d.Cmd("inspect", "-f", "{{.State.Error}}", containerName) + out = strings.TrimSpace(out) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Equals, runError) + + // now restart daemon + err = s.d.Restart() + c.Assert(err, checker.IsNil) + + // Check that those values are still around + out, err = s.d.Cmd("inspect", "-f", "{{.State.ExitCode}}", containerName) + out = strings.TrimSpace(out) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Equals, "127") + + out, err = s.d.Cmd("inspect", "-f", "{{.State.Error}}", containerName) + out = strings.TrimSpace(out) + c.Assert(err, checker.IsNil) + c.Assert(out, checker.Equals, runError) +}