Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Oct 28, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
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 <cezarsa@gmail.com>
  • Loading branch information
cezarsa committed Oct 28, 2016
commit 7bd2611789e6898576f7229255c238f7c1129293
24 changes: 13 additions & 11 deletions api/types/swarm/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
80 changes: 80 additions & 0 deletions cli/command/service/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -416,6 +477,8 @@ type serviceOptions struct {
registryAuth bool

logDriver logDriverOptions

healthcheck healthCheckOptions
}

func newServiceOptions() *serviceOptions {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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"
)
49 changes: 49 additions & 0 deletions cli/command/service/opts_test.go
Original file line number Diff line number Diff line change
@@ -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"
)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}
50 changes: 50 additions & 0 deletions cli/command/service/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
79 changes: 79 additions & 0 deletions cli/command/service/update_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
}
}
}
}
Loading