From d3dd2b14024f8d5553d1c71e7b855fa845477304 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Wed, 1 Nov 2017 14:25:46 -0700 Subject: [PATCH] Move check definition to a sub-struct --- agent/consul/catalog_endpoint.go | 7 +++- agent/consul/catalog_endpoint_test.go | 39 +++++++++++++++++++ agent/operator_endpoint.go | 51 ------------------------- agent/structs/structs.go | 8 +++- agent/util.go | 55 +++++++++++++++++++++++++++ agent/util_test.go | 45 ++++++++++++++++++++++ api/coordinate_test.go | 2 +- api/health.go | 6 +++ website/source/api/catalog.html.md | 18 ++++++++- 9 files changed, 173 insertions(+), 58 deletions(-) diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 75adf6cfd6..0d6fef4ee9 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -28,8 +28,11 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error defer metrics.MeasureSince([]string{"catalog", "register"}, time.Now()) // Verify the args. - if args.Node == "" || (args.Address == "" && !args.SkipNodeUpdate) { - return fmt.Errorf("Must provide node and address") + if args.Node == "" { + return fmt.Errorf("Must provide node") + } + if args.Address == "" && !args.SkipNodeUpdate { + return fmt.Errorf("Must provide address if SkipNodeUpdate is not set") } if args.ID != "" { if _, err := uuid.ParseUUID(string(args.ID)); err != nil { diff --git a/agent/consul/catalog_endpoint_test.go b/agent/consul/catalog_endpoint_test.go index dd2d496088..f6825f990a 100644 --- a/agent/consul/catalog_endpoint_test.go +++ b/agent/consul/catalog_endpoint_test.go @@ -78,6 +78,45 @@ func TestCatalog_RegisterService_InvalidAddress(t *testing.T) { } } +func TestCatalog_RegisterService_SkipNodeUpdate(t *testing.T) { + t.Parallel() + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + // Register a node + arg := structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + } + var out struct{} + err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &arg, &out) + if err != nil { + t.Fatal(err) + } + + // Update it with a blank address, should fail. + arg.Address = "" + arg.Service = &structs.NodeService{ + Service: "db", + Port: 8000, + } + err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &arg, &out) + if err == nil || err.Error() != "Must provide address if SkipNodeUpdate is not set" { + t.Fatalf("got error %v want 'Must provide address...'", err) + } + + // Set SkipNodeUpdate, should succeed + arg.SkipNodeUpdate = true + err = msgpackrpc.CallWithCodec(codec, "Catalog.Register", &arg, &out) + if err != nil { + t.Fatal(err) + } +} + func TestCatalog_Register_NodeID(t *testing.T) { t.Parallel() dir1, s1 := testServer(t) diff --git a/agent/operator_endpoint.go b/agent/operator_endpoint.go index 240a9fc726..7ae505b194 100644 --- a/agent/operator_endpoint.go +++ b/agent/operator_endpoint.go @@ -4,7 +4,6 @@ import ( "fmt" "net/http" "strconv" - "strings" "time" "github.com/hashicorp/consul/agent/structs" @@ -266,56 +265,6 @@ func (s *HTTPServer) OperatorAutopilotConfiguration(resp http.ResponseWriter, re } } -type durationFixer map[string]struct{} - -func NewDurationFixer(fields ...string) durationFixer { - d := make(map[string]struct{}) - for _, field := range fields { - d[field] = struct{}{} - } - return d -} - -// FixupDurations is used to handle parsing any field names in the map to time.Durations -func (d durationFixer) FixupDurations(raw interface{}) error { - rawMap, ok := raw.(map[string]interface{}) - if !ok { - return nil - } - for key, val := range rawMap { - if key == "NodeMeta" { - continue - } - - switch val.(type) { - case map[string]interface{}: - if err := d.FixupDurations(val); err != nil { - return err - } - - case []interface{}: - for _, v := range val.([]interface{}) { - if err := d.FixupDurations(v); err != nil { - return err - } - } - - default: - if _, ok := d[strings.ToLower(key)]; ok { - // Convert a string value into an integer - if vStr, ok := val.(string); ok { - dur, err := time.ParseDuration(vStr) - if err != nil { - return err - } - rawMap[key] = dur - } - } - } - } - return nil -} - // OperatorServerHealth is used to get the health of the servers in the local DC func (s *HTTPServer) OperatorServerHealth(resp http.ResponseWriter, req *http.Request) (interface{}, error) { if req.Method != "GET" { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 2c997ae2af..b50f912f86 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -482,6 +482,12 @@ type HealthCheck struct { ServiceName string // optional service name ServiceTags []string // optional service tags + Definition HealthCheckDefinition + + RaftIndex +} + +type HealthCheckDefinition struct { HTTP string `json:",omitempty"` TLSSkipVerify bool `json:",omitempty"` Header map[string][]string `json:",omitempty"` @@ -490,8 +496,6 @@ type HealthCheck struct { Interval api.ReadableDuration `json:",omitempty"` Timeout api.ReadableDuration `json:",omitempty"` DeregisterCriticalServiceAfter api.ReadableDuration `json:",omitempty"` - - RaftIndex } // IsSame checks if one HealthCheck is the same as another, without looking diff --git a/agent/util.go b/agent/util.go index 1044a7742e..0ab2fb6f24 100644 --- a/agent/util.go +++ b/agent/util.go @@ -9,6 +9,8 @@ import ( "os/signal" osuser "os/user" "strconv" + "strings" + "time" "github.com/hashicorp/consul/types" "github.com/hashicorp/go-msgpack/codec" @@ -113,3 +115,56 @@ func ForwardSignals(cmd *exec.Cmd, logFn func(error), shutdownCh <-chan struct{} } }() } + +type durationFixer map[string]bool + +func NewDurationFixer(fields ...string) durationFixer { + d := make(map[string]bool) + for _, field := range fields { + d[field] = true + } + return d +} + +// FixupDurations is used to handle parsing any field names in the map to time.Durations +func (d durationFixer) FixupDurations(raw interface{}) error { + rawMap, ok := raw.(map[string]interface{}) + if !ok { + return nil + } + for key, val := range rawMap { + switch val.(type) { + case map[string]interface{}: + if err := d.FixupDurations(val); err != nil { + return err + } + + case []interface{}: + for _, v := range val.([]interface{}) { + if err := d.FixupDurations(v); err != nil { + return err + } + } + + case []map[string]interface{}: + for _, v := range val.([]map[string]interface{}) { + if err := d.FixupDurations(v); err != nil { + return err + } + } + + default: + if d[strings.ToLower(key)] { + // Convert a string value into an integer + if vStr, ok := val.(string); ok { + dur, err := time.ParseDuration(vStr) + if err != nil { + return err + } + rawMap[key] = dur + } + } + } + } + return nil +} diff --git a/agent/util_test.go b/agent/util_test.go index b10f274c02..f90589782e 100644 --- a/agent/util_test.go +++ b/agent/util_test.go @@ -4,8 +4,10 @@ import ( "os" "runtime" "testing" + "time" "github.com/hashicorp/consul/testutil" + "github.com/pascaldekloe/goe/verify" ) func TestStringHash(t *testing.T) { @@ -74,3 +76,46 @@ func TestSetFilePermissions(t *testing.T) { t.Fatalf("bad: %s", fi.Mode()) } } + +func TestDurationFixer(t *testing.T) { + obj := map[string]interface{}{ + "key1": []map[string]interface{}{ + { + "subkey1": "10s", + }, + { + "subkey2": "5d", + }, + }, + "key2": map[string]interface{}{ + "subkey3": "30s", + "subkey4": "20m", + }, + "key3": "11s", + "key4": "49h", + } + expected := map[string]interface{}{ + "key1": []map[string]interface{}{ + { + "subkey1": 10 * time.Second, + }, + { + "subkey2": "5d", + }, + }, + "key2": map[string]interface{}{ + "subkey3": "30s", + "subkey4": 20 * time.Minute, + }, + "key3": "11s", + "key4": 49 * time.Hour, + } + + fixer := NewDurationFixer("key4", "subkey1", "subkey4") + if err := fixer.FixupDurations(obj); err != nil { + t.Fatal(err) + } + + // Ensure we only processed the intended fieldnames + verify.Values(t, "", obj, expected) +} diff --git a/api/coordinate_test.go b/api/coordinate_test.go index 4cf47259fd..3526201488 100644 --- a/api/coordinate_test.go +++ b/api/coordinate_test.go @@ -92,7 +92,7 @@ func TestAPI_CoordinateUpdate(t *testing.T) { t.Fatal(err) } - retryer := &retry.Timer{Timeout: 10 * time.Second, Wait: 1 * time.Second} + retryer := &retry.Timer{Timeout: 5 * time.Second, Wait: 1 * time.Second} retry.RunWith(retryer, t, func(r *retry.R) { coords, _, err := coord.Node(node, nil) if err != nil { diff --git a/api/health.go b/api/health.go index 3f132759e7..53f3de4f79 100644 --- a/api/health.go +++ b/api/health.go @@ -35,6 +35,12 @@ type HealthCheck struct { ServiceName string ServiceTags []string + Definition HealthCheckDefinition +} + +// HealthCheckDefinition is used to store the details about +// a health check's execution. +type HealthCheckDefinition struct { HTTP string Header map[string][]string Method string diff --git a/website/source/api/catalog.html.md b/website/source/api/catalog.html.md index dfb95cd0e7..bfee1d0f43 100644 --- a/website/source/api/catalog.html.md +++ b/website/source/api/catalog.html.md @@ -69,9 +69,16 @@ The table below shows this endpoint's support for treated as a service level health check, instead of a node level health check. The `Status` must be one of `passing`, `warning`, or `critical`. + The `Definition` field can be provided with details for a TCP or HTTP health + check. For more information, see the [Health Checks](/docs/agent/checks.html) page. + Multiple checks can be provided by replacing `Check` with `Checks` and sending an array of `Check` objects. +- `SkipNodeUpdate` `(bool: false)` - Specifies whether to skip updating the + node part of the registration. Useful in the case where only a health check + or service entry on a node needs to be updated. + It is important to note that `Check` does not have to be provided with `Service` and vice versa. A catalog entry can have either, neither, or both. @@ -106,8 +113,15 @@ and vice versa. A catalog entry can have either, neither, or both. "Name": "Redis health check", "Notes": "Script based health check", "Status": "passing", - "ServiceID": "redis1" - } + "ServiceID": "redis1", + "Definition": { + "TCP": "localhost:8888", + "Interval": "5s", + "Timeout": "1s", + "DeregisterCriticalServiceAfter": "30s" + } + }, + "SkipNodeUpdate": false } ```