From 87e916a171eca346506a735d7b604150e5ecbad0 Mon Sep 17 00:00:00 2001 From: Cezar Sa Espinola Date: Thu, 13 Oct 2016 15:28:32 -0300 Subject: [PATCH] Add --health-* commands to service create and update A HealthConfig entry was added to the ContainerSpec associated with the service being created or updated. Signed-off-by: Cezar Sa Espinola --- command/service/opts.go | 80 ++++++++++++++++++++++++++++++++++ command/service/opts_test.go | 49 +++++++++++++++++++++ command/service/update.go | 50 +++++++++++++++++++++ command/service/update_test.go | 79 +++++++++++++++++++++++++++++++++ 4 files changed, 258 insertions(+) diff --git a/command/service/opts.go b/command/service/opts.go index 5fdc56de05..0c91360c6e 100644 --- a/command/service/opts.go +++ b/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" ) diff --git a/command/service/opts_test.go b/command/service/opts_test.go index 8ef3cacb45..52016cbfc5 100644 --- a/command/service/opts_test.go +++ b/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") +} diff --git a/command/service/update.go b/command/service/update.go index e1f7cad66b..356c27a5c6 100644 --- a/command/service/update.go +++ b/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 +} diff --git a/command/service/update_test.go b/command/service/update_test.go index 6e68e977ac..731358753e 100644 --- a/command/service/update_test.go +++ b/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) + } + } + } +}