From e399c558e6eeef519a84a2e0bb11dade5a345014 Mon Sep 17 00:00:00 2001 From: allencloud Date: Tue, 17 Jan 2017 15:55:45 +0800 Subject: [PATCH] validate healthcheck params in daemon side Signed-off-by: allencloud --- api/swagger.yaml | 4 +- cli/command/container/opts.go | 3 + daemon/container.go | 15 +++++ docs/api/v1.24.md | 15 +++++ integration-cli/docker_api_create_test.go | 68 +++++++++++++++++++++++ integration-cli/docker_cli_health_test.go | 6 +- 6 files changed, 106 insertions(+), 5 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index 524ac5c356..e52815e67a 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -742,10 +742,10 @@ definitions: items: type: "string" Interval: - description: "The time to wait between checks in nanoseconds. 0 means inherit." + description: "The time to wait between checks in nanoseconds. It should be 0 or not less than 1000000000(1s). 0 means inherit." type: "integer" Timeout: - description: "The time to wait before considering the check to have hung. 0 means inherit." + description: "The time to wait before considering the check to have hung. It should be 0 or not less than 1000000000(1s). 0 means inherit." type: "integer" Retries: description: "The number of consecutive failures needed to consider a container as unhealthy. 0 means inherit." diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index 55cc3c3b29..245d8e856d 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -511,6 +511,9 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*container.Config, *c if copts.healthTimeout < 0 { return nil, nil, nil, fmt.Errorf("--health-timeout cannot be negative") } + if copts.healthRetries < 0 { + return nil, nil, nil, fmt.Errorf("--health-retries cannot be negative") + } healthConfig = &container.HealthConfig{ Test: probe, diff --git a/daemon/container.go b/daemon/container.go index 53705c9a48..adbfc21d9b 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -231,6 +231,21 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon return nil, err } } + + // Validate the healthcheck params of Config + if config.Healthcheck != nil { + if config.Healthcheck.Interval != 0 && config.Healthcheck.Interval < time.Second { + return nil, fmt.Errorf("Interval in Healthcheck cannot be less than one second") + } + + if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < time.Second { + return nil, fmt.Errorf("Timeout in Healthcheck cannot be less than one second") + } + + if config.Healthcheck.Retries < 0 { + return nil, fmt.Errorf("Retries in Healthcheck cannot be negative") + } + } } if hostConfig == nil { diff --git a/docs/api/v1.24.md b/docs/api/v1.24.md index 963a781ced..7514d05564 100644 --- a/docs/api/v1.24.md +++ b/docs/api/v1.24.md @@ -281,6 +281,12 @@ Create a container "Volumes": { "/volumes/data": {} }, + "Healthcheck":{ + "Test": ["CMD-SHELL", "curl localhost:3000"], + "Interval": 1000000000, + "Timeout": 10000000000, + "Retries": 10 + }, "WorkingDir": "", "NetworkDisabled": false, "MacAddress": "12:34:56:78:9a:bc", @@ -385,6 +391,15 @@ Create a container - **Image** - A string specifying the image name to use for the container. - **Volumes** - An object mapping mount point paths (strings) inside the container to empty objects. +- **Healthcheck** - A test to perform to check that the container is healthy. + - **Test** - The test to perform. Possible values are: + + `{}` inherit healthcheck from image or parent image + + `{"NONE"}` disable healthcheck + + `{"CMD", args...}` exec arguments directly + + `{"CMD-SHELL", command}` run command with system's default shell + - **Interval** - The time to wait between checks in nanoseconds. It should be 0 or not less than 1000000000(1s). 0 means inherit. + - **Timeout** - The time to wait before considering the check to have hung. It should be 0 or not less than 1000000000(1s). 0 means inherit. + - **Retries** - The number of consecutive failures needed to consider a container as unhealthy. 0 means inherit. - **WorkingDir** - A string specifying the working directory for commands to run in. - **NetworkDisabled** - Boolean value, when true disables networking for the diff --git a/integration-cli/docker_api_create_test.go b/integration-cli/docker_api_create_test.go index c2616c2e73..7328f4d068 100644 --- a/integration-cli/docker_api_create_test.go +++ b/integration-cli/docker_api_create_test.go @@ -2,6 +2,7 @@ package main import ( "net/http" + "time" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/request" @@ -83,3 +84,70 @@ func (s *DockerSuite) TestAPICreateEmptyEnv(c *check.C) { expected = "invalid environment variable: =foo" c.Assert(getErrorMessage(c, body), checker.Contains, expected) } + +func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) { + // test invalid Interval in Healthcheck: less than 0s + name := "test1" + config := map[string]interface{}{ + "Image": "busybox", + "Healthcheck": map[string]interface{}{ + "Interval": time.Duration(-10000000), + "Timeout": time.Duration(1000000000), + "Retries": int(1000), + }, + } + + status, body, err := request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) + c.Assert(err, check.IsNil) + c.Assert(status, check.Equals, http.StatusInternalServerError) + expected := "Interval in Healthcheck cannot be less than one second" + c.Assert(getErrorMessage(c, body), checker.Contains, expected) + + // test invalid Interval in Healthcheck: larger than 0s but less than 1s + name = "test2" + config = map[string]interface{}{ + "Image": "busybox", + "Healthcheck": map[string]interface{}{ + "Interval": time.Duration(500000000), + "Timeout": time.Duration(1000000000), + "Retries": int(1000), + }, + } + status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) + c.Assert(err, check.IsNil) + c.Assert(status, check.Equals, http.StatusInternalServerError) + expected = "Interval in Healthcheck cannot be less than one second" + c.Assert(getErrorMessage(c, body), checker.Contains, expected) + + // test invalid Timeout in Healthcheck: less than 1s + name = "test3" + config = map[string]interface{}{ + "Image": "busybox", + "Healthcheck": map[string]interface{}{ + "Interval": time.Duration(1000000000), + "Timeout": time.Duration(-100000000), + "Retries": int(1000), + }, + } + status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) + c.Assert(err, check.IsNil) + c.Assert(status, check.Equals, http.StatusInternalServerError) + expected = "Timeout in Healthcheck cannot be less than one second" + c.Assert(getErrorMessage(c, body), checker.Contains, expected) + + // test invalid Retries in Healthcheck: less than 0 + name = "test4" + config = map[string]interface{}{ + "Image": "busybox", + "Healthcheck": map[string]interface{}{ + "Interval": time.Duration(1000000000), + "Timeout": time.Duration(1000000000), + "Retries": int(-10), + }, + } + status, body, err = request.SockRequest("POST", "/containers/create?name="+name, config, daemonHost()) + c.Assert(err, check.IsNil) + c.Assert(status, check.Equals, http.StatusInternalServerError) + expected = "Retries in Healthcheck cannot be negative" + c.Assert(getErrorMessage(c, body), checker.Contains, expected) +} diff --git a/integration-cli/docker_cli_health_test.go b/integration-cli/docker_cli_health_test.go index 911e035f8c..e04433c42e 100644 --- a/integration-cli/docker_cli_health_test.go +++ b/integration-cli/docker_cli_health_test.go @@ -95,7 +95,7 @@ func (s *DockerSuite) TestHealth(c *check.C) { // Enable the checks from the CLI _, _ = dockerCmd(c, "run", "-d", "--name=fatal_healthcheck", - "--health-interval=0.5s", + "--health-interval=1s", "--health-retries=3", "--health-cmd=cat /status", "no_healthcheck") @@ -121,13 +121,13 @@ func (s *DockerSuite) TestHealth(c *check.C) { // Note: if the interval is too small, it seems that Docker spends all its time running health // checks and never gets around to killing it. _, _ = dockerCmd(c, "run", "-d", "--name=test", - "--health-interval=1s", "--health-cmd=sleep 5m", "--health-timeout=1ms", imageName) + "--health-interval=1s", "--health-cmd=sleep 5m", "--health-timeout=1s", imageName) waitForHealthStatus(c, "test", "starting", "unhealthy") health = getHealth(c, "test") last = health.Log[len(health.Log)-1] c.Check(health.Status, checker.Equals, "unhealthy") c.Check(last.ExitCode, checker.Equals, -1) - c.Check(last.Output, checker.Equals, "Health check exceeded timeout (1ms)") + c.Check(last.Output, checker.Equals, "Health check exceeded timeout (1s)") dockerCmd(c, "rm", "-f", "test") // Check JSON-format