From 7bd2611789e6898576f7229255c238f7c1129293 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 --- api/types/swarm/container.go | 24 +++--- cli/command/service/opts.go | 80 +++++++++++++++++++ cli/command/service/opts_test.go | 49 ++++++++++++ cli/command/service/update.go | 50 ++++++++++++ cli/command/service/update_test.go | 79 ++++++++++++++++++ contrib/completion/bash/docker | 5 ++ contrib/completion/zsh/_docker | 5 ++ daemon/cluster/convert/container.go | 30 +++++++ .../cluster/executor/container/container.go | 29 +++++-- docs/reference/commandline/service_create.md | 5 ++ docs/reference/commandline/service_update.md | 5 ++ 11 files changed, 344 insertions(+), 17 deletions(-) diff --git a/api/types/swarm/container.go b/api/types/swarm/container.go index 4a84f2e53f..7ea87caa38 100644 --- a/api/types/swarm/container.go +++ b/api/types/swarm/container.go @@ -3,20 +3,22 @@ package swarm import ( "time" + "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/mount" ) // ContainerSpec represents the spec of a container. type ContainerSpec struct { - Image string `json:",omitempty"` - Labels map[string]string `json:",omitempty"` - Command []string `json:",omitempty"` - Args []string `json:",omitempty"` - Env []string `json:",omitempty"` - Dir string `json:",omitempty"` - User string `json:",omitempty"` - Groups []string `json:",omitempty"` - TTY bool `json:",omitempty"` - Mounts []mount.Mount `json:",omitempty"` - StopGracePeriod *time.Duration `json:",omitempty"` + Image string `json:",omitempty"` + Labels map[string]string `json:",omitempty"` + Command []string `json:",omitempty"` + Args []string `json:",omitempty"` + Env []string `json:",omitempty"` + Dir string `json:",omitempty"` + User string `json:",omitempty"` + Groups []string `json:",omitempty"` + TTY bool `json:",omitempty"` + Mounts []mount.Mount `json:",omitempty"` + StopGracePeriod *time.Duration `json:",omitempty"` + Healthcheck *container.HealthConfig `json:",omitempty"` } diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 5fdc56de05..0c91360c6e 100644 --- a/cli/command/service/opts.go +++ b/cli/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/cli/command/service/opts_test.go b/cli/command/service/opts_test.go index 8ef3cacb45..52016cbfc5 100644 --- a/cli/command/service/opts_test.go +++ b/cli/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/cli/command/service/update.go b/cli/command/service/update.go index e1f7cad66b..356c27a5c6 100644 --- a/cli/command/service/update.go +++ b/cli/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/cli/command/service/update_test.go b/cli/command/service/update_test.go index 6e68e977ac..731358753e 100644 --- a/cli/command/service/update_test.go +++ b/cli/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) + } + } + } +} diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker index b948a63916..84783295d3 100644 --- a/contrib/completion/bash/docker +++ b/contrib/completion/bash/docker @@ -2570,6 +2570,10 @@ _docker_service_update() { --env -e --force --group-add + --health-cmd + --health-interval + --health-retries + --health-timeout --label -l --limit-cpu --limit-memory @@ -2577,6 +2581,7 @@ _docker_service_update() { --log-opt --mount --network + --no-healthcheck --publish -p --replicas --reserve-cpu diff --git a/contrib/completion/zsh/_docker b/contrib/completion/zsh/_docker index 99ef3e7480..0f32e32bee 100644 --- a/contrib/completion/zsh/_docker +++ b/contrib/completion/zsh/_docker @@ -1089,6 +1089,10 @@ __docker_service_subcommand() { "($help)--endpoint-mode=[Placement constraints]:mode:(dnsrr vip)" "($help)*"{-e=,--env=}"[Set environment variables]:env: " "($help)*--group-add=[Add additional user groups to the container]:group:_groups" + "($help)--health-cmd=[Command to run to check health]:command: " + "($help)--health-interval=[Time between running the check]:time: " + "($help)--health-retries=[Consecutive failures needed to report unhealthy]:retries:(1 2 3 4 5)" + "($help)--health-timeout=[Maximum time to allow one check to run]:time: " "($help)*--label=[Service labels]:label: " "($help)--limit-cpu=[Limit CPUs]:value: " "($help)--limit-memory=[Limit Memory]:value: " @@ -1096,6 +1100,7 @@ __docker_service_subcommand() { "($help)*--log-opt=[Logging driver options]:log driver options:__docker_log_options" "($help)*--mount=[Attach a mount to the service]:mount: " "($help)*--network=[Network attachments]:network: " + "($help)--no-healthcheck[Disable any container-specified HEALTHCHECK]" "($help)*"{-p=,--publish=}"[Publish a port as a node port]:port: " "($help)--replicas=[Number of tasks]:replicas: " "($help)--reserve-cpu=[Reserve CPUs]:value: " diff --git a/daemon/cluster/convert/container.go b/daemon/cluster/convert/container.go index 7cac7960b6..36c4aed52f 100644 --- a/daemon/cluster/convert/container.go +++ b/daemon/cluster/convert/container.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + container "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" types "github.com/docker/docker/api/types/swarm" swarmapi "github.com/docker/swarmkit/api" @@ -56,6 +57,11 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec { grace, _ := ptypes.Duration(c.StopGracePeriod) containerSpec.StopGracePeriod = &grace } + + if c.Healthcheck != nil { + containerSpec.Healthcheck = healthConfigFromGRPC(c.Healthcheck) + } + return containerSpec } @@ -115,5 +121,29 @@ func containerToGRPC(c types.ContainerSpec) (*swarmapi.ContainerSpec, error) { containerSpec.Mounts = append(containerSpec.Mounts, mount) } + if c.Healthcheck != nil { + containerSpec.Healthcheck = healthConfigToGRPC(c.Healthcheck) + } + return containerSpec, nil } + +func healthConfigFromGRPC(h *swarmapi.HealthConfig) *container.HealthConfig { + interval, _ := ptypes.Duration(h.Interval) + timeout, _ := ptypes.Duration(h.Timeout) + return &container.HealthConfig{ + Test: h.Test, + Interval: interval, + Timeout: timeout, + Retries: int(h.Retries), + } +} + +func healthConfigToGRPC(h *container.HealthConfig) *swarmapi.HealthConfig { + return &swarmapi.HealthConfig{ + Test: h.Test, + Interval: ptypes.DurationProto(h.Interval), + Timeout: ptypes.DurationProto(h.Timeout), + Retries: int32(h.Retries), + } +} diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index fffa7fab62..7b5203715f 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -18,6 +18,7 @@ import ( "github.com/docker/docker/reference" "github.com/docker/swarmkit/agent/exec" "github.com/docker/swarmkit/api" + "github.com/docker/swarmkit/protobuf/ptypes" ) const ( @@ -124,12 +125,13 @@ func (c *containerConfig) image() string { func (c *containerConfig) config() *enginecontainer.Config { config := &enginecontainer.Config{ - Labels: c.labels(), - User: c.spec().User, - Env: c.spec().Env, - WorkingDir: c.spec().Dir, - Image: c.image(), - Volumes: c.volumes(), + Labels: c.labels(), + User: c.spec().User, + Env: c.spec().Env, + WorkingDir: c.spec().Dir, + Image: c.image(), + Volumes: c.volumes(), + Healthcheck: c.healthcheck(), } if len(c.spec().Command) > 0 { @@ -224,6 +226,21 @@ func (c *containerConfig) binds() []string { return r } +func (c *containerConfig) healthcheck() *enginecontainer.HealthConfig { + hcSpec := c.spec().Healthcheck + if hcSpec == nil { + return nil + } + interval, _ := ptypes.Duration(hcSpec.Interval) + timeout, _ := ptypes.Duration(hcSpec.Timeout) + return &enginecontainer.HealthConfig{ + Test: hcSpec.Test, + Interval: interval, + Timeout: timeout, + Retries: int(hcSpec.Retries), + } +} + func getMountMask(m *api.Mount) string { var maskOpts []string if m.ReadOnly { diff --git a/docs/reference/commandline/service_create.md b/docs/reference/commandline/service_create.md index 3feaf6435d..2c023f0c51 100644 --- a/docs/reference/commandline/service_create.md +++ b/docs/reference/commandline/service_create.md @@ -27,6 +27,10 @@ Options: -e, --env value Set environment variables (default []) --env-file value Read in a file of environment variables (default []) --group-add value Add additional user groups to the container (default []) + --health-cmd string Command to run to check health + --health-interval duration Time between running the check + --health-retries int Consecutive failures needed to report unhealthy + --health-timeout duration Maximum time to allow one check to run --help Print usage -l, --label value Service labels (default []) --limit-cpu value Limit CPUs (default 0.000) @@ -37,6 +41,7 @@ Options: --mount value Attach a mount to the service --name string Service name --network value Network attachments (default []) + --no-healthcheck Disable any container-specified HEALTHCHECK -p, --publish value Publish a port as a node port (default []) --replicas value Number of tasks (default none) --reserve-cpu value Reserve CPUs (default 0.000) diff --git a/docs/reference/commandline/service_update.md b/docs/reference/commandline/service_update.md index c30e5e973e..58ffe6e442 100644 --- a/docs/reference/commandline/service_update.md +++ b/docs/reference/commandline/service_update.md @@ -32,6 +32,10 @@ Options: --force Force update even if no changes require it --group-add value Add additional user groups to the container (default []) --group-rm value Remove previously added user groups from the container (default []) + --health-cmd string Command to run to check health + --health-interval duration Time between running the check + --health-retries int Consecutive failures needed to report unhealthy + --health-timeout duration Maximum time to allow one check to run --help Print usage --image string Service image tag --label-add value Add or update service labels (default []) @@ -42,6 +46,7 @@ Options: --log-opt value Logging driver options (default []) --mount-add value Add or update a mount on a service --mount-rm value Remove a mount by its target path (default []) + --no-healthcheck Disable any container-specified HEALTHCHECK --publish-add value Add or update a published port (default []) --publish-rm value Remove a published port by its target port (default []) --replicas value Number of tasks (default none)