From b6db56b5eba00c4e8ad7a6f6c5b018e15dc883eb Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Mon, 18 Apr 2016 13:10:15 -0700 Subject: [PATCH] Reset restart timeout if execution longer than 10s Restore the 1.10 logic that will reset the restart manager's timeout or backoff delay if a container executes longer than 10s reguardless of exit status or policy. Signed-off-by: Michael Crosby --- container/container.go | 2 +- libcontainerd/container.go | 2 ++ libcontainerd/container_linux.go | 4 +++- libcontainerd/container_windows.go | 4 +++- restartmanager/restartmanager.go | 10 +++++--- restartmanager/restartmanager_test.go | 33 ++++++++++++++++++++++++++- 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/container/container.go b/container/container.go index e658c4b4dd..f749259044 100644 --- a/container/container.go +++ b/container/container.go @@ -519,7 +519,7 @@ func copyEscapable(dst io.Writer, src io.ReadCloser, keys []byte) (written int64 // ShouldRestart decides whether the daemon should restart the container or not. // This is based on the container's restart policy. func (container *Container) ShouldRestart() bool { - shouldRestart, _, _ := container.restartManager.ShouldRestart(uint32(container.ExitCode), container.HasBeenManuallyStopped) + shouldRestart, _, _ := container.restartManager.ShouldRestart(uint32(container.ExitCode), container.HasBeenManuallyStopped, container.FinishedAt.Sub(container.StartedAt)) return shouldRestart } diff --git a/libcontainerd/container.go b/libcontainerd/container.go index 197990f2b2..30bc95028c 100644 --- a/libcontainerd/container.go +++ b/libcontainerd/container.go @@ -2,6 +2,7 @@ package libcontainerd import ( "fmt" + "time" "github.com/docker/docker/restartmanager" ) @@ -18,6 +19,7 @@ type containerCommon struct { restartManager restartmanager.RestartManager restarting bool processes map[string]*process + startedAt time.Time } // WithRestartManager sets the restartmanager to be used with the container. diff --git a/libcontainerd/container_linux.go b/libcontainerd/container_linux.go index 1857874b32..f0821fb470 100644 --- a/libcontainerd/container_linux.go +++ b/libcontainerd/container_linux.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "syscall" + "time" "github.com/Sirupsen/logrus" containerd "github.com/docker/containerd/api/grpc/types" @@ -74,6 +75,7 @@ func (ctr *container) start() error { ctr.closeFifos(iopipe) return err } + ctr.startedAt = time.Now() if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil { return err @@ -118,7 +120,7 @@ func (ctr *container) handleEvent(e *containerd.Event) error { st.State = StateExitProcess } if st.State == StateExit && ctr.restartManager != nil { - restart, wait, err := ctr.restartManager.ShouldRestart(e.Status, false) + restart, wait, err := ctr.restartManager.ShouldRestart(e.Status, false, time.Since(ctr.startedAt)) if err != nil { logrus.Warnf("container %s %v", ctr.containerID, err) } else if restart { diff --git a/libcontainerd/container_windows.go b/libcontainerd/container_windows.go index cee7464faa..362673f45d 100644 --- a/libcontainerd/container_windows.go +++ b/libcontainerd/container_windows.go @@ -4,6 +4,7 @@ import ( "io" "strings" "syscall" + "time" "github.com/Microsoft/hcsshim" "github.com/Sirupsen/logrus" @@ -78,6 +79,7 @@ func (ctr *container) start() error { } return err } + ctr.startedAt = time.Now() // Convert io.ReadClosers to io.Readers if stdout != nil { @@ -168,7 +170,7 @@ func (ctr *container) waitExit(pid uint32, processFriendlyName string, isFirstPr } if si.State == StateExit && ctr.restartManager != nil { - restart, wait, err := ctr.restartManager.ShouldRestart(uint32(exitCode), false) + restart, wait, err := ctr.restartManager.ShouldRestart(uint32(exitCode), false, time.Since(ctr.startedAt)) if err != nil { logrus.Error(err) } else if restart { diff --git a/restartmanager/restartmanager.go b/restartmanager/restartmanager.go index 38730a9dd9..4ed912ed3b 100644 --- a/restartmanager/restartmanager.go +++ b/restartmanager/restartmanager.go @@ -16,7 +16,7 @@ const ( // RestartManager defines object that controls container restarting rules. type RestartManager interface { Cancel() error - ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool) (bool, chan error, error) + ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) } type restartManager struct { @@ -41,7 +41,7 @@ func (rm *restartManager) SetPolicy(policy container.RestartPolicy) { rm.Unlock() } -func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool) (bool, chan error, error) { +func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) { if rm.policy.IsNone() { return false, nil, nil } @@ -60,7 +60,11 @@ func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped if rm.active { return false, nil, fmt.Errorf("invalid call on active restartmanager") } - + // if the container ran for more than 10s, reguardless of status and policy reset the + // the timeout back to the default. + if executionDuration.Seconds() >= 10 { + rm.timeout = 0 + } if rm.timeout == 0 { rm.timeout = defaultTimeout } else { diff --git a/restartmanager/restartmanager_test.go b/restartmanager/restartmanager_test.go index 22f6a0c20e..95a36b426b 100644 --- a/restartmanager/restartmanager_test.go +++ b/restartmanager/restartmanager_test.go @@ -1,3 +1,34 @@ package restartmanager -// FIXME +import ( + "testing" + "time" + + "github.com/docker/engine-api/types/container" +) + +func TestRestartManagerTimeout(t *testing.T) { + rm := New(container.RestartPolicy{Name: "always"}, 0).(*restartManager) + should, _, err := rm.ShouldRestart(0, false, 1*time.Second) + if err != nil { + t.Fatal(err) + } + if !should { + t.Fatal("container should be restarted") + } + if rm.timeout != 100*time.Millisecond { + t.Fatalf("restart manager should have a timeout of 100ms but has %s", rm.timeout) + } +} + +func TestRestartManagerTimeoutReset(t *testing.T) { + rm := New(container.RestartPolicy{Name: "always"}, 0).(*restartManager) + rm.timeout = 5 * time.Second + _, _, err := rm.ShouldRestart(0, false, 10*time.Second) + if err != nil { + t.Fatal(err) + } + if rm.timeout != 100*time.Millisecond { + t.Fatalf("restart manager should have a timeout of 100ms but has %s", rm.timeout) + } +}