Merge pull request #22125 from crosbymichael/restart-timeout

Reset restart timeout if execution longer than 10s
Upstream-commit: 17d5c97c900d90bee7a1ba4182bf9ea51e5c386d
Component: engine

components/engine/container/container.go

@@ -520,7 +520,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
 }
 

components/engine/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.

components/engine/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"
@@ -90,6 +91,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
@@ -134,7 +136,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 {

components/engine/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 {

components/engine/restartmanager/restartmanager.go

@@ -21,7 +21,7 @@ var ErrRestartCanceled = errors.New("restart canceled")
 // 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 {
@@ -46,7 +46,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
 	}
@@ -65,7 +65,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 {

components/engine/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)
+	}
+}