From 0ef42f7bcbdde3b92dc9fade76e65d8f3a3d3ad7 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 11:49:44 +0100 Subject: [PATCH 1/4] Fix converting run health check options Signed-off-by: Guillaume Tardif --- cli/options/run/opts.go | 29 +++++++++++++++++------------ cli/options/run/opts_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/cli/options/run/opts.go b/cli/options/run/opts.go index eaca7a2e..bd6a8b2a 100644 --- a/cli/options/run/opts.go +++ b/cli/options/run/opts.go @@ -86,13 +86,6 @@ func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, erro envVars = append(envVars, vars...) } - var healthCmd []string - var healthInterval types.Duration - if len(r.HealthCmd) > 0 { - healthCmd = strings.Split(r.HealthCmd, " ") - healthInterval = types.Duration(r.HealthInterval) - } - return containers.ContainerConfig{ ID: r.Name, Image: image, @@ -106,14 +99,26 @@ func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, erro RestartPolicyCondition: restartPolicy, DomainName: r.DomainName, AutoRemove: r.Rm, - Healthcheck: containers.Healthcheck{ - Disable: len(healthCmd) == 0, - Test: healthCmd, - Interval: healthInterval, - }, + Healthcheck: r.toHealthcheck(), }, nil } +func (r *Opts) toHealthcheck() containers.Healthcheck { + var healthCmd []string + + if len(r.HealthCmd) > 0 { + healthCmd = strings.Split(r.HealthCmd, " ") + } + return containers.Healthcheck{ + Disable: len(healthCmd) == 0, + Test: healthCmd, + Interval: types.Duration(r.HealthInterval), + StartPeriod: types.Duration(r.HealthStartPeriod), + Timeout: types.Duration(r.HealthTimeout), + Retries: r.HealthRetries, + } +} + var restartPolicyMap = map[string]string{ "": containers.RestartPolicyNone, containers.RestartPolicyNone: containers.RestartPolicyNone, diff --git a/cli/options/run/opts_test.go b/cli/options/run/opts_test.go index 547d0968..639a71f9 100644 --- a/cli/options/run/opts_test.go +++ b/cli/options/run/opts_test.go @@ -20,7 +20,9 @@ import ( "errors" "regexp" "testing" + "time" + "github.com/compose-spec/compose-go/types" "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" @@ -227,3 +229,31 @@ func TestValidateRestartPolicy(t *testing.T) { assert.Equal(t, testCase.expected, result) } } + +func TestToHealthcheck(t *testing.T) { + testOpt := Opts{ + HealthCmd: "curl", + } + + assert.DeepEqual(t, testOpt.toHealthcheck(), containers.Healthcheck{ + Disable: false, + Test: []string{"curl"}, + }) + + testOpt = Opts{ + HealthCmd: "curl", + HealthRetries: 3, + HealthInterval: 5 * time.Second, + HealthTimeout: 2 * time.Second, + HealthStartPeriod: 10 * time.Second, + } + + assert.DeepEqual(t, testOpt.toHealthcheck(), containers.Healthcheck{ + Disable: false, + Test: []string{"curl"}, + Retries: 3, + Interval: types.Duration(5 * time.Second), + StartPeriod: types.Duration(10 * time.Second), + Timeout: types.Duration(2 * time.Second), + }) +} From 575307d8afef5b2253ffb5f0efe70e8e03d3712f Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 11:50:10 +0100 Subject: [PATCH 2/4] Fix run default value for health check retries Signed-off-by: Guillaume Tardif --- cli/cmd/run/run.go | 2 +- cli/cmd/run/testdata/run-help.golden | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/cmd/run/run.go b/cli/cmd/run/run.go index 591719f5..8bbeef81 100644 --- a/cli/cmd/run/run.go +++ b/cli/cmd/run/run.go @@ -62,7 +62,7 @@ func Command(contextType string) *cobra.Command { cmd.Flags().BoolVar(&opts.Rm, "rm", false, "Automatically remove the container when it exits") cmd.Flags().StringVar(&opts.HealthCmd, "health-cmd", "", "Command to run to check health") cmd.Flags().DurationVar(&opts.HealthInterval, "health-interval", time.Duration(0), "Time between running the check (ms|s|m|h) (default 0s)") - cmd.Flags().IntVar(&opts.HealthRetries, "health-retries", 10, "Consecutive failures needed to report unhealthy") + cmd.Flags().IntVar(&opts.HealthRetries, "health-retries", 0, "Consecutive failures needed to report unhealthy") cmd.Flags().DurationVar(&opts.HealthStartPeriod, "health-start-period", time.Duration(0), "Start period for the container to initialize before starting "+ "health-retries countdown (ms|s|m|h) (default 0s)") cmd.Flags().DurationVar(&opts.HealthTimeout, "health-timeout", time.Duration(0), "Maximum time to allow one check to run (ms|s|m|h) (default 0s)") diff --git a/cli/cmd/run/testdata/run-help.golden b/cli/cmd/run/testdata/run-help.golden index 496f6449..a9066ff3 100644 --- a/cli/cmd/run/testdata/run-help.golden +++ b/cli/cmd/run/testdata/run-help.golden @@ -11,7 +11,7 @@ Flags: --env-file stringArray Path to environment files to be translated as environment variables --health-cmd string Command to run to check health --health-interval duration Time between running the check (ms|s|m|h) (default 0s) - --health-retries int Consecutive failures needed to report unhealthy (default 10) + --health-retries int Consecutive failures needed to report unhealthy --health-start-period duration Start period for the container to initialize before starting health-retries countdown (ms|s|m|h) (default 0s) --health-timeout duration Maximum time to allow one check to run (ms|s|m|h) (default 0s) -l, --label stringArray Set meta data on a container From a6823c10920724d70a0833713edb9b811b610b01 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 11:50:25 +0100 Subject: [PATCH 3/4] Fix run health check doc Signed-off-by: Guillaume Tardif --- docs/aci-container-features.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/aci-container-features.md b/docs/aci-container-features.md index e40b8d4a..43f41d5a 100644 --- a/docs/aci-container-features.md +++ b/docs/aci-container-features.md @@ -71,7 +71,7 @@ When running a container with `docker run`, by default the command line stays at ## Healthchecks -A health check can be described using the flags prefixed by `--healthcheck`. This is translated into `LivenessProbe` for ACI. If the health check fails then the container is considered unhealthy and terminated. +A health check can be described using the flags prefixed by `--health-`. This is translated into `LivenessProbe` for ACI. If the health check fails then the container is considered unhealthy and terminated. In order for the container to be restarted automatically, the container needs to be run with a restart policy (set by the `--restart` flag) other than `no`. Note that the default restart policy if one isn't set is `no`. In order to restart automatically, the container also need to have a restart policy set with `--restart` (`docker run` defaults to no restart policy) From 3d18eda869c6b9f607a764c61a6a7a881620bcea Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 11:55:14 +0100 Subject: [PATCH 4/4] Do not specify successThreshold, from MSFT : defaults to 1 and cannot be changed, related to Kube healthchecks: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#probe-v1-core Signed-off-by: Guillaume Tardif --- aci/convert/convert.go | 5 ++--- aci/convert/convert_test.go | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/aci/convert/convert.go b/aci/convert/convert.go index accd50ad..8049e602 100644 --- a/aci/convert/convert.go +++ b/aci/convert/convert.go @@ -272,7 +272,6 @@ func (s serviceConfigAciHelper) getLivenessProbe() *containerinstance.ContainerP } if retries != nil && *retries > 0 { probe.FailureThreshold = retries - probe.SuccessThreshold = retries } return &probe } @@ -365,8 +364,8 @@ func ContainerGroupToContainer(containerID string, cg containerinstance.Containe if cc.LivenessProbe.PeriodSeconds != nil { healthcheck.Interval = types.Duration(int64(*cc.LivenessProbe.PeriodSeconds) * int64(time.Second)) } - if cc.LivenessProbe.SuccessThreshold != nil { - healthcheck.Retries = int(*cc.LivenessProbe.SuccessThreshold) + if cc.LivenessProbe.FailureThreshold != nil { + healthcheck.Retries = int(*cc.LivenessProbe.FailureThreshold) } if cc.LivenessProbe.TimeoutSeconds != nil { healthcheck.Timeout = types.Duration(int64(*cc.LivenessProbe.TimeoutSeconds) * int64(time.Second)) diff --git a/aci/convert/convert_test.go b/aci/convert/convert_test.go index 8c74a774..5032e301 100644 --- a/aci/convert/convert_test.go +++ b/aci/convert/convert_test.go @@ -98,7 +98,7 @@ func TestContainerGroupToContainer(t *testing.T) { }), }, PeriodSeconds: to.Int32Ptr(10), - SuccessThreshold: to.Int32Ptr(3), + FailureThreshold: to.Int32Ptr(3), InitialDelaySeconds: to.Int32Ptr(2), TimeoutSeconds: to.Int32Ptr(1), }, @@ -178,7 +178,7 @@ func TestHealthcheckTranslation(t *testing.T) { assert.NilError(t, err) assert.DeepEqual(t, (*group.Containers)[0].LivenessProbe.Exec.Command, to.StringSlicePtr(test)) assert.Equal(t, *(*group.Containers)[0].LivenessProbe.PeriodSeconds, int32(10)) - assert.Equal(t, *(*group.Containers)[0].LivenessProbe.SuccessThreshold, int32(42)) + assert.Assert(t, (*group.Containers)[0].LivenessProbe.SuccessThreshold == nil) assert.Equal(t, *(*group.Containers)[0].LivenessProbe.FailureThreshold, int32(42)) assert.Equal(t, *(*group.Containers)[0].LivenessProbe.InitialDelaySeconds, int32(2)) assert.Equal(t, *(*group.Containers)[0].LivenessProbe.TimeoutSeconds, int32(3))