diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 1a6d7503ff..69fb193321 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/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: diff --git a/components/engine/api/types/container/config.go b/components/engine/api/types/container/config.go index 02e1b87a78..55a03fc981 100644 --- a/components/engine/api/types/container/config.go +++ b/components/engine/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. diff --git a/components/engine/builder/dockerfile/dispatchers.go b/components/engine/builder/dockerfile/dispatchers.go index 9a91756002..e8b2768c86 100644 --- a/components/engine/builder/dockerfile/dispatchers.go +++ b/components/engine/builder/dockerfile/dispatchers.go @@ -497,7 +497,7 @@ func cmd(b *Builder, args []string, attributes map[string]bool, original string) } // 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 == "" { @@ -507,8 +507,8 @@ func parseOptInterval(f *Flag) (time.Duration, error) { if err != nil { return 0, err } - if d < time.Duration(time.Second) { - return 0, fmt.Errorf("Interval %#v cannot be less than 1 second", f.name) + if d < time.Duration(container.MinimumDuration) { + return 0, fmt.Errorf("Interval %#v cannot be less than %s", f.name, container.MinimumDuration) } return d, nil } diff --git a/components/engine/builder/dockerfile/dispatchers_test.go b/components/engine/builder/dockerfile/dispatchers_test.go index 2ce8d75e53..f877798bd8 100644 --- a/components/engine/builder/dockerfile/dispatchers_test.go +++ b/components/engine/builder/dockerfile/dispatchers_test.go @@ -530,3 +530,21 @@ func TestShell(t *testing.T) { t.Fatalf("Shell should be set to %s, got %s", expectedShell, b.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()) + } +} diff --git a/components/engine/daemon/container.go b/components/engine/daemon/container.go index a3482687b7..6a660ba4f3 100644 --- a/components/engine/daemon/container.go +++ b/components/engine/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.Millisecond { - return nil, fmt.Errorf("Interval in Healthcheck cannot be less than one millisecond") + if config.Healthcheck.Interval != 0 && config.Healthcheck.Interval < containertypes.MinimumDuration { + return nil, fmt.Errorf("Interval in Healthcheck cannot be less than %s", containertypes.MinimumDuration) } - if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < time.Millisecond { - return nil, fmt.Errorf("Timeout in Healthcheck cannot be less than one millisecond") + if config.Healthcheck.Timeout != 0 && config.Healthcheck.Timeout < containertypes.MinimumDuration { + 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 && config.Healthcheck.StartPeriod < time.Millisecond { - return nil, fmt.Errorf("StartPeriod in Healthcheck cannot be less than one millisecond") + if config.Healthcheck.StartPeriod != 0 && config.Healthcheck.StartPeriod < containertypes.MinimumDuration { + return nil, fmt.Errorf("StartPeriod in Healthcheck cannot be less than %s", containertypes.MinimumDuration) } } } diff --git a/components/engine/docs/api/v1.24.md b/components/engine/docs/api/v1.24.md index b7586c93ea..4adfcab949 100644 --- a/components/engine/docs/api/v1.24.md +++ b/components/engine/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. - - **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. + - **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 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 diff --git a/components/engine/integration-cli/docker_api_create_test.go b/components/engine/integration-cli/docker_api_create_test.go index 7328f4d068..e404b6cf58 100644 --- a/components/engine/integration-cli/docker_api_create_test.go +++ b/components/engine/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), - "Timeout": time.Duration(1000000000), + "Interval": -10 * time.Millisecond, + "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), - "Timeout": time.Duration(1000000000), + "Interval": 500 * time.Microsecond, + "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), - "Timeout": time.Duration(-100000000), + "Interval": time.Second, + "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), - "Timeout": time.Duration(1000000000), + "Interval": time.Second, + "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) }