Merge pull request #27369 from cezarsa/hc

Add --health-* flags to service create and update

command/service/opts.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/opts"
@@ -68,6 +69,25 @@ func (c *nanoCPUs) Value() int64 {
 	return int64(*c)
 }
 
+// PositiveDurationOpt is an option type for time.Duration that uses a pointer.
+// It bahave similarly to DurationOpt but only allows positive duration values.
+type PositiveDurationOpt struct {
+	DurationOpt
+}
+
+// Set a new value on the option. Setting a negative duration value will cause
+// an error to be returned.
+func (d *PositiveDurationOpt) Set(s string) error {
+	err := d.DurationOpt.Set(s)
+	if err != nil {
+		return err
+	}
+	if *d.DurationOpt.value < 0 {
+		return fmt.Errorf("duration cannot be negative")
+	}
+	return nil
+}
+
 // DurationOpt is an option type for time.Duration that uses a pointer. This
 // allows us to get nil values outside, instead of defaulting to 0
 type DurationOpt struct {
@@ -377,6 +397,47 @@ func (ldo *logDriverOptions) toLogDriver() *swarm.Driver {
 	}
 }
 
+type healthCheckOptions struct {
+	cmd           string
+	interval      PositiveDurationOpt
+	timeout       PositiveDurationOpt
+	retries       int
+	noHealthcheck bool
+}
+
+func (opts *healthCheckOptions) toHealthConfig() (*container.HealthConfig, error) {
+	var healthConfig *container.HealthConfig
+	haveHealthSettings := opts.cmd != "" ||
+		opts.interval.Value() != nil ||
+		opts.timeout.Value() != nil ||
+		opts.retries != 0
+	if opts.noHealthcheck {
+		if haveHealthSettings {
+			return nil, fmt.Errorf("--%s conflicts with --health-* options", flagNoHealthcheck)
+		}
+		healthConfig = &container.HealthConfig{Test: []string{"NONE"}}
+	} else if haveHealthSettings {
+		var test []string
+		if opts.cmd != "" {
+			test = []string{"CMD-SHELL", opts.cmd}
+		}
+		var interval, timeout time.Duration
+		if ptr := opts.interval.Value(); ptr != nil {
+			interval = *ptr
+		}
+		if ptr := opts.timeout.Value(); ptr != nil {
+			timeout = *ptr
+		}
+		healthConfig = &container.HealthConfig{
+			Test:     test,
+			Interval: interval,
+			Timeout:  timeout,
+			Retries:  opts.retries,
+		}
+	}
+	return healthConfig, nil
+}
+
 // ValidatePort validates a string is in the expected format for a port definition
 func ValidatePort(value string) (string, error) {
 	portMappings, err := nat.ParsePortSpec(value)
@@ -416,6 +477,8 @@ type serviceOptions struct {
 	registryAuth bool
 
 	logDriver logDriverOptions
+
+	healthcheck healthCheckOptions
 }
 
 func newServiceOptions() *serviceOptions {
@@ -490,6 +553,12 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 		EndpointSpec: opts.endpoint.ToEndpointSpec(),
 	}
 
+	healthConfig, err := opts.healthcheck.toHealthConfig()
+	if err != nil {
+		return service, err
+	}
+	service.TaskTemplate.ContainerSpec.Healthcheck = healthConfig
+
 	switch opts.mode {
 	case "global":
 		if opts.replicas.Value() != nil {
@@ -541,6 +610,12 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
 
 	flags.StringVar(&opts.logDriver.name, flagLogDriver, "", "Logging driver for service")
 	flags.Var(&opts.logDriver.opts, flagLogOpt, "Logging driver options")
+
+	flags.StringVar(&opts.healthcheck.cmd, flagHealthCmd, "", "Command to run to check health")
+	flags.Var(&opts.healthcheck.interval, flagHealthInterval, "Time between running the check")
+	flags.Var(&opts.healthcheck.timeout, flagHealthTimeout, "Maximum time to allow one check to run")
+	flags.IntVar(&opts.healthcheck.retries, flagHealthRetries, 0, "Consecutive failures needed to report unhealthy")
+	flags.BoolVar(&opts.healthcheck.noHealthcheck, flagNoHealthcheck, false, "Disable any container-specified HEALTHCHECK")
 }
 
 const (
@@ -589,4 +664,9 @@ const (
 	flagRegistryAuth          = "with-registry-auth"
 	flagLogDriver             = "log-driver"
 	flagLogOpt                = "log-opt"
+	flagHealthCmd             = "health-cmd"
+	flagHealthInterval        = "health-interval"
+	flagHealthRetries         = "health-retries"
+	flagHealthTimeout         = "health-timeout"
+	flagNoHealthcheck         = "no-healthcheck"
 )

command/service/opts_test.go

@@ -1,9 +1,11 @@
 package service
 
 import (
+	"reflect"
 	"testing"
 	"time"
 
+	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/pkg/testutil/assert"
 )
@@ -40,6 +42,15 @@ func TestDurationOptSetAndValue(t *testing.T) {
 	var duration DurationOpt
 	assert.NilError(t, duration.Set("300s"))
 	assert.Equal(t, *duration.Value(), time.Duration(300*10e8))
+	assert.NilError(t, duration.Set("-300s"))
+	assert.Equal(t, *duration.Value(), time.Duration(-300*10e8))
+}
+
+func TestPositiveDurationOptSetAndValue(t *testing.T) {
+	var duration PositiveDurationOpt
+	assert.NilError(t, duration.Set("300s"))
+	assert.Equal(t, *duration.Value(), time.Duration(300*10e8))
+	assert.Error(t, duration.Set("-300s"), "cannot be negative")
 }
 
 func TestUint64OptString(t *testing.T) {
@@ -201,3 +212,41 @@ func TestMountOptTypeConflict(t *testing.T) {
 	assert.Error(t, m.Set("type=bind,target=/foo,source=/foo,volume-nocopy=true"), "cannot mix")
 	assert.Error(t, m.Set("type=volume,target=/foo,source=/foo,bind-propagation=rprivate"), "cannot mix")
 }
+
+func TestHealthCheckOptionsToHealthConfig(t *testing.T) {
+	dur := time.Second
+	opt := healthCheckOptions{
+		cmd:      "curl",
+		interval: PositiveDurationOpt{DurationOpt{value: &dur}},
+		timeout:  PositiveDurationOpt{DurationOpt{value: &dur}},
+		retries:  10,
+	}
+	config, err := opt.toHealthConfig()
+	assert.NilError(t, err)
+	assert.Equal(t, reflect.DeepEqual(config, &container.HealthConfig{
+		Test:     []string{"CMD-SHELL", "curl"},
+		Interval: time.Second,
+		Timeout:  time.Second,
+		Retries:  10,
+	}), true)
+}
+
+func TestHealthCheckOptionsToHealthConfigNoHealthcheck(t *testing.T) {
+	opt := healthCheckOptions{
+		noHealthcheck: true,
+	}
+	config, err := opt.toHealthConfig()
+	assert.NilError(t, err)
+	assert.Equal(t, reflect.DeepEqual(config, &container.HealthConfig{
+		Test: []string{"NONE"},
+	}), true)
+}
+
+func TestHealthCheckOptionsToHealthConfigConflict(t *testing.T) {
+	opt := healthCheckOptions{
+		cmd:           "curl",
+		noHealthcheck: true,
+	}
+	_, err := opt.toHealthConfig()
+	assert.Error(t, err, "--no-healthcheck conflicts with --health-* options")
+}

command/service/update.go

@@ -9,6 +9,7 @@ import (
 	"golang.org/x/net/context"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/cli"
@@ -266,6 +267,10 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
 		spec.TaskTemplate.ForceUpdate++
 	}
 
+	if err := updateHealthcheck(flags, cspec); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -537,3 +542,48 @@ func updateLogDriver(flags *pflag.FlagSet, taskTemplate *swarm.TaskSpec) error {
 
 	return nil
 }
+
+func updateHealthcheck(flags *pflag.FlagSet, containerSpec *swarm.ContainerSpec) error {
+	if !anyChanged(flags, flagNoHealthcheck, flagHealthCmd, flagHealthInterval, flagHealthRetries, flagHealthTimeout) {
+		return nil
+	}
+	if containerSpec.Healthcheck == nil {
+		containerSpec.Healthcheck = &container.HealthConfig{}
+	}
+	noHealthcheck, err := flags.GetBool(flagNoHealthcheck)
+	if err != nil {
+		return err
+	}
+	if noHealthcheck {
+		if !anyChanged(flags, flagHealthCmd, flagHealthInterval, flagHealthRetries, flagHealthTimeout) {
+			containerSpec.Healthcheck = &container.HealthConfig{
+				Test: []string{"NONE"},
+			}
+			return nil
+		}
+		return fmt.Errorf("--%s conflicts with --health-* options", flagNoHealthcheck)
+	}
+	if len(containerSpec.Healthcheck.Test) > 0 && containerSpec.Healthcheck.Test[0] == "NONE" {
+		containerSpec.Healthcheck.Test = nil
+	}
+	if flags.Changed(flagHealthInterval) {
+		val := *flags.Lookup(flagHealthInterval).Value.(*PositiveDurationOpt).Value()
+		containerSpec.Healthcheck.Interval = val
+	}
+	if flags.Changed(flagHealthTimeout) {
+		val := *flags.Lookup(flagHealthTimeout).Value.(*PositiveDurationOpt).Value()
+		containerSpec.Healthcheck.Timeout = val
+	}
+	if flags.Changed(flagHealthRetries) {
+		containerSpec.Healthcheck.Retries, _ = flags.GetInt(flagHealthRetries)
+	}
+	if flags.Changed(flagHealthCmd) {
+		cmd, _ := flags.GetString(flagHealthCmd)
+		if cmd != "" {
+			containerSpec.Healthcheck.Test = []string{"CMD-SHELL", cmd}
+		} else {
+			containerSpec.Healthcheck.Test = nil
+		}
+	}
+	return nil
+}

command/service/update_test.go

@@ -1,9 +1,12 @@
 package service
 
 import (
+	"reflect"
 	"sort"
 	"testing"
+	"time"
 
+	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/pkg/testutil/assert"
@@ -196,3 +199,79 @@ func TestUpdatePortsConflictingFlags(t *testing.T) {
 	err := updatePorts(flags, &portConfigs)
 	assert.Error(t, err, "conflicting port mapping")
 }
+
+func TestUpdateHealthcheckTable(t *testing.T) {
+	type test struct {
+		flags    [][2]string
+		initial  *container.HealthConfig
+		expected *container.HealthConfig
+		err      string
+	}
+	testCases := []test{
+		{
+			flags:    [][2]string{{"no-healthcheck", "true"}},
+			initial:  &container.HealthConfig{Test: []string{"CMD-SHELL", "cmd1"}, Retries: 10},
+			expected: &container.HealthConfig{Test: []string{"NONE"}},
+		},
+		{
+			flags:    [][2]string{{"health-cmd", "cmd1"}},
+			initial:  &container.HealthConfig{Test: []string{"NONE"}},
+			expected: &container.HealthConfig{Test: []string{"CMD-SHELL", "cmd1"}},
+		},
+		{
+			flags:    [][2]string{{"health-retries", "10"}},
+			initial:  &container.HealthConfig{Test: []string{"NONE"}},
+			expected: &container.HealthConfig{Retries: 10},
+		},
+		{
+			flags:    [][2]string{{"health-retries", "10"}},
+			initial:  &container.HealthConfig{Test: []string{"CMD", "cmd1"}},
+			expected: &container.HealthConfig{Test: []string{"CMD", "cmd1"}, Retries: 10},
+		},
+		{
+			flags:    [][2]string{{"health-interval", "1m"}},
+			initial:  &container.HealthConfig{Test: []string{"CMD", "cmd1"}},
+			expected: &container.HealthConfig{Test: []string{"CMD", "cmd1"}, Interval: time.Minute},
+		},
+		{
+			flags:    [][2]string{{"health-cmd", ""}},
+			initial:  &container.HealthConfig{Test: []string{"CMD", "cmd1"}, Retries: 10},
+			expected: &container.HealthConfig{Retries: 10},
+		},
+		{
+			flags:    [][2]string{{"health-retries", "0"}},
+			initial:  &container.HealthConfig{Test: []string{"CMD", "cmd1"}, Retries: 10},
+			expected: &container.HealthConfig{Test: []string{"CMD", "cmd1"}},
+		},
+		{
+			flags: [][2]string{{"health-cmd", "cmd1"}, {"no-healthcheck", "true"}},
+			err:   "--no-healthcheck conflicts with --health-* options",
+		},
+		{
+			flags: [][2]string{{"health-interval", "10m"}, {"no-healthcheck", "true"}},
+			err:   "--no-healthcheck conflicts with --health-* options",
+		},
+		{
+			flags: [][2]string{{"health-timeout", "1m"}, {"no-healthcheck", "true"}},
+			err:   "--no-healthcheck conflicts with --health-* options",
+		},
+	}
+	for i, c := range testCases {
+		flags := newUpdateCommand(nil).Flags()
+		for _, flag := range c.flags {
+			flags.Set(flag[0], flag[1])
+		}
+		cspec := &swarm.ContainerSpec{
+			Healthcheck: c.initial,
+		}
+		err := updateHealthcheck(flags, cspec)
+		if c.err != "" {
+			assert.Error(t, err, c.err)
+		} else {
+			assert.NilError(t, err)
+			if !reflect.DeepEqual(cspec.Healthcheck, c.expected) {
+				t.Errorf("incorrect result for test %d, expected health config:\n\t%#v\ngot:\n\t%#v", i, c.expected, cspec.Healthcheck)
+			}
+		}
+	}
+}