Repository: docker/compose-cli
Pull request #907 (2020-11-13 12:54:35)

Aci healthcheck threshold
**What I did**
* Fix healthcheck run params
* do not set successThreshold, from MSFT, this is set to 1 by default and cannot be changed for liveness probe, see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#probe-v1-core

**Related issue**
healthcheck deployment sometimes fail 

<!-- optional tests
You can add a / mention to run tests executed by default only on main branch : 
* `test-aci` to run ACI E2E tests
* `test-ecs` to run ECS E2E tests
* `test-windows` to run tests & E2E tests on windows
-->
/test-aci

**(not mandatory) A picture of a cute animal, if possible in relation with what you did**
![](https://i.pinimg.com/originals/ed/86/de/ed86de024f2b0828f87c2c23885d6a2b.jpg)


Patch:
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/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/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/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)


Test patch:
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))
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
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),
+	})
+}