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

Bans check updates for nodes other than top-level reg. updates. #2846

Merged
merged 1 commit into from
Mar 30, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion consul/autopilot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func TestAutopilot_CleanupStaleRaftServer(t *testing.T) {
// Add s4 to peers directly
s4addr := fmt.Sprintf("127.0.0.1:%d",
s4.config.SerfLANConfig.MemberlistConfig.BindPort)
s1.raft.AddVoter(raft.ServerID(s4.config.NodeID), raft.ServerAddress(s4addr),0, 0)
s1.raft.AddVoter(raft.ServerID(s4.config.NodeID), raft.ServerAddress(s4addr), 0, 0)

// Verify we have 4 peers
peers, err := s1.numPeers()
Expand Down
14 changes: 8 additions & 6 deletions consul/state/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,21 @@ func (s *StateStore) ensureRegistrationTxn(tx *memdb.Txn, idx uint64, req *struc
}
}

// TODO (slackpad) In Consul 0.8 ban checks that don't have the same
// node as the top-level registration. This is just weird to be able to
// update unrelated nodes' checks from in here. In 0.7.2 we banned this
// up in the ACL check since that's guarded behind an opt-in flag until
// Consul 0.8.

// Add the checks, if any.
if req.Check != nil {
if req.Check.Node != req.Node {
return fmt.Errorf("check node %q does not match node %q",
req.Check.Node, req.Node)
}
if err := s.ensureCheckTxn(tx, idx, req.Check); err != nil {
return fmt.Errorf("failed inserting check: %s", err)
}
}
for _, check := range req.Checks {
if check.Node != req.Node {
return fmt.Errorf("check node %q does not match node %q",
check.Node, req.Node)
}
if err := s.ensureCheckTxn(tx, idx, check); err != nil {
return fmt.Errorf("failed inserting check: %s", err)
}
Expand Down
34 changes: 33 additions & 1 deletion consul/state/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
// Verify that the additional check got registered.
verifyNode()
verifyService()
{
verifyChecks := func() {
idx, out, err := s.NodeChecks(nil, "node1")
if err != nil {
t.Fatalf("err: %s", err)
Expand All @@ -194,6 +194,38 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
t.Fatalf("bad check returned: %#v", c2)
}
}
verifyChecks()

// Try to register a check for some other node (top-level check).
req.Check = &structs.HealthCheck{
Node: "nope",
CheckID: "check1",
Name: "check",
}
err := s.EnsureRegistration(5, req)
if err == nil || !strings.Contains(err.Error(), "does not match node") {
t.Fatalf("err: %s", err)
}
verifyNode()
verifyService()
verifyChecks()

// Try to register a check for some other node (checks array).
req.Check = nil
req.Checks = structs.HealthChecks{
&structs.HealthCheck{
Node: "nope",
CheckID: "check2",
Name: "check",
},
}
err = s.EnsureRegistration(6, req)
if err == nil || !strings.Contains(err.Error(), "does not match node") {
t.Fatalf("err: %s", err)
}
verifyNode()
verifyService()
verifyChecks()
}

func TestStateStore_EnsureRegistration_Restore(t *testing.T) {
Expand Down