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
55 changes: 50 additions & 5 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 @@ -662,15 +688,34 @@ func (s *StateStore) NodeServices(ws memdb.WatchSet, nodeName string) (uint64, *
// Get the table index.
idx := maxIndexTxn(tx, "nodes", "services")

// Query the node
// Query the node by node name
watchCh, n, err := tx.FirstWatch("nodes", "id", nodeName)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rename nodeName to nodeNameOrID in here.

if err != nil {
return 0, nil, fmt.Errorf("node lookup failed: %s", err)
}
ws.Add(watchCh)

if n == nil {
return 0, nil, nil
if len(nodeName) >= minUUIDLookupLen {
// Attempt to lookup the node by it's node ID
var idWatchCh <-chan struct{}
idWatchCh, n, err = tx.FirstWatch("nodes", "uuid_prefix", nodeName)
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 you wan't to use the iterator form here to make sure the lookup isn't ambiguous:

iter, err := tx.Get("nodes", "uuid_prefix", nodeName)
n = iter.Next()
if n != nil {
  if iter.Next() == nil {
    // got it
  } else {
    // it was ambiguous, return nil
  }
}

if err != nil {
return 0, nil, fmt.Errorf("node ID lookup failed: %s", err)
}
if n == nil {
ws.Add(watchCh)
return 0, nil, nil
} else {
ws.Add(idWatchCh)
}
} else {
ws.Add(watchCh)
return 0, nil, nil
}
} else {
ws.Add(watchCh)
}

node := n.(*structs.Node)

// Read all of the services
Expand Down
22 changes: 20 additions & 2 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,6 +200,7 @@ 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",
}
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: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be true since old nodes won't have this yet.

Unique: true,
Indexer: &memdb.UUIDFieldIndex{
Field: "ID",
},
},
"meta": &memdb.IndexSchema{
Name: "meta",
AllowMissing: true,
Expand Down