From e1e019a36a8818a45a49f057c37f5bf0f2cd984a Mon Sep 17 00:00:00 2001 From: James Rasell Date: Tue, 17 Jul 2018 11:03:13 +0100 Subject: [PATCH 1/3] Add NodeName to the alloc/job status outputs. Currently when operators need to log onto a machine where an alloc is running they will need to perform both an alloc/job status call and then a call to discover the node name from the node list. This updates both the job status and alloc status output to include the node name within the information to make operator use easier. Closes #2359 Cloess #1180 --- api/allocations.go | 2 ++ command/alloc_status.go | 1 + command/job_status.go | 10 ++++++---- nomad/structs/structs.go | 5 +++++ scheduler/generic_sched.go | 1 + scheduler/system_sched.go | 1 + 6 files changed, 16 insertions(+), 4 deletions(-) diff --git a/api/allocations.go b/api/allocations.go index 8d02791273b..3de4db8b01f 100644 --- a/api/allocations.go +++ b/api/allocations.go @@ -72,6 +72,7 @@ type Allocation struct { EvalID string Name string NodeID string + NodeName string JobID string Job *Job TaskGroup string @@ -121,6 +122,7 @@ type AllocationListStub struct { EvalID string Name string NodeID string + NodeName string JobID string JobVersion uint64 TaskGroup string diff --git a/command/alloc_status.go b/command/alloc_status.go index 773791c59ec..03fe646b9e2 100644 --- a/command/alloc_status.go +++ b/command/alloc_status.go @@ -234,6 +234,7 @@ func formatAllocBasicInfo(alloc *api.Allocation, client *api.Client, uuidLength fmt.Sprintf("Eval ID|%s", limit(alloc.EvalID, uuidLength)), fmt.Sprintf("Name|%s", alloc.Name), fmt.Sprintf("Node ID|%s", limit(alloc.NodeID, uuidLength)), + fmt.Sprintf("Node Name|%s", alloc.NodeName), fmt.Sprintf("Job ID|%s", alloc.JobID), fmt.Sprintf("Job Version|%d", getVersion(alloc.Job)), fmt.Sprintf("Client Status|%s", alloc.ClientStatus), diff --git a/command/job_status.go b/command/job_status.go index 9e7f9db8bb8..0805e79b565 100644 --- a/command/job_status.go +++ b/command/job_status.go @@ -413,12 +413,13 @@ func formatAllocListStubs(stubs []*api.AllocationListStub, verbose bool, uuidLen allocs := make([]string, len(stubs)+1) if verbose { - allocs[0] = "ID|Eval ID|Node ID|Task Group|Version|Desired|Status|Created|Modified" + allocs[0] = "ID|Eval ID|Node ID|Node Name|Task Group|Version|Desired|Status|Created|Modified" for i, alloc := range stubs { - allocs[i+1] = fmt.Sprintf("%s|%s|%s|%s|%d|%s|%s|%s|%s", + allocs[i+1] = fmt.Sprintf("%s|%s|%s|%s|%s|%d|%s|%s|%s|%s", limit(alloc.ID, uuidLength), limit(alloc.EvalID, uuidLength), limit(alloc.NodeID, uuidLength), + alloc.NodeName, alloc.TaskGroup, alloc.JobVersion, alloc.DesiredStatus, @@ -427,14 +428,15 @@ func formatAllocListStubs(stubs []*api.AllocationListStub, verbose bool, uuidLen formatUnixNanoTime(alloc.ModifyTime)) } } else { - allocs[0] = "ID|Node ID|Task Group|Version|Desired|Status|Created|Modified" + allocs[0] = "ID|Node ID|Node Name|Task Group|Version|Desired|Status|Created|Modified" for i, alloc := range stubs { now := time.Now() createTimePretty := prettyTimeDiff(time.Unix(0, alloc.CreateTime), now) modTimePretty := prettyTimeDiff(time.Unix(0, alloc.ModifyTime), now) - allocs[i+1] = fmt.Sprintf("%s|%s|%s|%d|%s|%s|%s|%s", + allocs[i+1] = fmt.Sprintf("%s|%s|%s|%s|%d|%s|%s|%s|%s", limit(alloc.ID, uuidLength), limit(alloc.NodeID, uuidLength), + alloc.NodeName, alloc.TaskGroup, alloc.JobVersion, alloc.DesiredStatus, diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 969f1133887..5ef7a922859 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -5751,6 +5751,9 @@ type Allocation struct { // NodeID is the node this is being placed on NodeID string + // NodeName is the name of the node this is being placed on. + NodeName string + // Job is the parent job of the task group being allocated. // This is copied at allocation time to avoid issues if the job // definition is updated. @@ -6132,6 +6135,7 @@ func (a *Allocation) Stub() *AllocListStub { EvalID: a.EvalID, Name: a.Name, NodeID: a.NodeID, + NodeName: a.NodeName, JobID: a.JobID, JobVersion: a.Job.Version, TaskGroup: a.TaskGroup, @@ -6157,6 +6161,7 @@ type AllocListStub struct { EvalID string Name string NodeID string + NodeName string JobID string JobVersion uint64 TaskGroup string diff --git a/scheduler/generic_sched.go b/scheduler/generic_sched.go index 35909dedcfc..99d7131b00b 100644 --- a/scheduler/generic_sched.go +++ b/scheduler/generic_sched.go @@ -482,6 +482,7 @@ func (s *GenericScheduler) computePlacements(destructive, place []placementResul TaskGroup: tg.Name, Metrics: s.ctx.Metrics(), NodeID: option.Node.ID, + NodeName: option.Node.Name, DeploymentID: deploymentID, TaskResources: option.TaskResources, DesiredStatus: structs.AllocDesiredStatusRun, diff --git a/scheduler/system_sched.go b/scheduler/system_sched.go index 4a10d073115..5abb0caf849 100644 --- a/scheduler/system_sched.go +++ b/scheduler/system_sched.go @@ -316,6 +316,7 @@ func (s *SystemScheduler) computePlacements(place []allocTuple) error { TaskGroup: missing.TaskGroup.Name, Metrics: s.ctx.Metrics(), NodeID: option.Node.ID, + NodeName: option.Node.Name, TaskResources: option.TaskResources, DesiredStatus: structs.AllocDesiredStatusRun, ClientStatus: structs.AllocClientStatusPending, From 59dec15a7a0ec8d0cdf59e5c30d7292281c51be0 Mon Sep 17 00:00:00 2001 From: Arshneet Singh Date: Fri, 18 Jan 2019 03:55:17 -0800 Subject: [PATCH 2/3] Don't display node name if output isn't verbose. Add tests. --- command/alloc_status_test.go | 12 ++++++++++++ command/job_status.go | 5 ++--- command/job_status_test.go | 21 +++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/command/alloc_status_test.go b/command/alloc_status_test.go index fbd19942497..e4b5c239c17 100644 --- a/command/alloc_status_test.go +++ b/command/alloc_status_test.go @@ -120,9 +120,11 @@ func TestAllocStatusCommand_Run(t *testing.T) { } // get an alloc id allocId1 := "" + nodeName := "" if allocs, _, err := client.Jobs().Allocations(jobID, false, nil); err == nil { if len(allocs) > 0 { allocId1 = allocs[0].ID + nodeName = allocs[0].NodeName } } if allocId1 == "" { @@ -141,6 +143,16 @@ func TestAllocStatusCommand_Run(t *testing.T) { t.Fatalf("expected to have 'Modified' but saw: %s", out) } + if !strings.Contains(out, "Modified") { + t.Fatalf("expected to have 'Modified' but saw: %s", out) + } + + nodeNameRegexpStr := fmt.Sprintf(`\nNode Name\s+= %s\n`, regexp.QuoteMeta(nodeName)) + nodeNameRegexp := regexp.MustCompile(nodeNameRegexpStr) + if !nodeNameRegexp.MatchString(out) { + t.Fatalf("expected to have 'Node Name' but saw: %s", out) + } + ui.OutputWriter.Reset() if code := cmd.Run([]string{"-address=" + url, "-verbose", allocId1}); code != 0 { diff --git a/command/job_status.go b/command/job_status.go index 0805e79b565..6fa696ba5ab 100644 --- a/command/job_status.go +++ b/command/job_status.go @@ -428,15 +428,14 @@ func formatAllocListStubs(stubs []*api.AllocationListStub, verbose bool, uuidLen formatUnixNanoTime(alloc.ModifyTime)) } } else { - allocs[0] = "ID|Node ID|Node Name|Task Group|Version|Desired|Status|Created|Modified" + allocs[0] = "ID|Node ID|Task Group|Version|Desired|Status|Created|Modified" for i, alloc := range stubs { now := time.Now() createTimePretty := prettyTimeDiff(time.Unix(0, alloc.CreateTime), now) modTimePretty := prettyTimeDiff(time.Unix(0, alloc.ModifyTime), now) - allocs[i+1] = fmt.Sprintf("%s|%s|%s|%s|%d|%s|%s|%s|%s", + allocs[i+1] = fmt.Sprintf("%s|%s|%s|%d|%s|%s|%s|%s", limit(alloc.ID, uuidLength), limit(alloc.NodeID, uuidLength), - alloc.NodeName, alloc.TaskGroup, alloc.JobVersion, alloc.DesiredStatus, diff --git a/command/job_status_test.go b/command/job_status_test.go index e0bc40b565b..f4a71c7b8c9 100644 --- a/command/job_status_test.go +++ b/command/job_status_test.go @@ -2,6 +2,7 @@ package command import ( "fmt" + "regexp" "strings" "testing" "time" @@ -123,6 +124,14 @@ func TestJobStatusCommand_Run(t *testing.T) { if code := cmd.Run([]string{"-address=" + url, "-verbose", "job2_sfx"}); code != 0 { t.Fatalf("expected exit 0, got: %d", code) } + + nodeName := "" + if allocs, _, err := client.Jobs().Allocations("job2_sfx", false, nil); err == nil { + if len(allocs) > 0 { + nodeName = allocs[0].NodeName + } + } + out = ui.OutputWriter.String() if strings.Contains(out, "job1_sfx") || !strings.Contains(out, "job2_sfx") { t.Fatalf("expected only job2_sfx, got: %s", out) @@ -139,6 +148,18 @@ func TestJobStatusCommand_Run(t *testing.T) { if !strings.Contains(out, "Modified") { t.Fatal("should have modified header") } + + // string calculations based on 1-byte chars, not using runes + allocationsTableName := "Allocations\n" + allocationsTableStr := strings.Split(out, allocationsTableName)[1] + nodeNameHeaderStr := "Node Name" + nodeNameHeaderIndex := strings.Index(allocationsTableStr, nodeNameHeaderStr) + nodeNameRegexpStr := fmt.Sprintf(`.*%s.*\n.{%d}%s`, nodeNameHeaderStr, nodeNameHeaderIndex, regexp.QuoteMeta(nodeName)) + nodeNameRegexp := regexp.MustCompile(nodeNameRegexpStr) + if !nodeNameRegexp.MatchString(out) { + t.Fatalf("expected to have 'Node Name' but saw: %s", out) + } + ui.ErrorWriter.Reset() ui.OutputWriter.Reset() From de5386e05c78e4d97b93a6943979e8056d09384b Mon Sep 17 00:00:00 2001 From: Arshneet Singh Date: Wed, 23 Jan 2019 09:33:22 -0800 Subject: [PATCH 3/3] Remove redundant assertion and replace regex matches with require --- command/alloc_status_test.go | 9 +-------- command/job_status_test.go | 5 +---- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/command/alloc_status_test.go b/command/alloc_status_test.go index e4b5c239c17..b3c7fef2070 100644 --- a/command/alloc_status_test.go +++ b/command/alloc_status_test.go @@ -143,15 +143,8 @@ func TestAllocStatusCommand_Run(t *testing.T) { t.Fatalf("expected to have 'Modified' but saw: %s", out) } - if !strings.Contains(out, "Modified") { - t.Fatalf("expected to have 'Modified' but saw: %s", out) - } - nodeNameRegexpStr := fmt.Sprintf(`\nNode Name\s+= %s\n`, regexp.QuoteMeta(nodeName)) - nodeNameRegexp := regexp.MustCompile(nodeNameRegexpStr) - if !nodeNameRegexp.MatchString(out) { - t.Fatalf("expected to have 'Node Name' but saw: %s", out) - } + require.Regexp(t, regexp.MustCompile(nodeNameRegexpStr), out) ui.OutputWriter.Reset() diff --git a/command/job_status_test.go b/command/job_status_test.go index f4a71c7b8c9..745af018e4c 100644 --- a/command/job_status_test.go +++ b/command/job_status_test.go @@ -155,10 +155,7 @@ func TestJobStatusCommand_Run(t *testing.T) { nodeNameHeaderStr := "Node Name" nodeNameHeaderIndex := strings.Index(allocationsTableStr, nodeNameHeaderStr) nodeNameRegexpStr := fmt.Sprintf(`.*%s.*\n.{%d}%s`, nodeNameHeaderStr, nodeNameHeaderIndex, regexp.QuoteMeta(nodeName)) - nodeNameRegexp := regexp.MustCompile(nodeNameRegexpStr) - if !nodeNameRegexp.MatchString(out) { - t.Fatalf("expected to have 'Node Name' but saw: %s", out) - } + require.Regexp(t, regexp.MustCompile(nodeNameRegexpStr), out) ui.ErrorWriter.Reset() ui.OutputWriter.Reset()