1. docker
  2. Commit ecf889816c8ac58d13020a76df36cf48546d9f9f
Browse code

Fix restartpolicy max-retry validation

the restart policy validation was moved from
the client to the daemon in 94e95e4711643640701bd614902e75a2d01f12c5

As part of that change, retry-counts < 1
were marked as "invalid".

However, the default is 0 (unlimited), causing

docker run -d --restart=on-failure nginx

To fail.

This changes the validation to only invalidate
retry-counts < 0.

A test was added, and other tests renamed
to allow running just these tests :)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9db5d649aea1c3d4728d0159bb5175a49f77748e)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>

Sebastiaan van Stijn authored on 2016/12/02 06:24:30
Showing 3 changed files
daemon/container.go
History View file @ ecf8898
... ... 
@@ -268,11 +268,11 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
268 268 
 	switch p.Name {
269 269 
 	case "always", "unless-stopped", "no":
270 270 
 		if p.MaximumRetryCount != 0 {
271 
-			return nil, fmt.Errorf("maximum restart count not valid with restart policy of '%s'", p.Name)
271 
+			return nil, fmt.Errorf("maximum retry count cannot be used with restart policy '%s'", p.Name)
272 272 
 		}
273 273 
 	case "on-failure":
274 
-		if p.MaximumRetryCount < 1 {
275 
-			return nil, fmt.Errorf("maximum restart count must be a positive integer")
274 
+		if p.MaximumRetryCount < 0 {
275 
+			return nil, fmt.Errorf("maximum retry count cannot be negative")
276 276 
 		}
277 277 
 	case "":
278 278 
 	// do nothing
integration-cli/docker_api_containers_test.go
History View file @ ecf8898
... ... 
@@ -728,7 +728,7 @@ func (s *DockerSuite) TestContainerAPIInvalidPortSyntax(c *check.C) {
728 728 
 	c.Assert(string(b[:]), checker.Contains, "invalid port")
729 729 
 }
730 730
731 
-func (s *DockerSuite) TestContainerAPIInvalidRestartPolicyName(c *check.C) {
731 
+func (s *DockerSuite) TestContainerAPIRestartPolicyInvalidPolicyName(c *check.C) {
732 732 
 	config := `{
733 733 
 		"Image": "busybox",
734 734 
 		"HostConfig": {
... ... 
@@ -748,7 +748,7 @@ func (s *DockerSuite) TestContainerAPIInvalidRestartPolicyName(c *check.C) {
748 748 
 	c.Assert(string(b[:]), checker.Contains, "invalid restart policy")
749 749 
 }
750 750
751 
-func (s *DockerSuite) TestContainerAPIInvalidRestartPolicyRetryMismatch(c *check.C) {
751 
+func (s *DockerSuite) TestContainerAPIRestartPolicyRetryMismatch(c *check.C) {
752 752 
 	config := `{
753 753 
 		"Image": "busybox",
754 754 
 		"HostConfig": {
... ... 
@@ -765,10 +765,10 @@ func (s *DockerSuite) TestContainerAPIInvalidRestartPolicyRetryMismatch(c *check
765 765
766 766 
 	b, err := readBody(body)
767 767 
 	c.Assert(err, checker.IsNil)
768 
-	c.Assert(string(b[:]), checker.Contains, "maximum restart count not valid with restart policy")
768 
+	c.Assert(string(b[:]), checker.Contains, "maximum retry count cannot be used with restart policy")
769 769 
 }
770 770
771 
-func (s *DockerSuite) TestContainerAPIInvalidRestartPolicyPositiveRetryCount(c *check.C) {
771 
+func (s *DockerSuite) TestContainerAPIRestartPolicyNegativeRetryCount(c *check.C) {
772 772 
 	config := `{
773 773 
 		"Image": "busybox",
774 774 
 		"HostConfig": {
... ... 
@@ -785,7 +785,23 @@ func (s *DockerSuite) TestContainerAPIInvalidRestartPolicyPositiveRetryCount(c *
785 785
786 786 
 	b, err := readBody(body)
787 787 
 	c.Assert(err, checker.IsNil)
788 
-	c.Assert(string(b[:]), checker.Contains, "maximum restart count must be a positive integer")
788 
+	c.Assert(string(b[:]), checker.Contains, "maximum retry count cannot be negative")
789 
+}
790 
+
791 
+func (s *DockerSuite) TestContainerAPIRestartPolicyDefaultRetryCount(c *check.C) {
792 
+	config := `{
793 
+		"Image": "busybox",
794 
+		"HostConfig": {
795 
+			"RestartPolicy": {
796 
+				"Name": "on-failure",
797 
+				"MaximumRetryCount": 0
798 
+			}
799 
+		}
800 
+	}`
801 
+
802 
+	res, _, err := sockRequestRaw("POST", "/containers/create", strings.NewReader(config), "application/json")
803 
+	c.Assert(err, checker.IsNil)
804 
+	c.Assert(res.StatusCode, checker.Equals, http.StatusCreated)
789 805 
 }
790 806
791 807 
 // Issue 7941 - test to make sure a "null" in JSON is just ignored.
integration-cli/docker_cli_restart_test.go
History View file @ ecf8898
... ... 
@@ -74,7 +74,7 @@ func (s *DockerSuite) TestRestartWithVolumes(c *check.C) {
74 74 
 }
75 75
76 76 
 func (s *DockerSuite) TestRestartPolicyNO(c *check.C) {
77 
-	out, _ := dockerCmd(c, "run", "-d", "--restart=no", "busybox", "false")
77 
+	out, _ := dockerCmd(c, "create", "--restart=no", "busybox")
78 78
79 79 
 	id := strings.TrimSpace(string(out))
80 80 
 	name := inspectField(c, id, "HostConfig.RestartPolicy.Name")
... ... 
@@ -82,7 +82,7 @@ func (s *DockerSuite) TestRestartPolicyNO(c *check.C) {
82 82 
 }
83 83
84 84 
 func (s *DockerSuite) TestRestartPolicyAlways(c *check.C) {
85 
-	out, _ := dockerCmd(c, "run", "-d", "--restart=always", "busybox", "false")
85 
+	out, _ := dockerCmd(c, "create", "--restart=always", "busybox")
86 86
87 87 
 	id := strings.TrimSpace(string(out))
88 88 
 	name := inspectField(c, id, "HostConfig.RestartPolicy.Name")
... ... 
@@ -95,12 +95,36 @@ func (s *DockerSuite) TestRestartPolicyAlways(c *check.C) {
95 95 
 }
96 96
97 97 
 func (s *DockerSuite) TestRestartPolicyOnFailure(c *check.C) {
98 
-	out, _ := dockerCmd(c, "run", "-d", "--restart=on-failure:1", "busybox", "false")
98 
+	out, _, err := dockerCmdWithError("create", "--restart=on-failure:-1", "busybox")
99 
+	c.Assert(err, check.NotNil, check.Commentf(out))
100 
+	c.Assert(out, checker.Contains, "maximum retry count cannot be negative")
101 
+
102 
+	out, _ = dockerCmd(c, "create", "--restart=on-failure:1", "busybox")
99 103
100 104 
 	id := strings.TrimSpace(string(out))
101 105 
 	name := inspectField(c, id, "HostConfig.RestartPolicy.Name")
106 
+	maxRetry := inspectField(c, id, "HostConfig.RestartPolicy.MaximumRetryCount")
107 
+
102 108 
 	c.Assert(name, checker.Equals, "on-failure")
109 
+	c.Assert(maxRetry, checker.Equals, "1")
110 
+
111 
+	out, _ = dockerCmd(c, "create", "--restart=on-failure:0", "busybox")
103 112
113 
+	id = strings.TrimSpace(string(out))
114 
+	name = inspectField(c, id, "HostConfig.RestartPolicy.Name")
115 
+	maxRetry = inspectField(c, id, "HostConfig.RestartPolicy.MaximumRetryCount")
116 
+
117 
+	c.Assert(name, checker.Equals, "on-failure")
118 
+	c.Assert(maxRetry, checker.Equals, "0")
119 
+
120 
+	out, _ = dockerCmd(c, "create", "--restart=on-failure", "busybox")
121 
+
122 
+	id = strings.TrimSpace(string(out))
123 
+	name = inspectField(c, id, "HostConfig.RestartPolicy.Name")
124 
+	maxRetry = inspectField(c, id, "HostConfig.RestartPolicy.MaximumRetryCount")
125 
+
126 
+	c.Assert(name, checker.Equals, "on-failure")
127 
+	c.Assert(maxRetry, checker.Equals, "0")
104 128 
 }
105 129
106 130 
 // a good container with --restart=on-failure:3