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

Allow lookups based on short identifiers #575

Merged
merged 17 commits into from
Jan 6, 2016
Merged
Show file tree
Hide file tree
Changes from 7 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 command/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (m *monitor) monitor(evalID string) int {
state.index = eval.CreateIndex

// Query the allocations associated with the evaluation
allocs, _, err := m.client.Evaluations().Allocations(evalID, nil)
allocs, _, err := m.client.Evaluations().Allocations(eval.ID, nil)
if err != nil {
m.ui.Error(fmt.Sprintf("Error reading allocations: %s", err))
return 1
Expand Down
9 changes: 8 additions & 1 deletion command/node_drain.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ func (c *NodeDrainCommand) Run(args []string) int {
return 1
}

// Check if node exists
node, _, err := client.Nodes().Info(nodeID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying node info: %s", err))
return 1
}

// Toggle node draining
if _, err := client.Nodes().ToggleDrain(nodeID, enable, nil); err != nil {
if _, err := client.Nodes().ToggleDrain(node.ID, enable, nil); err != nil {
c.Ui.Error(fmt.Sprintf("Error toggling drain mode: %s", err))
return 1
}
Expand Down
32 changes: 30 additions & 2 deletions command/node_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"sort"
"strings"

"github.com/hashicorp/nomad/api"
)

type NodeStatusCommand struct {
Expand Down Expand Up @@ -78,12 +80,14 @@ func (c *NodeStatusCommand) Run(args []string) int {
return 0
}

shortenNodeId := shouldShortenNodeIds(nodes)
Copy link
Contributor

Choose a reason for hiding this comment

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

There should probably be a verbose flag that does not shorten the node ids


// Format the nodes list
out := make([]string, len(nodes)+1)
out[0] = "ID|DC|Name|Class|Drain|Status"
for i, node := range nodes {
out[i+1] = fmt.Sprintf("%s|%s|%s|%s|%v|%s",
node.ID,
shortenId(node.ID, shortenNodeId),
node.Datacenter,
node.Name,
node.NodeClass,
Expand Down Expand Up @@ -132,7 +136,7 @@ func (c *NodeStatusCommand) Run(args []string) int {
var allocs []string
if !short {
// Query the node allocations
nodeAllocs, _, err := client.Nodes().Allocations(nodeID, nil)
nodeAllocs, _, err := client.Nodes().Allocations(node.ID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying node allocations: %s", err))
return 1
Expand Down Expand Up @@ -160,3 +164,27 @@ func (c *NodeStatusCommand) Run(args []string) int {
}
return 0
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove both these methods and replace them with one that takes the original nodes and returns a map of their original ids to their shortened ones.

shortIds(nodes []*api.NodeListStub) map[string]string

With this method I think you could be a bit more advanced with how you shorten. You could start at 8 characters and if there is a collision, then up it to 9, etc, until you get no collisions. Further it is probably still worth while to shorten the non-custom ones if there exists a custom one.

// check if there is a collision if we shorten the Node ids
func shouldShortenNodeIds(nodes []*api.NodeListStub) bool {
ids := map[string]bool{}

for _, node := range nodes {
if len(node.ID) != 36 {
return false //We have a custom ID, don't shorten anything
} else if ids[node.ID[:8]] == true {
return false //There is a collision
} else {
ids[node.ID[:8]] = true
}
}
return true
}

// shorten an UUID syntax XXXXXXXX-XX... to 8 chars XXXXXXXX
func shortenId(id string, shouldShortenId bool) string {
if shouldShortenId == true {
return id[:8]
}
return id
}
45 changes: 45 additions & 0 deletions command/node_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"
"testing"

"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/testutil"
"github.com/mitchellh/cli"
)
Expand Down Expand Up @@ -108,3 +109,47 @@ func TestNodeStatusCommand_Fails(t *testing.T) {
t.Fatalf("expected not found error, got: %s", out)
}
}

func Test_ShortenId(t *testing.T) {
id := "1234567890"
shortID := "12345678"

dontShorten := shortenId(id, false)
if dontShorten != id {
t.Fatalf("Shorten ID should not short id on false, expected %s, got: %s", id, dontShorten)
}

shorten := shortenId(id, true)
if shorten != shortID {
t.Fatalf("Shorten ID should short id on true, expected %s, got: %s", shortID, shorten)
}
}

func Test_ShouldShortenNodeIds(t *testing.T) {
var list []*api.NodeListStub
nodeCustomId := &api.NodeListStub{
ID: "my_own_id",
}
nodeOne := &api.NodeListStub{
ID: "11111111-1111-1111-1111-111111111111",
}
nodeTwo := &api.NodeListStub{
ID: "11111111-2222-2222-2222-222222222222",
}

list = append(list, nodeCustomId)
if shouldShortenNodeIds(list) != false {
t.Fatalf("ShouldShortenNodeIds should return false when using custom id")
}

list = nil
list = append(list, nodeOne)
if shouldShortenNodeIds(list) != true {
t.Fatalf("ShouldShortenNodeIds should return true when no collisions")
}

list = append(list, nodeTwo)
if shouldShortenNodeIds(list) != false {
t.Fatalf("ShouldShortenNodeIds should return false when collision detected")
}
}
4 changes: 2 additions & 2 deletions command/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,14 @@ func (c *StatusCommand) Run(args []string) int {
var evals, allocs []string
if !short {
// Query the evaluations
jobEvals, _, err := client.Jobs().Evaluations(jobID, nil)
jobEvals, _, err := client.Jobs().Evaluations(job.ID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying job evaluations: %s", err))
return 1
}

// Query the allocations
jobAllocs, _, err := client.Jobs().Allocations(jobID, nil)
jobAllocs, _, err := client.Jobs().Allocations(job.ID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error querying job allocations: %s", err))
return 1
Expand Down
5 changes: 3 additions & 2 deletions command/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,14 @@ func (c *StopCommand) Run(args []string) int {
}

// Check if the job exists
if _, _, err := client.Jobs().Info(jobID, nil); err != nil {
job, _, err := client.Jobs().Info(jobID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error deregistering job: %s", err))
return 1
}

// Invoke the stop
evalID, _, err := client.Jobs().Deregister(jobID, nil)
evalID, _, err := client.Jobs().Deregister(job.ID, nil)
if err != nil {
c.Ui.Error(fmt.Sprintf("Error deregistering job: %s", err))
return 1
Expand Down
40 changes: 36 additions & 4 deletions nomad/alloc_endpoint.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package nomad

import (
"fmt"
"time"

"github.com/armon/go-metrics"
Expand Down Expand Up @@ -80,17 +81,48 @@ func (a *Alloc) GetAlloc(args *structs.AllocSpecificRequest,
if err != nil {
return err
}
out, err := snap.AllocByID(args.AllocID)
if err != nil {
return err

var out *structs.Allocation

// Exact lookup if the identifier length is 36 (full UUID)
if len(args.AllocID) == 36 {
out, err = snap.AllocByID(args.AllocID)
if err != nil {
return err
}
} else {
iter, err := snap.AllocByIDPrefix(args.AllocID)
Copy link
Contributor

Choose a reason for hiding this comment

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

So as I mentioned earlier we don't want to put this logic here (for all the endpoints). These endpoint are for looking up the particular id.

I think the best way to add this feature is to add a ?prefix=true parameter to the list requests (there is one for job/eval/alloc).

You would have to add:

  • parsing here and example call
  • add to struct here
  • update list methods to use the new state store calls you added. Example
    • So if prefix is set use JobByIdPrefix instead of Jobs
  • The CLI commands would then need to first call with the specific ID and if there is an error call list with the prefix and then show the output to the user.

This has the benefit that it makes the list endpoint more usable, maintains the API of the get endpoints, makes everything much more testable, and we aren't using the error as a return value.

if err != nil {
return err
}

// Gather all matching allocations
var allocs []*structs.Allocation
var allocIds []string
for {
raw := iter.Next()
if raw == nil {
break
}
alloc := raw.(*structs.Allocation)
allocIds = append(allocIds, alloc.ID)
allocs = append(allocs, alloc)
}

if len(allocs) == 1 {
// Return unique allocation
out = allocs[0]
} else if len(allocs) > 1 {
return fmt.Errorf("Ambiguous identifier: %+v", allocIds)
}
}

// Setup the output
reply.Alloc = out
if out != nil {
reply.Index = out.ModifyIndex
} else {
// Use the last index that affected the nodes table
// Use the last index that affected the allocs table
index, err := snap.Index("allocs")
if err != nil {
return err
Expand Down
37 changes: 34 additions & 3 deletions nomad/eval_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,40 @@ func (e *Eval) GetEval(args *structs.EvalSpecificRequest,
if err != nil {
return err
}
out, err := snap.EvalByID(args.EvalID)
if err != nil {
return err

var out *structs.Evaluation

// Exact lookup if the identifier length is 36 (full UUID)
if len(args.EvalID) == 36 {
out, err = snap.EvalByID(args.EvalID)
if err != nil {
return err
}
} else {
iter, err := snap.EvalByIDPrefix(args.EvalID)
if err != nil {
return err
}

// Gather all matching evaluations
var evals []*structs.Evaluation
var evalIds []string
for {
raw := iter.Next()
if raw == nil {
break
}
eval := raw.(*structs.Evaluation)
evalIds = append(evalIds, eval.ID)
evals = append(evals, eval)
}

if len(evals) == 1 {
// Return unique evaluation
out = evals[0]
} else if len(evals) > 1 {
return fmt.Errorf("Ambiguous identifier: %+v", evalIds)
}
}

// Setup the output
Expand Down
28 changes: 28 additions & 0 deletions nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,34 @@ func (j *Job) GetJob(args *structs.JobSpecificRequest,
return err
}

// Exact lookup failed so try a prefix based lookup
if out == nil {
iter, err := snap.JobByIDPrefix(args.JobID)
if err != nil {
return err
}

// Gather all matching jobs
var jobs []*structs.Job
var jobIds []string
for {
raw := iter.Next()
if raw == nil {
break
}
job := raw.(*structs.Job)
jobIds = append(jobIds, job.ID)
jobs = append(jobs, job)
}

if len(jobs) == 1 {
// Return unique match
out = jobs[0]
} else if len(jobs) > 1 {
return fmt.Errorf("Ambiguous identifier: %+v", jobIds)
}
}

// Setup the output
reply.Job = out
if out != nil {
Expand Down
25 changes: 25 additions & 0 deletions nomad/node_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,31 @@ func (n *Node) GetNode(args *structs.NodeSpecificRequest,
return err
}

if out == nil {
iter, err := snap.NodeByIDPrefix(args.NodeID)
if err != nil {
return err
}

// Gather all matching nodes
var nodes []*structs.Node
for {
raw := iter.Next()
if raw == nil {
break
}
node := raw.(*structs.Node)
nodes = append(nodes, node)
}

if len(nodes) == 1 {
// return unique node
out = nodes[0]
} else {
return fmt.Errorf("Ambiguous identifier: %v", nodes)
}
}

// Setup the output
reply.Node = out
if out != nil {
Expand Down
Loading