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.

Sign up for GitHub

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

Jump to bottom

Don't exec uname for node attribute kernel.version #2380

Merged
merged 2 commits into from
Mar 1, 2017
Merged

Don't exec uname for node attribute kernel.version #2380

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5875bc1
Don't exec uname for node attribute kernel.version
Daviey Mar 1, 2017
5d484dc
Enable kernel.version on Windows
Daviey Mar 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ IMPROVEMENTS:
* client: Don't force uppercase meta keys in env vars [GH-2338]
* client: Reproducible Node ID on OSes that provide system-level UUID
[GH-2277]
* client: Don't exec `uname -r` for node attribute kernel.version [GH-2380]
* driver/docker: Add support for volume drivers [GH-2351]
* driver/docker: Docker image coordinator and caching [GH-2361]
* jobspec: Add leader task to allow graceful shutdown of other tasks within
Expand Down
9 changes: 1 addition & 8 deletions client/fingerprint/host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
package fingerprint

import (
"fmt"
"log"
"os/exec"
"runtime"
"strings"

"github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -38,11 +35,7 @@ func (f *HostFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (b
node.Attributes["kernel.version"] = ""

if runtime.GOOS != "windows" {
Copy link
Member

Choose a reason for hiding this comment

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

KernelVersion will be an empty string on platforms without support so you can remove this guard and set kernel.version above directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, i left it there because I assumed it was not an interesting value on Windows, but happy enable it.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I doubt it will ever be interesting on Windows but the fewer runtime.GOOS checks we have the happier I am! :)

Who knows, maybe someday gopsutils will populate a meaningful value for Windows.

out, err := exec.Command("uname", "-r").Output()
if err != nil {
return false, fmt.Errorf("Failed to run uname: %s", err)
}
node.Attributes["kernel.version"] = strings.Trim(string(out), "\n")
node.Attributes["kernel.version"] = hostInfo.KernelVersion
}

node.Attributes["unique.hostname"] = hostInfo.Hostname
Expand Down