diff --git a/agent/agent_test.go b/agent/agent_test.go index 107203ccd8..d6fafed3c0 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -2003,6 +2003,9 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { Name: "memory check", Status: api.HealthCritical, Notes: "my cool notes", + Definition: structs.HealthCheckDefinition{ + Interval: 30 * time.Second, + }, } if got, want := result, expected; !verify.Values(t, "", got, want) { t.FailNow() diff --git a/agent/http.go b/agent/http.go index 2f742eaa05..e289374e36 100644 --- a/agent/http.go +++ b/agent/http.go @@ -9,6 +9,7 @@ import ( "net/http/pprof" "net/url" "os" + "reflect" "regexp" "strconv" "strings" @@ -20,6 +21,7 @@ import ( "github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/api" cleanhttp "github.com/hashicorp/go-cleanhttp" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" @@ -510,8 +512,11 @@ func decodeBody(req *http.Request, out interface{}, cb func(interface{}) error) } decodeConf := &mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeDurationHookFunc(), - Result: &out, + DecodeHook: mapstructure.ComposeDecodeHookFunc( + mapstructure.StringToTimeDurationHookFunc(), + stringToReadableDurationFunc(), + ), + Result: &out, } decoder, err := mapstructure.NewDecoder(decodeConf) @@ -522,6 +527,32 @@ func decodeBody(req *http.Request, out interface{}, cb func(interface{}) error) return decoder.Decode(raw) } +// stringToReadableDurationFunc is a mapstructure hook for decoding a string +// into an api.ReadableDuration for backwards compatibility. +func stringToReadableDurationFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + var v api.ReadableDuration + if t != reflect.TypeOf(v) { + return data, nil + } + + switch { + case f.Kind() == reflect.String: + if dur, err := time.ParseDuration(data.(string)); err != nil { + return nil, err + } else { + v = api.ReadableDuration(dur) + } + return v, nil + default: + return data, nil + } + } +} + // setTranslateAddr is used to set the address translation header. This is only // present if the feature is active. func setTranslateAddr(resp http.ResponseWriter, active bool) { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 1b569fb16d..349e1ae426 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -920,9 +920,9 @@ type HealthCheckDefinition struct { func (d *HealthCheckDefinition) MarshalJSON() ([]byte, error) { type Alias HealthCheckDefinition exported := &struct { - Interval string - Timeout string - DeregisterCriticalServiceAfter string + Interval string `json:",omitempty"` + Timeout string `json:",omitempty"` + DeregisterCriticalServiceAfter string `json:",omitempty"` *Alias }{ Interval: d.Interval.String(), diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index 0c943b9037..e407f758c1 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -640,6 +640,15 @@ func TestStructs_HealthCheck_IsSame(t *testing.T) { checkStringField(&other.ServiceName) } +func TestStructs_HealthCheck_Marshalling(t *testing.T) { + d := &HealthCheckDefinition{} + buf, err := d.MarshalJSON() + require.NoError(t, err) + require.NotContains(t, string(buf), `"Interval":""`) + require.NotContains(t, string(buf), `"Timeout":""`) + require.NotContains(t, string(buf), `"DeregisterCriticalServiceAfter":""`) +} + func TestStructs_HealthCheck_Clone(t *testing.T) { hc := &HealthCheck{ Node: "node1", diff --git a/agent/txn_endpoint.go b/agent/txn_endpoint.go index d11958ebd8..b471c8a954 100644 --- a/agent/txn_endpoint.go +++ b/agent/txn_endpoint.go @@ -239,9 +239,9 @@ func (s *HTTPServer) convertOps(resp http.ResponseWriter, req *http.Request) (st Header: check.Definition.Header, Method: check.Definition.Method, TCP: check.Definition.TCP, - Interval: check.Definition.Interval, - Timeout: check.Definition.Timeout, - DeregisterCriticalServiceAfter: check.Definition.DeregisterCriticalServiceAfter, + Interval: check.Definition.IntervalDuration, + Timeout: check.Definition.TimeoutDuration, + DeregisterCriticalServiceAfter: check.Definition.DeregisterCriticalServiceAfterDuration, }, RaftIndex: structs.RaftIndex{ ModifyIndex: check.ModifyIndex, diff --git a/api/health.go b/api/health.go index b3f6b41cf8..9faf6b665a 100644 --- a/api/health.go +++ b/api/health.go @@ -46,19 +46,24 @@ type HealthCheck struct { // HealthCheckDefinition is used to store the details about // a health check's execution. type HealthCheckDefinition struct { - HTTP string - Header map[string][]string - Method string - TLSSkipVerify bool - TCP string - Interval time.Duration - Timeout time.Duration - DeregisterCriticalServiceAfter time.Duration + HTTP string + Header map[string][]string + Method string + TLSSkipVerify bool + TCP string + IntervalDuration time.Duration `json:"-"` + TimeoutDuration time.Duration `json:"-"` + DeregisterCriticalServiceAfterDuration time.Duration `json:"-"` + + // DEPRECATED in Consul 1.4.1. Use the above time.Duration fields instead. + Interval ReadableDuration + Timeout ReadableDuration + DeregisterCriticalServiceAfter ReadableDuration } func (d *HealthCheckDefinition) MarshalJSON() ([]byte, error) { type Alias HealthCheckDefinition - return json.Marshal(&struct { + out := &struct { Interval string Timeout string DeregisterCriticalServiceAfter string @@ -68,7 +73,25 @@ func (d *HealthCheckDefinition) MarshalJSON() ([]byte, error) { Timeout: d.Timeout.String(), DeregisterCriticalServiceAfter: d.DeregisterCriticalServiceAfter.String(), Alias: (*Alias)(d), - }) + } + + if d.IntervalDuration != 0 { + out.Interval = d.IntervalDuration.String() + } else if d.Interval != 0 { + out.Interval = d.Interval.String() + } + if d.TimeoutDuration != 0 { + out.Timeout = d.TimeoutDuration.String() + } else if d.Timeout != 0 { + out.Timeout = d.Timeout.String() + } + if d.DeregisterCriticalServiceAfterDuration != 0 { + out.DeregisterCriticalServiceAfter = d.DeregisterCriticalServiceAfterDuration.String() + } else if d.DeregisterCriticalServiceAfter != 0 { + out.DeregisterCriticalServiceAfter = d.DeregisterCriticalServiceAfter.String() + } + + return json.Marshal(out) } func (d *HealthCheckDefinition) UnmarshalJSON(data []byte) error { @@ -84,21 +107,26 @@ func (d *HealthCheckDefinition) UnmarshalJSON(data []byte) error { if err := json.Unmarshal(data, &aux); err != nil { return err } + + // Parse the values into both the time.Duration and old ReadableDuration fields. var err error if aux.Interval != "" { - if d.Interval, err = time.ParseDuration(aux.Interval); err != nil { + if d.IntervalDuration, err = time.ParseDuration(aux.Interval); err != nil { return err } + d.Interval = ReadableDuration(d.IntervalDuration) } if aux.Timeout != "" { - if d.Timeout, err = time.ParseDuration(aux.Timeout); err != nil { + if d.TimeoutDuration, err = time.ParseDuration(aux.Timeout); err != nil { return err } + d.Timeout = ReadableDuration(d.TimeoutDuration) } if aux.DeregisterCriticalServiceAfter != "" { - if d.DeregisterCriticalServiceAfter, err = time.ParseDuration(aux.DeregisterCriticalServiceAfter); err != nil { + if d.DeregisterCriticalServiceAfterDuration, err = time.ParseDuration(aux.DeregisterCriticalServiceAfter); err != nil { return err } + d.DeregisterCriticalServiceAfter = ReadableDuration(d.DeregisterCriticalServiceAfterDuration) } return nil } diff --git a/api/txn_test.go b/api/txn_test.go index c164ebe92b..ecfdd6fcb9 100644 --- a/api/txn_test.go +++ b/api/txn_test.go @@ -33,12 +33,26 @@ func TestAPI_ClientTxn(t *testing.T) { ID: "foo1", Service: "foo", }, - Check: &AgentCheck{ - CheckID: "bar", - Status: "critical", - Definition: HealthCheckDefinition{ - TCP: "1.1.1.1", - Interval: 5 * time.Second, + Checks: HealthChecks{ + { + CheckID: "bar", + Status: "critical", + Definition: HealthCheckDefinition{ + TCP: "1.1.1.1", + IntervalDuration: 5 * time.Second, + TimeoutDuration: 10 * time.Second, + DeregisterCriticalServiceAfterDuration: 20 * time.Second, + }, + }, + { + CheckID: "baz", + Status: "passing", + Definition: HealthCheckDefinition{ + TCP: "2.2.2.2", + Interval: ReadableDuration(40 * time.Second), + Timeout: ReadableDuration(80 * time.Second), + DeregisterCriticalServiceAfter: ReadableDuration(160 * time.Second), + }, }, }, } @@ -93,6 +107,12 @@ func TestAPI_ClientTxn(t *testing.T) { Check: HealthCheck{Node: "foo", CheckID: "bar"}, }, }, + &TxnOp{ + Check: &CheckTxnOp{ + Verb: CheckGet, + Check: HealthCheck{Node: "foo", CheckID: "baz"}, + }, + }, } ok, ret, _, err := txn.Txn(ops, nil) if err != nil { @@ -119,7 +139,7 @@ func TestAPI_ClientTxn(t *testing.T) { t.Fatalf("transaction failure") } - if ret == nil || len(ret.Errors) != 0 || len(ret.Results) != 5 { + if ret == nil || len(ret.Errors) != 0 || len(ret.Results) != 6 { t.Fatalf("bad: %v", ret) } expected := TxnResults{ @@ -165,8 +185,31 @@ func TestAPI_ClientTxn(t *testing.T) { CheckID: "bar", Status: "critical", Definition: HealthCheckDefinition{ - TCP: "1.1.1.1", - Interval: 5 * time.Second, + TCP: "1.1.1.1", + Interval: ReadableDuration(5 * time.Second), + IntervalDuration: 5 * time.Second, + Timeout: ReadableDuration(10 * time.Second), + TimeoutDuration: 10 * time.Second, + DeregisterCriticalServiceAfter: ReadableDuration(20 * time.Second), + DeregisterCriticalServiceAfterDuration: 20 * time.Second, + }, + CreateIndex: ret.Results[4].Check.CreateIndex, + ModifyIndex: ret.Results[4].Check.CreateIndex, + }, + }, + &TxnResult{ + Check: &HealthCheck{ + Node: "foo", + CheckID: "baz", + Status: "passing", + Definition: HealthCheckDefinition{ + TCP: "2.2.2.2", + Interval: ReadableDuration(40 * time.Second), + IntervalDuration: 40 * time.Second, + Timeout: ReadableDuration(80 * time.Second), + TimeoutDuration: 80 * time.Second, + DeregisterCriticalServiceAfter: ReadableDuration(160 * time.Second), + DeregisterCriticalServiceAfterDuration: 160 * time.Second, }, CreateIndex: ret.Results[4].Check.CreateIndex, ModifyIndex: ret.Results[4].Check.CreateIndex,