Re-add ReadableDuration types to health check definition

This is to fix the backwards-incompatible change made in 1.4.1 by
changing these fields to time.Duration.

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()

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,7 +512,10 @@ func decodeBody(req *http.Request, out interface{}, cb func(interface{}) error)
 	}
 
 	decodeConf := &mapstructure.DecoderConfig{
-		DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
+		DecodeHook: mapstructure.ComposeDecodeHookFunc(
+			mapstructure.StringToTimeDurationHookFunc(),
+			stringToReadableDurationFunc(),
+		),
 		Result: &out,
 	}
 
@@ -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) {

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,
+							Interval:                       check.Definition.IntervalDuration,
-							Timeout:                        check.Definition.Timeout,
+							Timeout:                        check.Definition.TimeoutDuration,
-							DeregisterCriticalServiceAfter: check.Definition.DeregisterCriticalServiceAfter,
+							DeregisterCriticalServiceAfter: check.Definition.DeregisterCriticalServiceAfterDuration,
 						},
 						RaftIndex: structs.RaftIndex{
 							ModifyIndex: check.ModifyIndex,

api/health.go

@@ -51,14 +51,19 @@ type HealthCheckDefinition struct {
 	Method                                 string
 	TLSSkipVerify                          bool
 	TCP                                    string
-	Interval                       time.Duration
+	IntervalDuration                       time.Duration `json:"-"`
-	Timeout                        time.Duration
+	TimeoutDuration                        time.Duration `json:"-"`
-	DeregisterCriticalServiceAfter time.Duration
+	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,19 @@ 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()
+	}
+	if d.TimeoutDuration != 0 {
+		out.Timeout = d.TimeoutDuration.String()
+	}
+	if d.DeregisterCriticalServiceAfterDuration != 0 {
+		out.DeregisterCriticalServiceAfter = d.DeregisterCriticalServiceAfterDuration.String()
+	}
+
+	return json.Marshal(out)
 }
 
 func (d *HealthCheckDefinition) UnmarshalJSON(data []byte) error {
@@ -84,21 +101,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
 }

api/txn_test.go

@@ -33,12 +33,26 @@ func TestAPI_ClientTxn(t *testing.T) {
 			ID:      "foo1",
 			Service: "foo",
 		},
-		Check: &AgentCheck{
+		Checks: HealthChecks{
+			{
 				CheckID: "bar",
 				Status:  "critical",
 				Definition: HealthCheckDefinition{
 					TCP:                                    "1.1.1.1",
-				Interval: 5 * time.Second,
+					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{
@@ -166,7 +186,30 @@ func TestAPI_ClientTxn(t *testing.T) {
 				Status:  "critical",
 				Definition: HealthCheckDefinition{
 					TCP:                                    "1.1.1.1",
-					Interval: 5 * time.Second,
+					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,