Merge pull request #24073 from johnharris85/move-restart-policy-check-to-daemon

Move restart-policy validation from client to daemon.

daemon/container.go

@@ -255,6 +255,23 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
 		}
 	}
 
+	p := hostConfig.RestartPolicy
+
+	switch p.Name {
+	case "always", "unless-stopped", "no":
+		if p.MaximumRetryCount != 0 {
+			return nil, fmt.Errorf("maximum restart count not valid with restart policy of '%s'", p.Name)
+		}
+	case "on-failure":
+		if p.MaximumRetryCount < 1 {
+			return nil, fmt.Errorf("maximum restart count must be a positive integer")
+		}
+	case "":
+	// do nothing
+	default:
+		return nil, fmt.Errorf("invalid restart policy '%s'", p.Name)
+	}
+
 	// Now do platform-specific verification
 	return verifyPlatformContainerSettings(daemon, hostConfig, config, update)
 }

integration-cli/docker_api_containers_test.go

@@ -703,6 +703,66 @@ func (s *DockerSuite) TestContainerApiInvalidPortSyntax(c *check.C) {
 	c.Assert(string(b[:]), checker.Contains, "invalid port")
 }
 
+func (s *DockerSuite) TestContainerApiInvalidRestartPolicyName(c *check.C) {
+	config := `{
+		"Image": "busybox",
+		"HostConfig": {
+			"RestartPolicy": {
+				"Name": "something",
+				"MaximumRetryCount": 0
+			}
+		}
+	}`
+
+	res, body, err := sockRequestRaw("POST", "/containers/create", strings.NewReader(config), "application/json")
+	c.Assert(err, checker.IsNil)
+	c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError)
+
+	b, err := readBody(body)
+	c.Assert(err, checker.IsNil)
+	c.Assert(string(b[:]), checker.Contains, "invalid restart policy")
+}
+
+func (s *DockerSuite) TestContainerApiInvalidRestartPolicyRetryMismatch(c *check.C) {
+	config := `{
+		"Image": "busybox",
+		"HostConfig": {
+			"RestartPolicy": {
+				"Name": "always",
+				"MaximumRetryCount": 2
+			}
+		}
+	}`
+
+	res, body, err := sockRequestRaw("POST", "/containers/create", strings.NewReader(config), "application/json")
+	c.Assert(err, checker.IsNil)
+	c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError)
+
+	b, err := readBody(body)
+	c.Assert(err, checker.IsNil)
+	c.Assert(string(b[:]), checker.Contains, "maximum restart count not valid with restart policy")
+}
+
+func (s *DockerSuite) TestContainerApiInvalidRestartPolicyPositiveRetryCount(c *check.C) {
+	config := `{
+		"Image": "busybox",
+		"HostConfig": {
+			"RestartPolicy": {
+				"Name": "on-failure",
+				"MaximumRetryCount": -2
+			}
+		}
+	}`
+
+	res, body, err := sockRequestRaw("POST", "/containers/create", strings.NewReader(config), "application/json")
+	c.Assert(err, checker.IsNil)
+	c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError)
+
+	b, err := readBody(body)
+	c.Assert(err, checker.IsNil)
+	c.Assert(string(b[:]), checker.Contains, "maximum restart count must be a positive integer")
+}
+
 // Issue 7941 - test to make sure a "null" in JSON is just ignored.
 // W/o this fix a null in JSON would be parsed into a string var as "null"
 func (s *DockerSuite) TestContainerApiPostCreateNull(c *check.C) {

runconfig/opts/parse.go

@@ -729,34 +729,21 @@ func ParseRestartPolicy(policy string) (container.RestartPolicy, error) {
 		return p, nil
 	}
 
-	var (
-		parts = strings.Split(policy, ":")
-		name  = parts[0]
-	)
+	parts := strings.Split(policy, ":")
 
-	p.Name = name
-	switch name {
-	case "always", "unless-stopped":
-		if len(parts) > 1 {
-			return p, fmt.Errorf("maximum restart count not valid with restart policy of \"%s\"", name)
-		}
-	case "no":
-		// do nothing
-	case "on-failure":
-		if len(parts) > 2 {
-			return p, fmt.Errorf("restart count format is not valid, usage: 'on-failure:N' or 'on-failure'")
-		}
-		if len(parts) == 2 {
-			count, err := strconv.Atoi(parts[1])
-			if err != nil {
-				return p, err
-			}
-
-			p.MaximumRetryCount = count
-		}
-	default:
-		return p, fmt.Errorf("invalid restart policy %s", name)
+	if len(parts) > 2 {
+		return p, fmt.Errorf("invalid restart policy format")
 	}
+	if len(parts) == 2 {
+		count, err := strconv.Atoi(parts[1])
+		if err != nil {
+			return p, fmt.Errorf("maximum retry count must be an integer")
+		}
+
+		p.MaximumRetryCount = count
+	}
+
+	p.Name = parts[0]
 
 	return p, nil
 }

runconfig/opts/parse_test.go

@@ -556,11 +556,8 @@ func TestParseModes(t *testing.T) {
 
 func TestParseRestartPolicy(t *testing.T) {
 	invalids := map[string]string{
-		"something":          "invalid restart policy something",
-		"always:2":           "maximum restart count not valid with restart policy of \"always\"",
-		"always:2:3":         "maximum restart count not valid with restart policy of \"always\"",
-		"on-failure:invalid": `strconv.ParseInt: parsing "invalid": invalid syntax`,
-		"on-failure:2:5":     "restart count format is not valid, usage: 'on-failure:N' or 'on-failure'",
+		"always:2:3":         "invalid restart policy format",
+		"on-failure:invalid": "maximum retry count must be an integer",
 	}
 	valids := map[string]container.RestartPolicy{
 		"": {},