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

Store check type in catalog #6561

Merged
merged 6 commits into from
Oct 17, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
53 changes: 53 additions & 0 deletions agent/catalog_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,59 @@ func TestCatalogServices_NodeMetaFilter(t *testing.T) {
}
}

func TestCatalogRegister_checkRegistration(t *testing.T) {
t.Parallel()
a := NewTestAgent(t, t.Name(), "")
defer a.Shutdown()

// Register node with a service and check
check := structs.HealthCheck{
Node: "foo",
CheckID: "foo-check",
Name: "foo check",
ServiceID: "api",
Definition: structs.HealthCheckDefinition{
TCP: "localhost:8888",
Interval: 5 * time.Second,
},
}

args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "foo",
Address: "127.0.0.1",
Service: &structs.NodeService{
Service: "api",
},
Check: &check,
}

var out struct{}
if err := a.RPC("Catalog.Register", args, &out); err != nil {
t.Fatalf("err: %v", err)
}

retry.Run(t, func(r *retry.R) {
req, _ := http.NewRequest("GET", "/v1/health/checks/api", nil)
resp := httptest.NewRecorder()
obj, err := a.srv.HealthServiceChecks(resp, req)
if err != nil {
r.Fatalf("err: %v", err)
}

checks := obj.(structs.HealthChecks)
if len(checks) != 1 {
r.Fatalf("expected 1 check, got: %d", len(checks))
}
if checks[0].CheckID != check.CheckID {
r.Fatalf("expected check id %s, got %s", check.Type, checks[0].Type)
}
if checks[0].Type != "tcp" {
r.Fatalf("expected check type tcp, got %s", checks[0].Type)
}
})
}

func TestCatalogServiceNodes(t *testing.T) {
t.Parallel()
a := NewTestAgent(t, t.Name(), "")
Expand Down
7 changes: 7 additions & 0 deletions agent/consul/catalog_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ func (c *Catalog) Register(args *structs.RegisterRequest, reply *struct{}) error
check.Node = args.Node
}
checkPreApply(check)

// Populate check type for cases when this is a direct reg with /catalog/register
// Typically this is populated when a check is registered with an agent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: if this endpoint is being used then by definition it's a direct reg with /catalog/register.

Agent Anti-entropy goes straight to RPC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm trying to communicate is that the user registered the check directly to the catalog. Anti-entropy is different because of the indirection. Users add checks to the agent then the agent triggers a sync.

How about:

// Populate check type for cases when a check is registered in the catalog directly and not via anti-entropy

if check.Type == "" {
chkType := check.CheckType()
check.Type = chkType.Type()
}
}

// Check the complete register request against the given ACL policy.
Expand Down
26 changes: 26 additions & 0 deletions agent/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,32 @@ func (c *HealthCheck) Clone() *HealthCheck {
return clone
}

func (c *HealthCheck) CheckType() *CheckType {
return &CheckType{
CheckID: c.CheckID,
Name: c.Name,
Status: c.Status,
Notes: c.Notes,

ScriptArgs: c.Definition.ScriptArgs,
AliasNode: c.Definition.AliasNode,
AliasService: c.Definition.AliasService,
HTTP: c.Definition.HTTP,
GRPC: c.Definition.GRPC,
GRPCUseTLS: c.Definition.GRPCUseTLS,
Header: c.Definition.Header,
Method: c.Definition.Method,
TCP: c.Definition.TCP,
Interval: c.Definition.Interval,
DockerContainerID: c.Definition.DockerContainerID,
Shell: c.Definition.Shell,
TLSSkipVerify: c.Definition.TLSSkipVerify,
Timeout: c.Definition.Timeout,
TTL: c.Definition.TTL,
DeregisterCriticalServiceAfter: c.Definition.DeregisterCriticalServiceAfter,
}
}

// HealthChecks is a collection of HealthCheck structs.
type HealthChecks []*HealthCheck

Expand Down
2 changes: 2 additions & 0 deletions api/txn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ func TestAPI_ClientTxn(t *testing.T) {
DeregisterCriticalServiceAfter: ReadableDuration(20 * time.Second),
DeregisterCriticalServiceAfterDuration: 20 * time.Second,
},
Type: "tcp",
CreateIndex: ret.Results[4].Check.CreateIndex,
ModifyIndex: ret.Results[4].Check.CreateIndex,
},
Expand All @@ -212,6 +213,7 @@ func TestAPI_ClientTxn(t *testing.T) {
DeregisterCriticalServiceAfter: ReadableDuration(160 * time.Second),
DeregisterCriticalServiceAfterDuration: 160 * time.Second,
},
Type: "tcp",
CreateIndex: ret.Results[4].Check.CreateIndex,
ModifyIndex: ret.Results[4].Check.CreateIndex,
},
Expand Down