-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 11 commits
7ae975e
cec0170
91075e1
bdf4347
5d3bd1b
23bfbbf
7d2f1c6
dcecac1
2a809b9
1af7c57
e89b5af
9057422
db82564
8218442
a73aaf1
c65fdc3
6295108
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,55 @@ func TestHTTP_NodesList(t *testing.T) { | |
}) | ||
} | ||
|
||
func TestHTTP_NodesPrefixList(t *testing.T) { | ||
httpTest(t, nil, func(s *TestServer) { | ||
ids := []string{"aaaaa", "aaaab", "aaabb", "aabbb", "abbbb", "bbbbb"} | ||
for i := 0; i < 5; i++ { | ||
// Create the node | ||
node := mock.Node() | ||
node.ID = ids[i] | ||
args := structs.NodeRegisterRequest{ | ||
Node: node, | ||
WriteRequest: structs.WriteRequest{Region: "global"}, | ||
} | ||
var resp structs.NodeUpdateResponse | ||
if err := s.Agent.RPC("Node.Register", &args, &resp); err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
} | ||
|
||
// Make the HTTP request | ||
req, err := http.NewRequest("GET", "/v1/nodes?prefix=aaa", nil) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
respW := httptest.NewRecorder() | ||
|
||
// Make the request | ||
obj, err := s.Server.NodesRequest(respW, req) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
|
||
// Check for the index | ||
if respW.HeaderMap.Get("X-Nomad-Index") == "" { | ||
t.Fatalf("missing index") | ||
} | ||
if respW.HeaderMap.Get("X-Nomad-KnownLeader") != "true" { | ||
t.Fatalf("missing known leader") | ||
} | ||
if respW.HeaderMap.Get("X-Nomad-LastContact") == "" { | ||
t.Fatalf("missing last contact") | ||
} | ||
|
||
// Check the job | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nodes |
||
n := obj.([]*structs.NodeListStub) | ||
if len(n) != 3 { | ||
t.Fatalf("bad: %#v", n) | ||
} | ||
}) | ||
} | ||
|
||
func TestHTTP_NodeForceEval(t *testing.T) { | ||
httpTest(t, nil, func(s *TestServer) { | ||
// Create the node | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 toggling drain mode: %s", err)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is strictly not an error for toggling the drain mode, but rather for looking up the node. However, the testcase checks for this string so I'd like to hear your thoughts on this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine as the |
||
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 | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ import ( | |
"fmt" | ||
"sort" | ||
"strings" | ||
|
||
"github.com/hashicorp/nomad/api" | ||
) | ||
|
||
type NodeStatusCommand struct { | ||
|
@@ -100,8 +102,41 @@ func (c *NodeStatusCommand) Run(args []string) int { | |
nodeID := args[0] | ||
node, _, err := client.Nodes().Info(nodeID, nil) | ||
if err != nil { | ||
c.Ui.Error(fmt.Sprintf("Error querying node info: %s", err)) | ||
return 1 | ||
// Exact lookup failed, try with prefix based search | ||
nodes, _, err := client.Nodes().List(&api.QueryOptions{Prefix: nodeID}) | ||
if err != nil { | ||
c.Ui.Error(fmt.Sprintf("Error querying node info: %s", err)) | ||
return 1 | ||
} | ||
// Return error if no nodes are found | ||
if len(nodes) == 0 { | ||
c.Ui.Error(fmt.Sprintf("Node not found")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would make this more verbose. |
||
return 1 | ||
} | ||
if len(nodes) > 1 { | ||
// Format the nodes list that matches the prefix so that the user | ||
// can create a more specific request | ||
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, | ||
node.Datacenter, | ||
node.Name, | ||
node.NodeClass, | ||
node.Drain, | ||
node.Status) | ||
} | ||
// Dump the output | ||
c.Ui.Output(formatList(out)) | ||
return 0 | ||
} | ||
// Query full node information for unique prefix match | ||
node, _, err = client.Nodes().Info(nodes[0].ID, nil) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The extra lookup is required since the list function does not return the full information that we need, kinda annoying to make three api calls but I can't see another way? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah since we return the stub you have to. Its okay tho, its the CLI, it isn't high QPS where performance matters too much |
||
if err != nil { | ||
c.Ui.Error(fmt.Sprintf("Error querying node info: %s", err)) | ||
return 1 | ||
} | ||
} | ||
|
||
m := node.Attributes | ||
|
@@ -132,7 +167,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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is also added in the structs. Once this PR is done I will take a look at removing the api specific structs and using those in the structs package as specified in issue #329 .