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

Add the Nomad agent version to the node-status CLI output. #3002

Merged
merged 2 commits into from
Aug 22, 2017
Merged

Add the Nomad agent version to the node-status CLI output. #3002

merged 2 commits into from
Aug 22, 2017

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Aug 10, 2017

The Nomad agent version will now be displayed when running the nomad node-status command which brings the command inline with the nomad server-members command which already displays the version information.

Closes #2993

@@ -996,6 +996,9 @@ type Node struct {
// together for the purpose of determining scheduling pressure.
NodeClass string

// Build represents the Nomad version the node is running
Build string
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove from the Node struct and only add to the stub

@@ -1072,6 +1076,7 @@ type NodeListStub struct {
Datacenter string
Name string
NodeClass string
Build string
Copy link
Contributor

Choose a reason for hiding this comment

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

Build -> Version

@@ -1057,6 +1060,7 @@ func (n *Node) Stub() *NodeListStub {
Datacenter: n.Datacenter,
Name: n.Name,
NodeClass: n.NodeClass,
Build: n.Build,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can pull this out of the attributes map. The key is nomad.version

@@ -149,9 +149,9 @@ func (c *NodeStatusCommand) Run(args []string) int {
// Format the nodes list
out := make([]string, len(nodes)+1)
if c.list_allocs {
out[0] = "ID|DC|Name|Class|Drain|Status|Running Allocs"
out[0] = "ID|DC|Name|Class|Build|Drain|Status|Running Allocs"
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep it for only -verbose. Most people aren't running mixed clusters so it will just needlessly expand the width of output

Copy link
Member Author

@jrasell jrasell Aug 11, 2017

Choose a reason for hiding this comment

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

Any thoughts on how to best accomplish this @dadgar? From my understanding verbose is currently only used to determine the length of the nodeID displayed. I am trying to avoid using numerous else ifs to reconcile the differences between the -allocs and -verbose flag usage:

if c.list_allocs && !c.verbose {
    out[0] = "ID|DC|Name|Class|Drain|Status|Running Allocs"
} else if c.list_allocs && c.verbose {
    out[0] = "ID|DC|Name|Class|Build|Drain|Status|Running Allocs"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, you may want to try breaking it into:

out := "ID|DC|Name|Class|"
if c.verbose {
  out += "Version|"
}
out += "Drain|Status"
if c.list_allocs {
  out += "|Running Allocs"
}

@jrasell
Copy link
Member Author

jrasell commented Aug 16, 2017

Output examples:
$ ./jrasell-nomad node-status
ID        DC   Name           Class   Drain  Status
f88d3097  dc1  voyager.local  <none>  false  ready

$ ./jrasell-nomad node-status -verbose
ID                                    DC   Name           Class   Version   Drain  Status
f88d3097-7498-e7a7-15e2-5a17b4de10b2  dc1  voyager.local  <none>  0.6.0dev  false  ready

$ ./jrasell-nomad node-status -verbose -allocs
ID                                    DC   Name           Class   Version   Drain  Status  Running Allocs
f88d3097-7498-e7a7-15e2-5a17b4de10b2  dc1  voyager.local  <none>  0.6.0dev  false  ready   0

$ ./jrasell-nomad node-status -allocs
ID        DC   Name           Class   Drain  Status  Running Allocs
f88d3097  dc1  voyager.local  <none>  false  ready   0

@jrasell
Copy link
Member Author

jrasell commented Aug 16, 2017

I don't think the failure is related to this change.

@jrasell
Copy link
Member Author

jrasell commented Aug 22, 2017

@dadgar anything else I can do to get this merged in?

@dadgar
Copy link
Contributor

dadgar commented Aug 22, 2017

Sorry for the delay! LGTM

@dadgar dadgar merged commit 7a6ccf1 into hashicorp:master Aug 22, 2017
@hsmade
Copy link
Contributor

hsmade commented Aug 23, 2017

thx @jrasell and @dadgar !

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants