From 94e95e4711643640701bd614902e75a2d01f12c5 Mon Sep 17 00:00:00 2001 From: johnharris85 Date: Tue, 28 Jun 2016 15:33:55 -0700 Subject: [PATCH] Move restart-policy validation from client to daemon. Signed-off-by: John Harris --- daemon/container.go | 17 ++++++ integration-cli/docker_api_containers_test.go | 60 +++++++++++++++++++ runconfig/opts/parse.go | 39 ++++-------- runconfig/opts/parse_test.go | 7 +-- 4 files changed, 92 insertions(+), 31 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index b9f63dedde..098af39d3f 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -251,6 +251,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) } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index fcb69e591f..d8dad2f1c8 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/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) { diff --git a/runconfig/opts/parse.go b/runconfig/opts/parse.go index 9d2ba67ae6..c1d3ee2cfb 100644 --- a/runconfig/opts/parse.go +++ b/runconfig/opts/parse.go @@ -722,34 +722,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 } diff --git a/runconfig/opts/parse_test.go b/runconfig/opts/parse_test.go index 371770f2eb..100b5d93a1 100644 --- a/runconfig/opts/parse_test.go +++ b/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{ "": {},