Browse code

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 <crosbymichael@gmail.com>

Michael Crosby authored on 2016/04/19 05:10:15
Showing 6 changed files
... ...
@@ -519,7 +519,7 @@ func copyEscapable(dst io.Writer, src io.ReadCloser, keys []byte) (written int64
519 519
 // ShouldRestart decides whether the daemon should restart the container or not.
520 520
 // This is based on the container's restart policy.
521 521
 func (container *Container) ShouldRestart() bool {
522
-	shouldRestart, _, _ := container.restartManager.ShouldRestart(uint32(container.ExitCode), container.HasBeenManuallyStopped)
522
+	shouldRestart, _, _ := container.restartManager.ShouldRestart(uint32(container.ExitCode), container.HasBeenManuallyStopped, container.FinishedAt.Sub(container.StartedAt))
523 523
 	return shouldRestart
524 524
 }
525 525
 
... ...
@@ -2,6 +2,7 @@ package libcontainerd
2 2
 
3 3
 import (
4 4
 	"fmt"
5
+	"time"
5 6
 
6 7
 	"github.com/docker/docker/restartmanager"
7 8
 )
... ...
@@ -18,6 +19,7 @@ type containerCommon struct {
18 18
 	restartManager restartmanager.RestartManager
19 19
 	restarting     bool
20 20
 	processes      map[string]*process
21
+	startedAt      time.Time
21 22
 }
22 23
 
23 24
 // WithRestartManager sets the restartmanager to be used with the container.
... ...
@@ -6,6 +6,7 @@ import (
6 6
 	"os"
7 7
 	"path/filepath"
8 8
 	"syscall"
9
+	"time"
9 10
 
10 11
 	"github.com/Sirupsen/logrus"
11 12
 	containerd "github.com/docker/containerd/api/grpc/types"
... ...
@@ -74,6 +75,7 @@ func (ctr *container) start() error {
74 74
 		ctr.closeFifos(iopipe)
75 75
 		return err
76 76
 	}
77
+	ctr.startedAt = time.Now()
77 78
 
78 79
 	if err := ctr.client.backend.AttachStreams(ctr.containerID, *iopipe); err != nil {
79 80
 		return err
... ...
@@ -118,7 +120,7 @@ func (ctr *container) handleEvent(e *containerd.Event) error {
118 118
 			st.State = StateExitProcess
119 119
 		}
120 120
 		if st.State == StateExit && ctr.restartManager != nil {
121
-			restart, wait, err := ctr.restartManager.ShouldRestart(e.Status, false)
121
+			restart, wait, err := ctr.restartManager.ShouldRestart(e.Status, false, time.Since(ctr.startedAt))
122 122
 			if err != nil {
123 123
 				logrus.Warnf("container %s %v", ctr.containerID, err)
124 124
 			} else if restart {
... ...
@@ -4,6 +4,7 @@ import (
4 4
 	"io"
5 5
 	"strings"
6 6
 	"syscall"
7
+	"time"
7 8
 
8 9
 	"github.com/Microsoft/hcsshim"
9 10
 	"github.com/Sirupsen/logrus"
... ...
@@ -78,6 +79,7 @@ func (ctr *container) start() error {
78 78
 		}
79 79
 		return err
80 80
 	}
81
+	ctr.startedAt = time.Now()
81 82
 
82 83
 	// Convert io.ReadClosers to io.Readers
83 84
 	if stdout != nil {
... ...
@@ -168,7 +170,7 @@ func (ctr *container) waitExit(pid uint32, processFriendlyName string, isFirstPr
168 168
 		}
169 169
 
170 170
 		if si.State == StateExit && ctr.restartManager != nil {
171
-			restart, wait, err := ctr.restartManager.ShouldRestart(uint32(exitCode), false)
171
+			restart, wait, err := ctr.restartManager.ShouldRestart(uint32(exitCode), false, time.Since(ctr.startedAt))
172 172
 			if err != nil {
173 173
 				logrus.Error(err)
174 174
 			} else if restart {
... ...
@@ -16,7 +16,7 @@ const (
16 16
 // RestartManager defines object that controls container restarting rules.
17 17
 type RestartManager interface {
18 18
 	Cancel() error
19
-	ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool) (bool, chan error, error)
19
+	ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error)
20 20
 }
21 21
 
22 22
 type restartManager struct {
... ...
@@ -41,7 +41,7 @@ func (rm *restartManager) SetPolicy(policy container.RestartPolicy) {
41 41
 	rm.Unlock()
42 42
 }
43 43
 
44
-func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool) (bool, chan error, error) {
44
+func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped bool, executionDuration time.Duration) (bool, chan error, error) {
45 45
 	if rm.policy.IsNone() {
46 46
 		return false, nil, nil
47 47
 	}
... ...
@@ -60,7 +60,11 @@ func (rm *restartManager) ShouldRestart(exitCode uint32, hasBeenManuallyStopped
60 60
 	if rm.active {
61 61
 		return false, nil, fmt.Errorf("invalid call on active restartmanager")
62 62
 	}
63
-
63
+	// if the container ran for more than 10s, reguardless of status and policy reset the
64
+	// the timeout back to the default.
65
+	if executionDuration.Seconds() >= 10 {
66
+		rm.timeout = 0
67
+	}
64 68
 	if rm.timeout == 0 {
65 69
 		rm.timeout = defaultTimeout
66 70
 	} else {
... ...
@@ -1,3 +1,34 @@
1 1
 package restartmanager
2 2
 
3
-// FIXME
3
+import (
4
+	"testing"
5
+	"time"
6
+
7
+	"github.com/docker/engine-api/types/container"
8
+)
9
+
10
+func TestRestartManagerTimeout(t *testing.T) {
11
+	rm := New(container.RestartPolicy{Name: "always"}, 0).(*restartManager)
12
+	should, _, err := rm.ShouldRestart(0, false, 1*time.Second)
13
+	if err != nil {
14
+		t.Fatal(err)
15
+	}
16
+	if !should {
17
+		t.Fatal("container should be restarted")
18
+	}
19
+	if rm.timeout != 100*time.Millisecond {
20
+		t.Fatalf("restart manager should have a timeout of 100ms but has %s", rm.timeout)
21
+	}
22
+}
23
+
24
+func TestRestartManagerTimeoutReset(t *testing.T) {
25
+	rm := New(container.RestartPolicy{Name: "always"}, 0).(*restartManager)
26
+	rm.timeout = 5 * time.Second
27
+	_, _, err := rm.ShouldRestart(0, false, 10*time.Second)
28
+	if err != nil {
29
+		t.Fatal(err)
30
+	}
31
+	if rm.timeout != 100*time.Millisecond {
32
+		t.Fatalf("restart manager should have a timeout of 100ms but has %s", rm.timeout)
33
+	}
34
+}