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

DNS lookup by Consul node ID #2702

Merged
merged 8 commits into from
Feb 2, 2017
70 changes: 62 additions & 8 deletions consul/state/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ import (
"github.com/hashicorp/go-memdb"
)

const (
// minUUIDLookupLen is used as a minimum length of a node name required before
// we test to see if the name is actually a UUID and perform an ID-based node
// lookup.
minUUIDLookupLen = 8
)

// Nodes is used to pull the full list of nodes for use during snapshots.
func (s *StateSnapshot) Nodes() (memdb.ResultIterator, error) {
iter, err := s.tx.Get("nodes", "id")
Expand Down Expand Up @@ -151,7 +158,7 @@ func (s *StateStore) ensureNodeTxn(tx *memdb.Txn, idx uint64, node *structs.Node
// Check for an existing node
existing, err := tx.First("nodes", "id", node.Node)
if err != nil {
return fmt.Errorf("node lookup failed: %s", err)
return fmt.Errorf("node name lookup failed: %s", err)
}

// Get the indexes
Expand All @@ -174,7 +181,7 @@ func (s *StateStore) ensureNodeTxn(tx *memdb.Txn, idx uint64, node *structs.Node
return nil
}

// GetNode is used to retrieve a node registration by node ID.
// GetNode is used to retrieve a node registration by node name ID.
func (s *StateStore) GetNode(id string) (uint64, *structs.Node, error) {
tx := s.db.Txn(false)
defer tx.Abort()
Expand All @@ -193,6 +200,25 @@ func (s *StateStore) GetNode(id string) (uint64, *structs.Node, error) {
return idx, nil, nil
}

// GetNodeID is used to retrieve a node registration by node ID.
func (s *StateStore) GetNodeID(id types.NodeID) (uint64, *structs.Node, error) {
tx := s.db.Txn(false)
defer tx.Abort()

// Get the table index.
idx := maxIndexTxn(tx, "nodes")

// Retrieve the node from the state store
node, err := tx.First("nodes", "uuid", string(id))
if err != nil {
return 0, nil, fmt.Errorf("node lookup failed: %s", err)
}
if node != nil {
return idx, node.(*structs.Node), nil
}
return idx, nil, nil
}

// Nodes is used to return all of the known nodes.
func (s *StateStore) Nodes(ws memdb.WatchSet) (uint64, structs.Nodes, error) {
tx := s.db.Txn(false)
Expand Down Expand Up @@ -655,23 +681,51 @@ func (s *StateStore) NodeService(nodeName string, serviceID string) (uint64, *st
}

// NodeServices is used to query service registrations by node ID.
func (s *StateStore) NodeServices(ws memdb.WatchSet, nodeName string) (uint64, *structs.NodeServices, error) {
func (s *StateStore) NodeServices(ws memdb.WatchSet, nodeNameOrID string) (uint64, *structs.NodeServices, error) {
tx := s.db.Txn(false)
defer tx.Abort()

// Get the table index.
idx := maxIndexTxn(tx, "nodes", "services")

// Query the node
watchCh, n, err := tx.FirstWatch("nodes", "id", nodeName)
// Query the node by node name
watchCh, n, err := tx.FirstWatch("nodes", "id", nodeNameOrID)
if err != nil {
return 0, nil, fmt.Errorf("node lookup failed: %s", err)
}
ws.Add(watchCh)
if n == nil {
return 0, nil, nil

if n != nil {
ws.Add(watchCh)
} else {
if len(nodeNameOrID) < minUUIDLookupLen {
ws.Add(watchCh)
return 0, nil, nil
}

// Attempt to lookup the node by it's node ID
Copy link
Contributor

Choose a reason for hiding this comment

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

"its"

iter, err := tx.Get("nodes", "uuid_prefix", nodeNameOrID)
if err != nil {
return 0, nil, fmt.Errorf("node ID lookup failed: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

This case is probably a soft error (maybe debug log it) where we should just add the watch channel and return 0, nil, nil. If they type in a bad name they could hit this and get a confusing error.

}
n = iter.Next()
if n == nil {
ws.Add(watchCh)
return 0, nil, nil
}

idWatchCh := iter.WatchCh()
if iter.Next() != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably worth a comment about trying to make sure there's not another node that matches the prefix. It's pretty obscure logic :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also wrt the comment below - it's not that we didn't find anything - there were too many things.

// Watch on the channel that did a node name lookup if we don't find
// anything when searching by Node ID.
ws.Add(watchCh)
return 0, nil, nil
}

ws.Add(idWatchCh)
}

node := n.(*structs.Node)
nodeName := node.Node

// Read all of the services
services, err := tx.Get("services", "node", nodeName)
Expand Down
71 changes: 52 additions & 19 deletions consul/state/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,23 @@ import (
"github.com/hashicorp/consul/lib"
"github.com/hashicorp/consul/types"
"github.com/hashicorp/go-memdb"
uuid "github.com/hashicorp/go-uuid"
)

func makeRandomNodeID(t *testing.T) types.NodeID {
id, err := uuid.GenerateUUID()
if err != nil {
t.Fatalf("err: %v", err)
}
return types.NodeID(id)
}

func TestStateStore_EnsureRegistration(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth it to add another test, or a clause here that calls NodeServices() with a UUID.

s := testStateStore(t)

// Start with just a node.
req := &structs.RegisterRequest{
ID: types.NodeID("40e4a748-2192-161a-0510-9bf59fe950b5"),
ID: makeRandomNodeID(t),
Node: "node1",
Address: "1.2.3.4",
TaggedAddresses: map[string]string{
Expand All @@ -27,6 +36,7 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
"somekey": "somevalue",
},
}
nodeID := req.ID
if err := s.EnsureRegistration(1, req); err != nil {
t.Fatalf("err: %s", err)
}
Expand All @@ -37,14 +47,21 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
if err != nil {
t.Fatalf("err: %s", err)
}
if out.ID != types.NodeID("40e4a748-2192-161a-0510-9bf59fe950b5") ||
if out.ID != nodeID ||
out.Node != "node1" || out.Address != "1.2.3.4" ||
len(out.TaggedAddresses) != 1 ||
out.TaggedAddresses["hello"] != "world" ||
out.Meta["somekey"] != "somevalue" ||
out.CreateIndex != 1 || out.ModifyIndex != 1 {
t.Fatalf("bad node returned: %#v", out)
}
_, out2, err := s.GetNodeID(nodeID)
if err != nil {
t.Fatalf("err: %s", err)
}
if !reflect.DeepEqual(out, out2) {
t.Fatalf("bad node returned: %#v -- %#v", out, out2)
}
}
verifyNode()

Expand Down Expand Up @@ -183,27 +200,39 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {

// Start with just a node.
req := &structs.RegisterRequest{
ID: makeRandomNodeID(t),
Node: "node1",
Address: "1.2.3.4",
}
nodeID := string(req.ID)
nodeName := string(req.Node)
restore := s.Restore()
if err := restore.Registration(1, req); err != nil {
t.Fatalf("err: %s", err)
}
restore.Commit()

// Retrieve the node and verify its contents.
verifyNode := func() {
_, out, err := s.GetNode("node1")
verifyNode := func(nodeLookup string) {
_, out, err := s.GetNode(nodeLookup)
if err != nil {
t.Fatalf("err: %s", err)
}
if out.Node != "node1" || out.Address != "1.2.3.4" ||
if out == nil {
_, out, err = s.GetNodeID(types.NodeID(nodeLookup))
if err != nil {
t.Fatalf("err: %s", err)
}
}

if out == nil || out.Address != "1.2.3.4" ||
!(out.Node == nodeLookup || string(out.ID) == nodeLookup) ||
out.CreateIndex != 1 || out.ModifyIndex != 1 {
t.Fatalf("bad node returned: %#v", out)
}
}
verifyNode()
verifyNode(nodeID)
verifyNode(nodeName)

// Add in a service definition.
req.Service = &structs.NodeService{
Expand All @@ -219,8 +248,8 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {
restore.Commit()

// Verify that the service got registered.
verifyService := func() {
idx, out, err := s.NodeServices(nil, "node1")
verifyService := func(nodeLookup string) {
idx, out, err := s.NodeServices(nil, nodeLookup)
if err != nil {
t.Fatalf("err: %s", err)
}
Expand All @@ -240,7 +269,7 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {

// Add in a top-level check.
req.Check = &structs.HealthCheck{
Node: "node1",
Node: nodeName,
CheckID: "check1",
Name: "check",
}
Expand All @@ -252,7 +281,7 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {

// Verify that the check got registered.
verifyCheck := func() {
idx, out, err := s.NodeChecks(nil, "node1")
idx, out, err := s.NodeChecks(nil, nodeName)
if err != nil {
t.Fatalf("err: %s", err)
}
Expand All @@ -263,19 +292,21 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {
t.Fatalf("bad: %#v", out)
}
c := out[0]
if c.Node != "node1" || c.CheckID != "check1" || c.Name != "check" ||
if c.Node != nodeName || c.CheckID != "check1" || c.Name != "check" ||
c.CreateIndex != 3 || c.ModifyIndex != 3 {
t.Fatalf("bad check returned: %#v", c)
}
}
verifyNode()
verifyService()
verifyNode(nodeID)
verifyNode(nodeName)
verifyService(nodeID)
verifyService(nodeName)
verifyCheck()

// Add in another check via the slice.
req.Checks = structs.HealthChecks{
&structs.HealthCheck{
Node: "node1",
Node: nodeName,
CheckID: "check2",
Name: "check",
},
Expand All @@ -287,10 +318,12 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {
restore.Commit()

// Verify that the additional check got registered.
verifyNode()
verifyService()
verifyNode(nodeID)
verifyNode(nodeName)
verifyService(nodeID)
verifyService(nodeName)
func() {
idx, out, err := s.NodeChecks(nil, "node1")
idx, out, err := s.NodeChecks(nil, nodeName)
if err != nil {
t.Fatalf("err: %s", err)
}
Expand All @@ -301,13 +334,13 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) {
t.Fatalf("bad: %#v", out)
}
c1 := out[0]
if c1.Node != "node1" || c1.CheckID != "check1" || c1.Name != "check" ||
if c1.Node != nodeName || c1.CheckID != "check1" || c1.Name != "check" ||
c1.CreateIndex != 3 || c1.ModifyIndex != 4 {
t.Fatalf("bad check returned: %#v", c1)
}

c2 := out[1]
if c2.Node != "node1" || c2.CheckID != "check2" || c2.Name != "check" ||
if c2.Node != nodeName || c2.CheckID != "check2" || c2.Name != "check" ||
c2.CreateIndex != 4 || c2.ModifyIndex != 4 {
t.Fatalf("bad check returned: %#v", c2)
}
Expand Down
5 changes: 5 additions & 0 deletions consul/state/prepared_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ var validUUID = regexp.MustCompile(`(?i)^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f

// isUUID returns true if the given string is a valid UUID.
func isUUID(str string) bool {
const uuidLen = 36
if len(str) != uuidLen {
return false
}

return validUUID.MatchString(str)
}

Expand Down
8 changes: 8 additions & 0 deletions consul/state/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ func nodesTableSchema() *memdb.TableSchema {
Lowercase: true,
},
},
"uuid": &memdb.IndexSchema{
Name: "uuid",
AllowMissing: true,
Unique: true,
Indexer: &memdb.UUIDFieldIndex{
Field: "ID",
},
},
"meta": &memdb.IndexSchema{
Name: "meta",
AllowMissing: true,
Expand Down