Browse Source

Merge pull request #907 from docker/aci_healthcheck_threshold

Nicolas De loof 5 years ago
parent
commit
38d2dd9b41

+ 2 - 3
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))

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

+ 1 - 1
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)")

+ 1 - 1
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

+ 17 - 12
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,

+ 30 - 0
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),
+	})
+}

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