agent: support custom check id and name

This patch adds support for a custom check id and name when
registering a service.

This is achieved by adding a CheckID and a Name field to the
CheckType structure which is used to register checks with a
service and when returning health check definitions.

CheckDefinition is a superset of CheckType which duplicates
some of the fields of CheckType. This patch decouples these
two structures by removing the embedding of CheckType in
CheckDefinition.

Fixes #3047
This commit is contained in:
Frank Schroeder 2017-05-15 21:49:13 +02:00
parent ad40a855bd
commit 8ad66f4bea
No known key found for this signature in database
GPG Key ID: 4D65C6EAEC87DECD
8 changed files with 360 additions and 189 deletions

View File

@ -1158,14 +1158,21 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes CheckTypes, pe
// Create an associated health check
for i, chkType := range chkTypes {
checkID := fmt.Sprintf("service:%s", service.ID)
if len(chkTypes) > 1 {
checkID += fmt.Sprintf(":%d", i+1)
checkID := string(chkType.CheckID)
if checkID == "" {
checkID = fmt.Sprintf("service:%s", service.ID)
if len(chkTypes) > 1 {
checkID += fmt.Sprintf(":%d", i+1)
}
}
name := chkType.Name
if name == "" {
name = fmt.Sprintf("Service '%s' check", service.Service)
}
check := &structs.HealthCheck{
Node: a.config.NodeName,
CheckID: types.CheckID(checkID),
Name: fmt.Sprintf("Service '%s' check", service.Service),
Name: name,
Status: api.HealthCritical,
Notes: chkType.Notes,
ServiceID: service.ID,
@ -1703,7 +1710,7 @@ func (a *Agent) loadChecks(conf *Config) error {
// Register the checks from config
for _, check := range conf.Checks {
health := check.HealthCheck(conf.NodeName)
chkType := &check.CheckType
chkType := check.CheckType()
if err := a.AddCheck(health, chkType, false, check.Token); err != nil {
return fmt.Errorf("Failed to register check '%s': %v %v", check.Name, err, check)
}

View File

@ -258,7 +258,7 @@ func (s *HTTPServer) AgentRegisterCheck(resp http.ResponseWriter, req *http.Requ
health := args.HealthCheck(s.agent.config.NodeName)
// Verify the check type.
chkType := &args.CheckType
chkType := args.CheckType()
if !chkType.Valid() {
resp.WriteHeader(400)
fmt.Fprint(resp, invalidCheckMessage)

View File

@ -650,9 +650,7 @@ func TestAgent_RegisterCheck(t *testing.T) {
// Register node
args := &CheckDefinition{
Name: "test",
CheckType: CheckType{
TTL: 15 * time.Second,
},
TTL: 15 * time.Second,
}
req, _ := http.NewRequest("GET", "/v1/agent/check/register?token=abc123", jsonReader(args))
obj, err := srv.AgentRegisterCheck(nil, req)
@ -693,10 +691,8 @@ func TestAgent_RegisterCheck_Passing(t *testing.T) {
// Register node
args := &CheckDefinition{
Name: "test",
CheckType: CheckType{
TTL: 15 * time.Second,
},
Name: "test",
TTL: 15 * time.Second,
Status: api.HealthPassing,
}
req, _ := http.NewRequest("GET", "/v1/agent/check/register", jsonReader(args))
@ -732,10 +728,8 @@ func TestAgent_RegisterCheck_BadStatus(t *testing.T) {
// Register node
args := &CheckDefinition{
Name: "test",
CheckType: CheckType{
TTL: 15 * time.Second,
},
Name: "test",
TTL: 15 * time.Second,
Status: "fluffy",
}
req, _ := http.NewRequest("GET", "/v1/agent/check/register", jsonReader(args))
@ -756,9 +750,7 @@ func TestAgent_RegisterCheck_ACLDeny(t *testing.T) {
args := &CheckDefinition{
Name: "test",
CheckType: CheckType{
TTL: 15 * time.Second,
},
TTL: 15 * time.Second,
}
t.Run("no token", func(t *testing.T) {
@ -1596,9 +1588,7 @@ func TestAgent_RegisterCheck_Service(t *testing.T) {
checkArgs := &CheckDefinition{
Name: "memcache_check2",
ServiceID: "memcache",
CheckType: CheckType{
TTL: 15 * time.Second,
},
TTL: 15 * time.Second,
}
req, _ = http.NewRequest("GET", "/v1/agent/check/register", jsonReader(checkArgs))
if _, err := srv.AgentRegisterCheck(nil, req); err != nil {

View File

@ -26,6 +26,7 @@ import (
"github.com/hashicorp/consul/version"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/raft"
"github.com/pascaldekloe/goe/verify"
)
const (
@ -423,100 +424,148 @@ func TestAgent_makeNodeID(t *testing.T) {
}
func TestAgent_AddService(t *testing.T) {
dir, agent := makeAgent(t, nextConfig())
cfg := nextConfig()
cfg.NodeName = "node1"
dir, agent := makeAgent(t, cfg)
defer os.RemoveAll(dir)
defer agent.Shutdown()
// Service registration with a single check
{
srv := &structs.NodeService{
ID: "redis",
Service: "redis",
Tags: []string{"foo"},
Port: 8000,
}
chkTypes := CheckTypes{
&CheckType{
TTL: time.Minute,
Notes: "redis heath check 2",
tests := []struct {
desc string
srv *structs.NodeService
chkTypes CheckTypes
healthChks map[string]*structs.HealthCheck
}{
{
"one check",
&structs.NodeService{
ID: "svcid1",
Service: "svcname1",
Tags: []string{"tag1"},
Port: 8100,
},
}
err := agent.AddService(srv, chkTypes, false, "")
if err != nil {
t.Fatalf("err: %v", err)
}
// Ensure we have a state mapping
if _, ok := agent.state.Services()["redis"]; !ok {
t.Fatalf("missing redis service")
}
// Ensure the check is registered
if _, ok := agent.state.Checks()["service:redis"]; !ok {
t.Fatalf("missing redis check")
}
// Ensure a TTL is setup
if _, ok := agent.checkTTLs["service:redis"]; !ok {
t.Fatalf("missing redis check ttl")
}
// Ensure the notes are passed through
if agent.state.Checks()["service:redis"].Notes == "" {
t.Fatalf("missing redis check notes")
}
CheckTypes{
&CheckType{
CheckID: "check1",
Name: "name1",
TTL: time.Minute,
Notes: "note1",
},
},
map[string]*structs.HealthCheck{
"check1": &structs.HealthCheck{
Node: "node1",
CheckID: "check1",
Name: "name1",
Status: "critical",
Notes: "note1",
ServiceID: "svcid1",
ServiceName: "svcname1",
},
},
},
{
"multiple checks",
&structs.NodeService{
ID: "svcid2",
Service: "svcname2",
Tags: []string{"tag2"},
Port: 8200,
},
CheckTypes{
&CheckType{
CheckID: "check1",
Name: "name1",
TTL: time.Minute,
Notes: "note1",
},
&CheckType{
CheckID: "check-noname",
TTL: time.Minute,
},
&CheckType{
Name: "check-noid",
TTL: time.Minute,
},
&CheckType{
TTL: time.Minute,
},
},
map[string]*structs.HealthCheck{
"check1": &structs.HealthCheck{
Node: "node1",
CheckID: "check1",
Name: "name1",
Status: "critical",
Notes: "note1",
ServiceID: "svcid2",
ServiceName: "svcname2",
},
"check-noname": &structs.HealthCheck{
Node: "node1",
CheckID: "check-noname",
Name: "Service 'svcname2' check",
Status: "critical",
ServiceID: "svcid2",
ServiceName: "svcname2",
},
"service:svcid2:3": &structs.HealthCheck{
Node: "node1",
CheckID: "service:svcid2:3",
Name: "check-noid",
Status: "critical",
ServiceID: "svcid2",
ServiceName: "svcname2",
},
"service:svcid2:4": &structs.HealthCheck{
Node: "node1",
CheckID: "service:svcid2:4",
Name: "Service 'svcname2' check",
Status: "critical",
ServiceID: "svcid2",
ServiceName: "svcname2",
},
},
},
}
// Service registration with multiple checks
{
srv := &structs.NodeService{
ID: "memcache",
Service: "memcache",
Tags: []string{"bar"},
Port: 8000,
}
chkTypes := CheckTypes{
&CheckType{
TTL: time.Minute,
Notes: "memcache health check 1",
},
&CheckType{
TTL: time.Second,
Notes: "memcache heath check 2",
},
}
if err := agent.AddService(srv, chkTypes, false, ""); err != nil {
t.Fatalf("err: %v", err)
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
// check the service registration
t.Run(tt.srv.ID, func(t *testing.T) {
err := agent.AddService(tt.srv, tt.chkTypes, false, "")
if err != nil {
t.Fatalf("err: %v", err)
}
// Ensure we have a state mapping
if _, ok := agent.state.Services()["memcache"]; !ok {
t.Fatalf("missing memcache service")
}
got, want := agent.state.Services()[tt.srv.ID], tt.srv
verify.Values(t, "", got, want)
})
// Ensure both checks were added
if _, ok := agent.state.Checks()["service:memcache:1"]; !ok {
t.Fatalf("missing memcache:1 check")
}
if _, ok := agent.state.Checks()["service:memcache:2"]; !ok {
t.Fatalf("missing memcache:2 check")
}
// check the health checks
for k, v := range tt.healthChks {
t.Run(k, func(t *testing.T) {
got, want := agent.state.Checks()[types.CheckID(k)], v
verify.Values(t, k, got, want)
})
}
// Ensure a TTL is setup
if _, ok := agent.checkTTLs["service:memcache:1"]; !ok {
t.Fatalf("missing memcache:1 check ttl")
}
if _, ok := agent.checkTTLs["service:memcache:2"]; !ok {
t.Fatalf("missing memcache:2 check ttl")
}
// Ensure the notes are passed through
if agent.state.Checks()["service:memcache:1"].Notes == "" {
t.Fatalf("missing redis check notes")
}
if agent.state.Checks()["service:memcache:2"].Notes == "" {
t.Fatalf("missing redis check notes")
}
// check the ttl checks
for k := range tt.healthChks {
t.Run(k+" ttl", func(t *testing.T) {
chk := agent.checkTTLs[types.CheckID(k)]
if chk == nil {
t.Fatal("got nil want TTL check")
}
if got, want := string(chk.CheckID), k; got != want {
t.Fatalf("got CheckID %v want %v", got, want)
}
if got, want := chk.TTL, time.Minute; got != want {
t.Fatalf("got TTL %v want %v", got, want)
}
})
}
})
}
}
@ -558,10 +607,10 @@ func TestAgent_RemoveService(t *testing.T) {
ID: "check2",
Name: "check2",
ServiceID: "memcache",
CheckType: CheckType{TTL: time.Minute},
TTL: time.Minute,
}
hc := check.HealthCheck("node1")
if err := agent.AddCheck(hc, &check.CheckType, false, ""); err != nil {
if err := agent.AddCheck(hc, check.CheckType(), false, ""); err != nil {
t.Fatalf("err: %s", err)
}
@ -619,6 +668,56 @@ func TestAgent_RemoveService(t *testing.T) {
}
}
func TestAgent_RemoveServiceRemovesAllChecks(t *testing.T) {
cfg := nextConfig()
cfg.NodeName = "node1"
dir, agent := makeAgent(t, cfg)
defer os.RemoveAll(dir)
defer agent.Shutdown()
svc := &structs.NodeService{ID: "redis", Service: "redis", Port: 8000}
chk1 := &CheckType{CheckID: "chk1", Name: "chk1", TTL: time.Minute}
chk2 := &CheckType{CheckID: "chk2", Name: "chk2", TTL: 2 * time.Minute}
hchk1 := &structs.HealthCheck{Node: "node1", CheckID: "chk1", Name: "chk1", Status: "critical", ServiceID: "redis", ServiceName: "redis"}
hchk2 := &structs.HealthCheck{Node: "node1", CheckID: "chk2", Name: "chk2", Status: "critical", ServiceID: "redis", ServiceName: "redis"}
// register service with chk1
if err := agent.AddService(svc, CheckTypes{chk1}, false, ""); err != nil {
t.Fatal("Failed to register service", err)
}
// verify chk1 exists
if agent.state.Checks()["chk1"] == nil {
t.Fatal("Could not find health check chk1")
}
// update the service with chk2
if err := agent.AddService(svc, CheckTypes{chk2}, false, ""); err != nil {
t.Fatal("Failed to update service", err)
}
// check that both checks are there
if got, want := agent.state.Checks()["chk1"], hchk1; !verify.Values(t, "", got, want) {
t.FailNow()
}
if got, want := agent.state.Checks()["chk2"], hchk2; !verify.Values(t, "", got, want) {
t.FailNow()
}
// Remove service
if err := agent.RemoveService("redis", false); err != nil {
t.Fatal("Failed to remove service", err)
}
// Check that both checks are gone
if agent.state.Checks()["chk1"] != nil {
t.Fatal("Found health check chk1 want nil")
}
if agent.state.Checks()["chk2"] != nil {
t.Fatal("Found health check chk2 want nil")
}
}
func TestAgent_AddCheck(t *testing.T) {
dir, agent := makeAgent(t, nextConfig())
defer os.RemoveAll(dir)
@ -1272,13 +1371,11 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) {
// Start again with the check registered in config
check2 := &CheckDefinition{
ID: "mem",
Name: "memory check",
Notes: "my cool notes",
CheckType: CheckType{
Script: "/bin/check-redis.py",
Interval: 30 * time.Second,
},
ID: "mem",
Name: "memory check",
Notes: "my cool notes",
Script: "/bin/check-redis.py",
Interval: 30 * time.Second,
}
config.Checks = []*CheckDefinition{check2}
@ -1308,9 +1405,7 @@ func TestAgent_loadChecks_token(t *testing.T) {
ID: "rabbitmq",
Name: "rabbitmq",
Token: "abc123",
CheckType: CheckType{
TTL: 10 * time.Second,
},
TTL: 10 * time.Second,
})
dir, agent := makeAgent(t, config)
defer os.RemoveAll(dir)

View File

@ -44,6 +44,17 @@ const (
// provided: TTL or Script/Interval or HTTP/Interval or TCP/Interval or
// Docker/Interval.
type CheckType struct {
// fields already embedded in CheckDefinition
// Note: CheckType.CheckID == CheckDefinition.ID
CheckID types.CheckID
Name string
Status string
Notes string
// fields copied to CheckDefinition
// Update CheckDefinition when adding fields here
Script string
HTTP string
TCP string
@ -51,18 +62,13 @@ type CheckType struct {
DockerContainerID string
Shell string
TLSSkipVerify bool
Timeout time.Duration
TTL time.Duration
Timeout time.Duration
TTL time.Duration
// DeregisterCriticalServiceAfter, if >0, will cause the associated
// service, if any, to be deregistered if this check is critical for
// longer than this duration.
DeregisterCriticalServiceAfter time.Duration
Status string
Notes string
}
type CheckTypes []*CheckType

View File

@ -15,6 +15,7 @@ import (
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/testutil"
"github.com/pascaldekloe/goe/verify"
)
func TestConfigEncryptBytes(t *testing.T) {
@ -1324,13 +1325,13 @@ func TestDecodeConfig_Checks(t *testing.T) {
"checks": [
{
"id": "chk1",
"name": "mem",
"name": "name1",
"script": "/bin/check_mem",
"interval": "5s"
},
{
"id": "chk2",
"name": "cpu",
"name": "name2",
"script": "/bin/check_cpu",
"interval": "10s"
},
@ -1369,76 +1370,61 @@ func TestDecodeConfig_Checks(t *testing.T) {
]
}`
config, err := DecodeConfig(bytes.NewReader([]byte(input)))
got, err := DecodeConfig(bytes.NewReader([]byte(input)))
if err != nil {
t.Fatalf("err: %s", err)
}
expected := &Config{
want := &Config{
Checks: []*CheckDefinition{
&CheckDefinition{
ID: "chk1",
Name: "mem",
CheckType: CheckType{
Script: "/bin/check_mem",
Interval: 5 * time.Second,
},
ID: "chk1",
Name: "name1",
Script: "/bin/check_mem",
Interval: 5 * time.Second,
},
&CheckDefinition{
ID: "chk2",
Name: "cpu",
CheckType: CheckType{
Script: "/bin/check_cpu",
Interval: 10 * time.Second,
},
ID: "chk2",
Name: "name2",
Script: "/bin/check_cpu",
Interval: 10 * time.Second,
},
&CheckDefinition{
ID: "chk3",
Name: "service:redis:tx",
ServiceID: "redis",
CheckType: CheckType{
Script: "/bin/check_redis_tx",
Interval: time.Minute,
},
Script: "/bin/check_redis_tx",
Interval: time.Minute,
},
&CheckDefinition{
ID: "chk4",
Name: "service:elasticsearch:health",
ServiceID: "elasticsearch",
CheckType: CheckType{
HTTP: "http://localhost:9200/_cluster_health",
Interval: 10 * time.Second,
Timeout: 100 * time.Millisecond,
},
HTTP: "http://localhost:9200/_cluster_health",
Interval: 10 * time.Second,
Timeout: 100 * time.Millisecond,
},
&CheckDefinition{
ID: "chk5",
Name: "service:sslservice",
ServiceID: "sslservice",
CheckType: CheckType{
HTTP: "https://sslservice/status",
Interval: 10 * time.Second,
Timeout: 100 * time.Millisecond,
TLSSkipVerify: false,
},
ID: "chk5",
Name: "service:sslservice",
ServiceID: "sslservice",
HTTP: "https://sslservice/status",
Interval: 10 * time.Second,
Timeout: 100 * time.Millisecond,
TLSSkipVerify: false,
},
&CheckDefinition{
ID: "chk6",
Name: "service:insecure-sslservice",
ServiceID: "insecure-sslservice",
CheckType: CheckType{
HTTP: "https://insecure-sslservice/status",
Interval: 10 * time.Second,
Timeout: 100 * time.Millisecond,
TLSSkipVerify: true,
},
ID: "chk6",
Name: "service:insecure-sslservice",
ServiceID: "insecure-sslservice",
HTTP: "https://insecure-sslservice/status",
Interval: 10 * time.Second,
Timeout: 100 * time.Millisecond,
TLSSkipVerify: true,
},
},
}
if !reflect.DeepEqual(config, expected) {
t.Fatalf("bad: %#v", config)
}
verify.Values(t, "", got, want)
}
func TestDecodeConfig_Multiples(t *testing.T) {
@ -1452,6 +1438,8 @@ func TestDecodeConfig_Multiples(t *testing.T) {
],
"port": 6000,
"check": {
"checkID": "chk1",
"name": "name1",
"script": "/bin/check_redis -p 6000",
"interval": "5s",
"ttl": "20s"
@ -1460,23 +1448,25 @@ func TestDecodeConfig_Multiples(t *testing.T) {
],
"checks": [
{
"id": "chk1",
"name": "mem",
"id": "chk2",
"name": "name2",
"script": "/bin/check_mem",
"interval": "10s"
}
]
}`
config, err := DecodeConfig(bytes.NewReader([]byte(input)))
got, err := DecodeConfig(bytes.NewReader([]byte(input)))
if err != nil {
t.Fatalf("err: %s", err)
}
expected := &Config{
want := &Config{
Services: []*ServiceDefinition{
&ServiceDefinition{
Check: CheckType{
CheckID: "chk1",
Name: "name1",
Interval: 5 * time.Second,
Script: "/bin/check_redis -p 6000",
TTL: 20 * time.Second,
@ -1491,19 +1481,15 @@ func TestDecodeConfig_Multiples(t *testing.T) {
},
Checks: []*CheckDefinition{
&CheckDefinition{
ID: "chk1",
Name: "mem",
CheckType: CheckType{
Script: "/bin/check_mem",
Interval: 10 * time.Second,
},
ID: "chk2",
Name: "name2",
Script: "/bin/check_mem",
Interval: 10 * time.Second,
},
},
}
if !reflect.DeepEqual(config, expected) {
t.Fatalf("bad: %#v", config)
}
verify.Values(t, "", got, want)
}
func TestDecodeConfig_Service(t *testing.T) {
@ -1864,3 +1850,43 @@ func TestUnixSockets(t *testing.T) {
t.Fatalf("bad: %v %v", ok, path2)
}
}
func TestCheckDefinitionToCheckType(t *testing.T) {
got := &CheckDefinition{
ID: "id",
Name: "name",
Status: "green",
Notes: "notes",
ServiceID: "svcid",
Token: "tok",
Script: "/bin/foo",
HTTP: "someurl",
TCP: "host:port",
Interval: 1 * time.Second,
DockerContainerID: "abc123",
Shell: "/bin/ksh",
TLSSkipVerify: true,
Timeout: 2 * time.Second,
TTL: 3 * time.Second,
DeregisterCriticalServiceAfter: 4 * time.Second,
}
want := &CheckType{
CheckID: "id",
Name: "name",
Status: "green",
Notes: "notes",
Script: "/bin/foo",
HTTP: "someurl",
TCP: "host:port",
Interval: 1 * time.Second,
DockerContainerID: "abc123",
Shell: "/bin/ksh",
TLSSkipVerify: true,
Timeout: 2 * time.Second,
TTL: 3 * time.Second,
DeregisterCriticalServiceAfter: 4 * time.Second,
}
verify.Values(t, "", got.CheckType(), want)
}

View File

@ -1,6 +1,8 @@
package agent
import (
"time"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/consul/structs"
"github.com/hashicorp/consul/types"
@ -52,7 +54,22 @@ type CheckDefinition struct {
ServiceID string
Token string
Status string
CheckType `mapstructure:",squash"`
// Copied fields from CheckType without the fields
// already present in CheckDefinition:
//
// ID (CheckID), Name, Status, Notes
//
Script string
HTTP string
TCP string
Interval time.Duration
DockerContainerID string
Shell string
TLSSkipVerify bool
Timeout time.Duration
TTL time.Duration
DeregisterCriticalServiceAfter time.Duration
}
func (c *CheckDefinition) HealthCheck(node string) *structs.HealthCheck {
@ -73,6 +90,25 @@ func (c *CheckDefinition) HealthCheck(node string) *structs.HealthCheck {
return health
}
func (c *CheckDefinition) CheckType() *CheckType {
return &CheckType{
CheckID: c.ID,
Name: c.Name,
Script: c.Script,
HTTP: c.HTTP,
TCP: c.TCP,
Interval: c.Interval,
DockerContainerID: c.DockerContainerID,
Shell: c.Shell,
TLSSkipVerify: c.TLSSkipVerify,
Timeout: c.Timeout,
TTL: c.TTL,
DeregisterCriticalServiceAfter: c.DeregisterCriticalServiceAfter,
Status: c.Status,
Notes: c.Notes,
}
}
// persistedService is used to wrap a service definition and bundle it
// with an ACL token so we can restore both at a later agent start.
type persistedService struct {

View File

@ -98,7 +98,18 @@ The table below shows this endpoint's support for
- `Check` `(Check: nil)` - Specifies a check. Please see the
[check documentation](/api/agent/check.html) for more information about the
accepted fields.
accepted fields. If you don't provide a name or id for the check then they
will be generated. To provide a custom id and/or name set the `CheckID`
and/or `Name` field.
- `Checks` `(array<Check>: nil`) - Specifies a list of checks. Please see the
[check documentation](/api/agent/check.html) for more information about the
accepted fields. If you don't provide a name or id for the check then they
will be generated. To provide a custom id and/or name set the `CheckID`
and/or `Name` field. The automatically generated `Name` and `CheckID` depend
on the position of the check within the array, so even though the behavior is
determinsitic, it is recommended for all checks to either let consul set the
`CheckID` by leaving the field empty/omitting it or to provide a unique value.
- `EnableTagOverride` `(bool: false)` - Specifies to disable the anti-entropy
feature for this service's tags. If `EnableTagOverride` is set to `true` then