Merge pull request #32504 from dongluochen/healthcheck_duration

do not allow duration less than 1 ms in healthcheck parameters

api/swagger.yaml

@@ -499,16 +499,16 @@ definitions:
         items:
           type: "string"
       Interval:
-        description: "The time to wait between checks in nanoseconds. It should be 0 or not less than 1000000000(1s). 0 means inherit."
+        description: "The time to wait between checks in nanoseconds. It should be 0 or at least 1000000 (1 ms). 0 means inherit."
         type: "integer"
       Timeout:
-        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."
+        description: "The time to wait before considering the check to have hung. It should be 0 or at least 1000000 (1 ms). 0 means inherit."
         type: "integer"
       Retries:
         description: "The number of consecutive failures needed to consider a container as unhealthy. 0 means inherit."
         type: "integer"
       StartPeriod:
-        description: "Start period for the container to initialize before starting health-retries countdown in nanoseconds. 0 means inherit."
+        description: "Start period for the container to initialize before starting health-retries countdown in nanoseconds. It should be 0 or at least 1000000 (1 ms). 0 means inherit."
         type: "integer"
 
   HostConfig:

api/types/container/config.go

@@ -7,6 +7,12 @@ import (
 	"github.com/docker/go-connections/nat"
 )
 
+// MinimumDuration puts a minimum on user configured duration.
+// This is to prevent API error on time unit. For example, API may
+// set 3 as healthcheck interval with intention of 3 seconds, but
+// Docker interprets it as 3 nanoseconds.
+const MinimumDuration = 1 * time.Millisecond
+
 // HealthConfig holds configuration settings for the HEALTHCHECK feature.
 type HealthConfig struct {
 	// Test is the test to perform to check that the container is healthy.

builder/dockerfile/dispatchers.go

@@ -486,7 +486,7 @@ func cmd(req dispatchRequest) error {
 }
 
 // parseOptInterval(flag) is the duration of flag.Value, or 0 if
-// empty. An error is reported if the value is given and less than 1 second.
+// empty. An error is reported if the value is given and less than minimum duration.
 func parseOptInterval(f *Flag) (time.Duration, error) {
 	s := f.Value
 	if s == "" {
@@ -496,8 +496,8 @@ func parseOptInterval(f *Flag) (time.Duration, error) {
 	if err != nil {
 		return 0, err
 	}
-	if d < time.Duration(time.Second) {
+	if d < time.Duration(container.MinimumDuration) {
-		return 0, fmt.Errorf("Interval %#v cannot be less than 1 second", f.name)
+		return 0, fmt.Errorf("Interval %#v cannot be less than %s", f.name, container.MinimumDuration)
 	}
 	return d, nil
 }

builder/dockerfile/dispatchers_test.go

@@ -401,3 +401,21 @@ func TestShell(t *testing.T) {
 	expectedShell := strslice.StrSlice([]string{shellCmd})
 	assert.Equal(t, expectedShell, req.runConfig.Shell)
 }
+
+func TestParseOptInterval(t *testing.T) {
+	flInterval := &Flag{
+		name:     "interval",
+		flagType: stringType,
+		Value:    "50ns",
+	}
+	_, err := parseOptInterval(flInterval)
+	if err == nil {
+		t.Fatalf("Error should be presented for interval %s", flInterval.Value)
+	}
+
+	flInterval.Value = "1ms"
+	_, err = parseOptInterval(flInterval)
+	if err != nil {
+		t.Fatalf("Unexpected error: %s", err.Error())
+	}
+}

cli/command/container/opts.go

@@ -229,10 +229,10 @@ func addFlags(flags *pflag.FlagSet) *containerOptions {
 
 	// Health-checking
 	flags.StringVar(&copts.healthCmd, "health-cmd", "", "Command to run to check health")
-	flags.DurationVar(&copts.healthInterval, "health-interval", 0, "Time between running the check (ns|us|ms|s|m|h) (default 0s)")
+	flags.DurationVar(&copts.healthInterval, "health-interval", 0, "Time between running the check (ms|s|m|h) (default 0s)")
 	flags.IntVar(&copts.healthRetries, "health-retries", 0, "Consecutive failures needed to report unhealthy")
-	flags.DurationVar(&copts.healthTimeout, "health-timeout", 0, "Maximum time to allow one check to run (ns|us|ms|s|m|h) (default 0s)")
+	flags.DurationVar(&copts.healthTimeout, "health-timeout", 0, "Maximum time to allow one check to run (ms|s|m|h) (default 0s)")
-	flags.DurationVar(&copts.healthStartPeriod, "health-start-period", 0, "Start period for the container to initialize before starting health-retries countdown (ns|us|ms|s|m|h) (default 0s)")
+	flags.DurationVar(&copts.healthStartPeriod, "health-start-period", 0, "Start period for the container to initialize before starting health-retries countdown (ms|s|m|h) (default 0s)")
 	flags.SetAnnotation("health-start-period", "version", []string{"1.29"})
 	flags.BoolVar(&copts.noHealthcheck, "no-healthcheck", false, "Disable any container-specified HEALTHCHECK")
 

cli/command/service/opts.go

@@ -802,13 +802,13 @@ func addServiceFlags(flags *pflag.FlagSet, opts *serviceOptions, defaultFlagValu
 
 	flags.StringVar(&opts.healthcheck.cmd, flagHealthCmd, "", "Command to run to check health")
 	flags.SetAnnotation(flagHealthCmd, "version", []string{"1.25"})
-	flags.Var(&opts.healthcheck.interval, flagHealthInterval, "Time between running the check (ns|us|ms|s|m|h)")
+	flags.Var(&opts.healthcheck.interval, flagHealthInterval, "Time between running the check (ms|s|m|h)")
 	flags.SetAnnotation(flagHealthInterval, "version", []string{"1.25"})
-	flags.Var(&opts.healthcheck.timeout, flagHealthTimeout, "Maximum time to allow one check to run (ns|us|ms|s|m|h)")
+	flags.Var(&opts.healthcheck.timeout, flagHealthTimeout, "Maximum time to allow one check to run (ms|s|m|h)")
 	flags.SetAnnotation(flagHealthTimeout, "version", []string{"1.25"})
 	flags.IntVar(&opts.healthcheck.retries, flagHealthRetries, 0, "Consecutive failures needed to report unhealthy")
 	flags.SetAnnotation(flagHealthRetries, "version", []string{"1.25"})
-	flags.Var(&opts.healthcheck.startPeriod, flagHealthStartPeriod, "Start period for the container to initialize before counting retries towards unstable (ns|us|ms|s|m|h)")
+	flags.Var(&opts.healthcheck.startPeriod, flagHealthStartPeriod, "Start period for the container to initialize before counting retries towards unstable (ms|s|m|h)")
 	flags.SetAnnotation(flagHealthStartPeriod, "version", []string{"1.29"})
 	flags.BoolVar(&opts.healthcheck.noHealthcheck, flagNoHealthcheck, false, "Disable any container-specified HEALTHCHECK")
 	flags.SetAnnotation(flagNoHealthcheck, "version", []string{"1.25"})

daemon/container.go

@@ -244,20 +244,20 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
 
 		// Validate the healthcheck params of Config
 		if config.Healthcheck != nil {
-			if config.Healthcheck.Interval != 0 && config.Healthcheck.Interval < time.Second {
+			if config.Healthcheck.Interval != 0 && config.Healthcheck.Interval < containertypes.MinimumDuration {
-				return nil, fmt.Errorf("Interval in Healthcheck cannot be less than one second")
+				return nil, fmt.Errorf("Interval in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
 			}
 
-			if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < time.Second {
+			if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < containertypes.MinimumDuration {
-				return nil, fmt.Errorf("Timeout in Healthcheck cannot be less than one second")
+				return nil, fmt.Errorf("Timeout in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
 			}
 
 			if config.Healthcheck.Retries < 0 {
 				return nil, fmt.Errorf("Retries in Healthcheck cannot be negative")
 			}
 
-			if config.Healthcheck.StartPeriod < 0 {
+			if config.Healthcheck.StartPeriod != 0 && config.Healthcheck.StartPeriod < containertypes.MinimumDuration {
-				return nil, fmt.Errorf("StartPeriod in Healthcheck cannot be negative")
+				return nil, fmt.Errorf("StartPeriod in Healthcheck cannot be less than %s", containertypes.MinimumDuration)
 			}
 		}
 	}

docs/api/v1.24.md

@@ -285,7 +285,8 @@ Create a container
               "Test": ["CMD-SHELL", "curl localhost:3000"],
               "Interval": 1000000000,
               "Timeout": 10000000000,
-              "Retries": 10
+              "Retries": 10,
+              "StartPeriod": 60000000000
            },
            "WorkingDir": "",
            "NetworkDisabled": false,
@@ -397,9 +398,10 @@ Create a container
               + `{"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.
+    -     **Interval** - The time to wait between checks in nanoseconds. It should be 0 or at least 1000000 (1 ms). 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.
+    -     **Timeout** - The time to wait before considering the check to have hung. It should be 0 or at least 1000000 (1 ms). 0 means inherit.
     -     **Retries** - The number of consecutive failures needed to consider a container as unhealthy. 0 means inherit.
+    -     **StartPeriod** - The time to wait for container initialization before starting health-retries countdown in nanoseconds. It should be 0 or at least 1000000 (1 ms). 0 means inherit.
 -   **WorkingDir** - A string specifying the working directory for commands to
       run in.
 -   **NetworkDisabled** - Boolean value, when true disables networking for the

docs/reference/commandline/service_create.md

@@ -33,10 +33,10 @@ Options:
       --env-file list                      Read in a file of environment variables
       --group list                         Set one or more supplementary user groups for the container
       --health-cmd string                  Command to run to check health
-      --health-interval duration           Time between running the check (ns|us|ms|s|m|h)
+      --health-interval duration           Time between running the check (ms|s|m|h)
       --health-retries int                 Consecutive failures needed to report unhealthy
-      --health-start-period duration       Start period for the container to initialize before counting retries towards unstable (ns|us|ms|s|m|h)
+      --health-start-period duration       Start period for the container to initialize before counting retries towards unstable (ms|s|m|h)
-      --health-timeout duration            Maximum time to allow one check to run (ns|us|ms|s|m|h)
+      --health-timeout duration            Maximum time to allow one check to run (ms|s|m|h)
       --help                               Print usage
       --host list                          Set one or more custom host-to-IP mappings (host:ip)
       --hostname string                    Container hostname

docs/reference/commandline/service_update.md

@@ -41,10 +41,10 @@ Options:
       --group-add list                     Add an additional supplementary user group to the container
       --group-rm list                      Remove a previously added supplementary user group from the container
       --health-cmd string                  Command to run to check health
-      --health-interval duration           Time between running the check (ns|us|ms|s|m|h)
+      --health-interval duration           Time between running the check (ms|s|m|h)
       --health-retries int                 Consecutive failures needed to report unhealthy
-      --health-start-period duration       Start period for the container to initialize before counting retries towards unstable (ns|us|ms|s|m|h)
+      --health-start-period duration       Start period for the container to initialize before counting retries towards unstable (ms|s|m|h)
-      --health-timeout duration            Maximum time to allow one check to run (ns|us|ms|s|m|h)
+      --health-timeout duration            Maximum time to allow one check to run (ms|s|m|h)
       --help                               Print usage
       --host-add list                      Add or update a custom host-to-IP mapping (host:ip)
       --host-rm list                       Remove a custom host-to-IP mapping (host:ip)

integration-cli/docker_api_create_test.go

@@ -1,9 +1,11 @@
 package main
 
 import (
+	"fmt"
 	"net/http"
 	"time"
 
+	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/request"
 	"github.com/go-check/check"
@@ -91,8 +93,8 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) {
 	config := map[string]interface{}{
 		"Image": "busybox",
 		"Healthcheck": map[string]interface{}{
-			"Interval": time.Duration(-10000000),
+			"Interval": -10 * time.Millisecond,
-			"Timeout":  time.Duration(1000000000),
+			"Timeout":  time.Second,
 			"Retries":  int(1000),
 		},
 	}
@@ -100,39 +102,38 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) {
 	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"
+	expected := fmt.Sprintf("Interval in Healthcheck cannot be less than %s", container.MinimumDuration)
 	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
 
-	// test invalid Interval in Healthcheck: larger than 0s but less than 1s
+	// test invalid Interval in Healthcheck: larger than 0s but less than 1ms
 	name = "test2"
 	config = map[string]interface{}{
 		"Image": "busybox",
 		"Healthcheck": map[string]interface{}{
-			"Interval": time.Duration(500000000),
+			"Interval": 500 * time.Microsecond,
-			"Timeout":  time.Duration(1000000000),
+			"Timeout":  time.Second,
 			"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
+	// test invalid Timeout in Healthcheck: less than 1ms
 	name = "test3"
 	config = map[string]interface{}{
 		"Image": "busybox",
 		"Healthcheck": map[string]interface{}{
-			"Interval": time.Duration(1000000000),
+			"Interval": time.Second,
-			"Timeout":  time.Duration(-100000000),
+			"Timeout":  -100 * time.Millisecond,
 			"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"
+	expected = fmt.Sprintf("Timeout in Healthcheck cannot be less than %s", container.MinimumDuration)
 	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
 
 	// test invalid Retries in Healthcheck: less than 0
@@ -140,8 +141,8 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) {
 	config = map[string]interface{}{
 		"Image": "busybox",
 		"Healthcheck": map[string]interface{}{
-			"Interval": time.Duration(1000000000),
+			"Interval": time.Second,
-			"Timeout":  time.Duration(1000000000),
+			"Timeout":  time.Second,
 			"Retries":  int(-10),
 		},
 	}
@@ -150,4 +151,21 @@ func (s *DockerSuite) TestAPICreateWithInvalidHealthcheckParams(c *check.C) {
 	c.Assert(status, check.Equals, http.StatusInternalServerError)
 	expected = "Retries in Healthcheck cannot be negative"
 	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
+
+	// test invalid StartPeriod in Healthcheck: not 0 and less than 1ms
+	name = "test3"
+	config = map[string]interface{}{
+		"Image": "busybox",
+		"Healthcheck": map[string]interface{}{
+			"Interval":    time.Second,
+			"Timeout":     time.Second,
+			"Retries":     int(1000),
+			"StartPeriod": 100 * time.Microsecond,
+		},
+	}
+	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 = fmt.Sprintf("StartPeriod in Healthcheck cannot be less than %s", container.MinimumDuration)
+	c.Assert(getErrorMessage(c, body), checker.Contains, expected)
 }