Merge pull request #30203 from allencloud/validate-healthcheck-params-in-daemon-side

validate healthcheck params in daemon side

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."

cli/command/container/opts.go

@@ -516,6 +516,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,

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 {

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

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)
+}

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