From 5e8151d700654e433d4d1ef5c48be180796ce651 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Wed, 24 Jan 2018 09:09:53 -0500 Subject: [PATCH] refactor Fingerprint to request/response construct --- client/client.go | 83 +++++++++-- client/driver/docker.go | 22 ++- client/driver/docker_test.go | 35 +++-- client/driver/exec_default.go | 7 +- client/driver/exec_linux.go | 17 +-- client/driver/exec_test.go | 16 ++- client/driver/java.go | 27 ++-- client/driver/java_test.go | 19 ++- client/driver/lxc.go | 17 +-- client/driver/lxc_test.go | 47 +++++-- client/driver/mock_driver.go | 7 +- client/driver/qemu.go | 15 +- client/driver/qemu_test.go | 33 +++-- client/driver/raw_exec.go | 15 +- client/driver/raw_exec_test.go | 26 ++-- client/driver/rkt.go | 24 ++-- client/driver/rkt_nonlinux.go | 5 +- client/driver/rkt_test.go | 21 ++- client/fingerprint/arch.go | 9 +- client/fingerprint/arch_test.go | 17 ++- client/fingerprint/cgroup.go | 7 +- client/fingerprint/cgroup_linux.go | 17 ++- client/fingerprint/cgroup_test.go | 132 +++++++++-------- client/fingerprint/consul.go | 45 +++--- client/fingerprint/consul_test.go | 45 +++--- client/fingerprint/cpu.go | 32 ++--- client/fingerprint/cpu_test.go | 80 +++++++---- client/fingerprint/env_aws.go | 39 +++-- client/fingerprint/env_aws_test.go | 188 +++++++++++++++---------- client/fingerprint/env_gce.go | 38 +++-- client/fingerprint/env_gce_test.go | 69 +++++---- client/fingerprint/fingerprint.go | 7 +- client/fingerprint/fingerprint_test.go | 41 ++++-- client/fingerprint/host.go | 19 ++- client/fingerprint/host_test.go | 17 ++- client/fingerprint/memory.go | 16 +-- client/fingerprint/memory_test.go | 19 ++- client/fingerprint/network.go | 22 ++- client/fingerprint/network_test.go | 98 +++++++++---- client/fingerprint/nomad.go | 11 +- client/fingerprint/nomad_test.go | 20 ++- client/fingerprint/signal.go | 9 +- client/fingerprint/signal_test.go | 4 +- client/fingerprint/storage.go | 29 ++-- client/fingerprint/storage_test.go | 16 +-- client/fingerprint/vault.go | 35 ++--- client/fingerprint/vault_test.go | 24 ++-- client/structs/structs.go | 14 ++ 48 files changed, 909 insertions(+), 646 deletions(-) diff --git a/client/client.go b/client/client.go index 314b9dda4053..005b478f016a 100644 --- a/client/client.go +++ b/client/client.go @@ -23,6 +23,7 @@ import ( "github.com/hashicorp/nomad/client/driver" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/client/stats" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/client/vaultclient" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" @@ -949,15 +950,31 @@ func (c *Client) fingerprint() error { return err } - c.configLock.Lock() - applies, err := f.Fingerprint(c.config, c.config.Node) - c.configLock.Unlock() + request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err = f.Fingerprint(request, response) if err != nil { return err } - if applies { + + // if an attribute should be skipped, remove it from the list which we will + // later apply to the node + for _, e := range skipped { + delete(response.Attributes, e) + } + + for name := range response.Attributes { applied = append(applied, name) } + + // add the diff found from each fingerprinter + c.updateNodeFromFingerprint(response) + p, period := f.Periodic() if p { // TODO: If more periodic fingerprinters are added, then @@ -966,6 +983,7 @@ func (c *Client) fingerprint() error { go c.fingerprintPeriodic(name, f, period) } } + c.logger.Printf("[DEBUG] client: applied fingerprints %v", applied) if len(skipped) != 0 { c.logger.Printf("[DEBUG] client: fingerprint modules skipped due to white/blacklist: %v", skipped) @@ -979,11 +997,21 @@ func (c *Client) fingerprintPeriodic(name string, f fingerprint.Fingerprint, d t for { select { case <-time.After(d): - c.configLock.Lock() - if _, err := f.Fingerprint(c.config, c.config.Node); err != nil { + request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) + + if err != nil { c.logger.Printf("[DEBUG] client: periodic fingerprinting for %v failed: %v", name, err) + } else { + c.updateNodeFromFingerprint(response) } - c.configLock.Unlock() + case <-c.shutdownCh: return } @@ -1017,16 +1045,30 @@ func (c *Client) setupDrivers() error { if err != nil { return err } - c.configLock.Lock() - applies, err := d.Fingerprint(c.config, c.config.Node) - c.configLock.Unlock() + + request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err = d.Fingerprint(request, response) if err != nil { return err } - if applies { + + // remove attributes we are supposed to skip + for _, attr := range skipped { + delete(response.Attributes, attr) + } + + for name := range response.Attributes { avail = append(avail, name) } + c.updateNodeFromFingerprint(response) + p, period := d.Periodic() if p { go c.fingerprintPeriodic(name, d, period) @@ -1035,6 +1077,7 @@ func (c *Client) setupDrivers() error { } c.logger.Printf("[DEBUG] client: available drivers %v", avail) + c.logger.Printf("[DEBUG] client: skipped attributes %v", skipped) if len(skipped) != 0 { c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skipped) @@ -1043,6 +1086,24 @@ func (c *Client) setupDrivers() error { return nil } +// updateNodeFromFingerprint updates the node with the result of +// fingerprinting the node from the diff that was created +func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) { + c.configLock.Lock() + defer c.configLock.Unlock() + for name, val := range response.Attributes { + c.config.Node.Attributes[name] = val + } + + // update node links and resources from the diff created from + // fingerprinting + for name, val := range response.Links { + c.config.Node.Links[name] = val + } + + c.config.Node.Resources.Merge(response.Resources) +} + // retryIntv calculates a retry interval value given the base func (c *Client) retryIntv(base time.Duration) time.Duration { if c.config.DevMode { diff --git a/client/driver/docker.go b/client/driver/docker.go index fb05c646d73d..b2ef185503e4 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -26,7 +26,6 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/client/driver/executor" dstructs "github.com/hashicorp/nomad/client/driver/structs" @@ -476,16 +475,14 @@ func NewDockerDriver(ctx *DriverContext) Driver { return &DockerDriver{DriverContext: *ctx} } -func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { - // Initialize docker API clients +func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { client, _, err := d.dockerClients() if err != nil { if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[INFO] driver.docker: failed to initialize client: %s", err) } - delete(node.Attributes, dockerDriverAttr) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } // This is the first operation taken on the client so we'll try to @@ -493,25 +490,24 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool // Docker isn't available so we'll simply disable the docker driver. env, err := client.Version() if err != nil { - delete(node.Attributes, dockerDriverAttr) if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.docker: could not connect to docker daemon at %s: %s", client.Endpoint(), err) } d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } - node.Attributes[dockerDriverAttr] = "1" - node.Attributes["driver.docker.version"] = env.Get("Version") + resp.Attributes[dockerDriverAttr] = "1" + resp.Attributes["driver.docker.version"] = env.Get("Version") privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false) if privileged { - node.Attributes[dockerPrivilegedConfigOption] = "1" + resp.Attributes[dockerPrivilegedConfigOption] = "1" } // Advertise if this node supports Docker volumes if d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) { - node.Attributes["driver."+dockerVolumesConfigOption] = "1" + resp.Attributes["driver."+dockerVolumesConfigOption] = "1" } // Detect bridge IP address - #2785 @@ -529,7 +525,7 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool } if n.IPAM.Config[0].Gateway != "" { - node.Attributes["driver.docker.bridge_ip"] = n.IPAM.Config[0].Gateway + resp.Attributes["driver.docker.bridge_ip"] = n.IPAM.Config[0].Gateway } else if d.fingerprintSuccess == nil { // Docker 17.09.0-ce dropped the Gateway IP from the bridge network // See https://github.com/moby/moby/issues/32648 @@ -540,7 +536,7 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool } d.fingerprintSuccess = helper.BoolToPtr(true) - return true, nil + return nil } // Validate is used to validate the driver configuration diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index a0fd49209b7a..aea0a8987eb4 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -171,17 +171,28 @@ func TestDockerDriver_Fingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - apply, err := d.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if apply != testutil.DockerIsConnected(t) { + + if testutil.DockerIsConnected(t) && response.Attributes["driver.docker"] == "" { t.Fatalf("Fingerprinter should detect when docker is available") } - if node.Attributes["driver.docker"] != "1" { + + if response.Attributes["driver.docker"] != "1" { t.Log("Docker daemon not available. The remainder of the docker tests will be skipped.") } - t.Logf("Found docker version %s", node.Attributes["driver.docker.version"]) + + t.Logf("Found docker version %s", response.Attributes["driver.docker.version"]) } // TestDockerDriver_Fingerprint_Bridge asserts that if Docker is running we set @@ -210,18 +221,26 @@ func TestDockerDriver_Fingerprint_Bridge(t *testing.T) { conf := testConfig(t) conf.Node = mock.Node() dd := NewDockerDriver(NewDriverContext("", "", conf, conf.Node, testLogger(), nil)) - ok, err := dd.Fingerprint(conf, conf.Node) + + request := &cstructs.FingerprintRequest{Config: conf, Node: conf.Node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err = dd.Fingerprint(request, response) if err != nil { t.Fatalf("error fingerprinting docker: %v", err) } - if !ok { + if response.Attributes["driver.docker"] == "" { t.Fatalf("expected Docker to be enabled but false was returned") } - if found := conf.Node.Attributes["driver.docker.bridge_ip"]; found != expectedAddr { + if found := response.Attributes["driver.docker.bridge_ip"]; found != expectedAddr { t.Fatalf("expected bridge ip %q but found: %q", expectedAddr, found) } - t.Logf("docker bridge ip: %q", conf.Node.Attributes["driver.docker.bridge_ip"]) + t.Logf("docker bridge ip: %q", response.Attributes["driver.docker.bridge_ip"]) } func TestDockerDriver_StartOpen_Wait(t *testing.T) { diff --git a/client/driver/exec_default.go b/client/driver/exec_default.go index 2f1e267870fa..65011c3ddd6a 100644 --- a/client/driver/exec_default.go +++ b/client/driver/exec_default.go @@ -3,12 +3,11 @@ package driver import ( - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper" - "github.com/hashicorp/nomad/nomad/structs" ) -func (d *ExecDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } diff --git a/client/driver/exec_linux.go b/client/driver/exec_linux.go index ab3203a49111..19ac61756f56 100644 --- a/client/driver/exec_linux.go +++ b/client/driver/exec_linux.go @@ -1,9 +1,8 @@ package driver import ( - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper" - "github.com/hashicorp/nomad/nomad/structs" "golang.org/x/sys/unix" ) @@ -13,28 +12,26 @@ const ( execDriverAttr = "driver.exec" ) -func (d *ExecDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { // Only enable if cgroups are available and we are root - if !cgroupsMounted(node) { + if !cgroupsMounted(req.Node) { if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.exec: cgroups unavailable, disabling") } d.fingerprintSuccess = helper.BoolToPtr(false) - delete(node.Attributes, execDriverAttr) - return false, nil + return nil } else if unix.Geteuid() != 0 { if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.exec: must run as root user, disabling") } - delete(node.Attributes, execDriverAttr) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } if d.fingerprintSuccess == nil || !*d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.exec: exec driver is enabled") } - node.Attributes[execDriverAttr] = "1" + resp.Attributes[execDriverAttr] = "1" d.fingerprintSuccess = helper.BoolToPtr(true) - return true, nil + return nil } diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index cd6365b8f154..92b1bebe65b3 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -37,14 +38,19 @@ func TestExecDriver_Fingerprint(t *testing.T) { "unique.cgroup.mountpoint": "/sys/fs/cgroup", }, } - apply, err := d.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") - } - if node.Attributes["driver.exec"] == "" { + if response.Attributes["driver.exec"] == "" { t.Fatalf("missing driver") } } diff --git a/client/driver/java.go b/client/driver/java.go index 8c162e0cdfe6..5736a563db78 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -18,7 +18,6 @@ import ( "github.com/hashicorp/go-plugin" "github.com/mitchellh/mapstructure" - "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/client/driver/executor" dstructs "github.com/hashicorp/nomad/client/driver/structs" @@ -112,15 +111,15 @@ func (d *JavaDriver) Abilities() DriverAbilities { } } -func (d *JavaDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { // Only enable if we are root and cgroups are mounted when running on linux systems. - if runtime.GOOS == "linux" && (syscall.Geteuid() != 0 || !cgroupsMounted(node)) { + if runtime.GOOS == "linux" && (syscall.Geteuid() != 0 || !cgroupsMounted(req.Node)) { if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.java: root privileges and mounted cgroups required on linux, disabling") } - delete(node.Attributes, "driver.java") + resp.Attributes[javaDriverAttr] = "" d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } // Find java version @@ -132,9 +131,9 @@ func (d *JavaDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, err := cmd.Run() if err != nil { // assume Java wasn't found - delete(node.Attributes, javaDriverAttr) + resp.Attributes[javaDriverAttr] = "" d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } // 'java -version' returns output on Stderr typically. @@ -152,9 +151,9 @@ func (d *JavaDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Println("[WARN] driver.java: error parsing Java version information, aborting") } - delete(node.Attributes, javaDriverAttr) + resp.Attributes[javaDriverAttr] = "" d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } // Assume 'java -version' returns 3 lines: @@ -166,13 +165,13 @@ func (d *JavaDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, versionString := info[0] versionString = strings.TrimPrefix(versionString, "java version ") versionString = strings.Trim(versionString, "\"") - node.Attributes[javaDriverAttr] = "1" - node.Attributes["driver.java.version"] = versionString - node.Attributes["driver.java.runtime"] = info[1] - node.Attributes["driver.java.vm"] = info[2] + resp.Attributes[javaDriverAttr] = "1" + resp.Attributes["driver.java.version"] = versionString + resp.Attributes["driver.java.runtime"] = info[1] + resp.Attributes["driver.java.vm"] = info[2] d.fingerprintSuccess = helper.BoolToPtr(true) - return true, nil + return nil } func (d *JavaDriver) Prestart(*ExecContext, *structs.Task) (*PrestartResponse, error) { diff --git a/client/driver/java_test.go b/client/driver/java_test.go index b1f6dc3f166b..cd6e56447a51 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/assert" @@ -49,14 +50,20 @@ func TestJavaDriver_Fingerprint(t *testing.T) { "unique.cgroup.mountpoint": "/sys/fs/cgroups", }, } - apply, err := d.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if apply != javaLocated() { - t.Fatalf("Fingerprinter should detect Java when it is installed") - } - if node.Attributes["driver.java"] != "1" { + + if response.Attributes["driver.java"] != "1" && javaLocated() { if v, ok := osJavaDriverSupport[runtime.GOOS]; v && ok { t.Fatalf("missing java driver") } else { @@ -64,7 +71,7 @@ func TestJavaDriver_Fingerprint(t *testing.T) { } } for _, key := range []string{"driver.java.version", "driver.java.runtime", "driver.java.vm"} { - if node.Attributes[key] == "" { + if response.Attributes[key] == "" { t.Fatalf("missing driver key (%s)", key) } } diff --git a/client/driver/lxc.go b/client/driver/lxc.go index fefb6f2fbd2a..77cf4524050e 100644 --- a/client/driver/lxc.go +++ b/client/driver/lxc.go @@ -14,7 +14,6 @@ import ( "syscall" "time" - "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/client/stats" "github.com/hashicorp/nomad/helper/fields" @@ -184,24 +183,26 @@ func (d *LxcDriver) FSIsolation() cstructs.FSIsolation { } // Fingerprint fingerprints the lxc driver configuration -func (d *LxcDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *LxcDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config + enabled := cfg.ReadBoolDefault(lxcConfigOption, true) if !enabled && !cfg.DevMode { - return false, nil + return nil } version := lxc.Version() if version == "" { - return false, nil + return nil } - node.Attributes["driver.lxc.version"] = version - node.Attributes["driver.lxc"] = "1" + resp.Attributes["driver.lxc.version"] = version + resp.Attributes["driver.lxc"] = "1" // Advertise if this node supports lxc volumes if d.config.ReadBoolDefault(lxcVolumesConfigOption, lxcVolumesConfigDefault) { - node.Attributes["driver."+lxcVolumesConfigOption] = "1" + resp.Attributes["driver."+lxcVolumesConfigOption] = "1" } - return true, nil + return nil } func (d *LxcDriver) Prestart(*ExecContext, *structs.Task) (*PrestartResponse, error) { diff --git a/client/driver/lxc_test.go b/client/driver/lxc_test.go index ddc78193c7d4..2da65d86cad3 100644 --- a/client/driver/lxc_test.go +++ b/client/driver/lxc_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" ctestutil "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -38,23 +39,39 @@ func TestLxcDriver_Fingerprint(t *testing.T) { node := &structs.Node{ Attributes: map[string]string{}, } - apply, err := d.Fingerprint(&config.Config{}, node) - if err != nil { - t.Fatalf("err: %v", err) - } - if !apply { - t.Fatalf("should apply by default") - } - apply, err = d.Fingerprint(&config.Config{Options: map[string]string{lxcConfigOption: "0"}}, node) - if err != nil { - t.Fatalf("err: %v", err) - } - if apply { - t.Fatalf("should not apply with config") + // test with an empty config + { + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) + } } - if node.Attributes["driver.lxc"] == "" { - t.Fatalf("missing driver") + + // test when lxc is enable din the config + { + conf := &config.Config{Options: map[string]string{lxcConfigOption: "1"}} + request := &cstructs.FingerprintRequest{Config: conf, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) + } + if response.Attributes["driver.lxc"] == "" { + t.Fatalf("missing driver") + } } } diff --git a/client/driver/mock_driver.go b/client/driver/mock_driver.go index 492794854268..a4ccc637a3d5 100644 --- a/client/driver/mock_driver.go +++ b/client/driver/mock_driver.go @@ -15,7 +15,6 @@ import ( "github.com/mitchellh/mapstructure" - "github.com/hashicorp/nomad/client/config" dstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/client/fingerprint" cstructs "github.com/hashicorp/nomad/client/structs" @@ -194,9 +193,9 @@ func (m *MockDriver) Validate(map[string]interface{}) error { } // Fingerprint fingerprints a node and returns if MockDriver is enabled -func (m *MockDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { - node.Attributes["driver.mock_driver"] = "1" - return true, nil +func (m *MockDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + resp.Attributes["driver.mock_driver"] = "1" + return nil } // MockDriverHandle is a driver handler which supervises a mock task diff --git a/client/driver/qemu.go b/client/driver/qemu.go index f256c829cd6d..ba3f96d3237a 100644 --- a/client/driver/qemu.go +++ b/client/driver/qemu.go @@ -17,7 +17,6 @@ import ( "github.com/coreos/go-semver/semver" plugin "github.com/hashicorp/go-plugin" - "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/executor" dstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/client/fingerprint" @@ -155,7 +154,7 @@ func (d *QemuDriver) FSIsolation() cstructs.FSIsolation { return cstructs.FSIsolationImage } -func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *QemuDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { bin := "qemu-system-x86_64" if runtime.GOOS == "windows" { // On windows, the "qemu-system-x86_64" command does not respond to the @@ -164,22 +163,20 @@ func (d *QemuDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, } outBytes, err := exec.Command(bin, "--version").Output() if err != nil { - delete(node.Attributes, qemuDriverAttr) - return false, nil + return nil } out := strings.TrimSpace(string(outBytes)) matches := reQemuVersion.FindStringSubmatch(out) if len(matches) != 2 { - delete(node.Attributes, qemuDriverAttr) - return false, fmt.Errorf("Unable to parse Qemu version string: %#v", matches) + return fmt.Errorf("Unable to parse Qemu version string: %#v", matches) } currentQemuVersion := matches[1] - node.Attributes[qemuDriverAttr] = "1" - node.Attributes[qemuDriverVersionAttr] = currentQemuVersion + resp.Attributes[qemuDriverAttr] = "1" + resp.Attributes[qemuDriverVersionAttr] = currentQemuVersion - return true, nil + return nil } func (d *QemuDriver) Prestart(_ *ExecContext, task *structs.Task) (*PrestartResponse, error) { diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go index ebab5dae61f3..523fbf41545d 100644 --- a/client/driver/qemu_test.go +++ b/client/driver/qemu_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -34,17 +35,24 @@ func TestQemuDriver_Fingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - apply, err := d.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") - } - if node.Attributes[qemuDriverAttr] == "" { + + if response.Attributes[qemuDriverAttr] == "" { t.Fatalf("Missing Qemu driver") } - if node.Attributes[qemuDriverVersionAttr] == "" { + + if response.Attributes[qemuDriverVersionAttr] == "" { t.Fatalf("Missing Qemu driver version") } } @@ -164,12 +172,19 @@ func TestQemuDriver_GracefulShutdown(t *testing.T) { defer ctx.AllocDir.Destroy() d := NewQemuDriver(ctx.DriverCtx) - apply, err := d.Fingerprint(&config.Config{}, ctx.DriverCtx.node) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: ctx.DriverCtx.node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") + + for name, value := range response.Attributes { + ctx.DriverCtx.node.Attributes[name] = value } dst := ctx.ExecCtx.TaskDir.Dir diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index 86b4406e9a48..0d1eedf35114 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/client/driver/executor" dstructs "github.com/hashicorp/nomad/client/driver/structs" @@ -92,18 +91,18 @@ func (d *RawExecDriver) FSIsolation() cstructs.FSIsolation { return cstructs.FSIsolationNone } -func (d *RawExecDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *RawExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { // Check that the user has explicitly enabled this executor. - enabled := cfg.ReadBoolDefault(rawExecConfigOption, false) + enabled := req.Config.ReadBoolDefault(rawExecConfigOption, false) - if enabled || cfg.DevMode { + if enabled || req.Config.DevMode { d.logger.Printf("[WARN] driver.raw_exec: raw exec is enabled. Only enable if needed") - node.Attributes[rawExecDriverAttr] = "1" - return true, nil + resp.Attributes[rawExecDriverAttr] = "1" + return nil } - delete(node.Attributes, rawExecDriverAttr) - return false, nil + resp.Attributes[rawExecDriverAttr] = "" + return nil } func (d *RawExecDriver) Prestart(*ExecContext, *structs.Task) (*PrestartResponse, error) { diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index dadaf07382e4..fa0fd7413ad7 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper/testtask" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" @@ -34,27 +35,30 @@ func TestRawExecDriver_Fingerprint(t *testing.T) { // Disable raw exec. cfg := &config.Config{Options: map[string]string{rawExecConfigOption: "false"}} - apply, err := d.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if apply { - t.Fatalf("should not apply") - } - if node.Attributes["driver.raw_exec"] != "" { + + if response.Attributes["driver.raw_exec"] != "" { t.Fatalf("driver incorrectly enabled") } // Enable raw exec. - cfg.Options[rawExecConfigOption] = "true" - apply, err = d.Fingerprint(cfg, node) + request.Config.Options[rawExecConfigOption] = "true" + err = d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") - } - if node.Attributes["driver.raw_exec"] != "1" { + + if response.Attributes["driver.raw_exec"] != "1" { t.Fatalf("driver not enabled") } } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 52c7c91f0a56..8f8d55e7ff57 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -311,31 +311,28 @@ func (d *RktDriver) Abilities() DriverAbilities { } } -func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { // Only enable if we are root when running on non-windows systems. if runtime.GOOS != "windows" && syscall.Geteuid() != 0 { if d.fingerprintSuccess == nil || *d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.rkt: must run as root user, disabling") } - delete(node.Attributes, rktDriverAttr) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } outBytes, err := exec.Command(rktCmd, "version").Output() if err != nil { - delete(node.Attributes, rktDriverAttr) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } out := strings.TrimSpace(string(outBytes)) rktMatches := reRktVersion.FindStringSubmatch(out) appcMatches := reAppcVersion.FindStringSubmatch(out) if len(rktMatches) != 2 || len(appcMatches) != 2 { - delete(node.Attributes, rktDriverAttr) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, fmt.Errorf("Unable to parse Rkt version string: %#v", rktMatches) + return fmt.Errorf("Unable to parse Rkt version string: %#v", rktMatches) } minVersion, _ := version.NewVersion(minRktVersion) @@ -347,21 +344,20 @@ func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, e d.logger.Printf("[WARN] driver.rkt: unsupported rkt version %s; please upgrade to >= %s", currentVersion, minVersion) } - delete(node.Attributes, rktDriverAttr) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + return nil } - node.Attributes[rktDriverAttr] = "1" - node.Attributes["driver.rkt.version"] = rktMatches[1] - node.Attributes["driver.rkt.appc.version"] = appcMatches[1] + resp.Attributes[rktDriverAttr] = "1" + resp.Attributes["driver.rkt.version"] = rktMatches[1] + resp.Attributes["driver.rkt.appc.version"] = appcMatches[1] // Advertise if this node supports rkt volumes if d.config.ReadBoolDefault(rktVolumesConfigOption, rktVolumesConfigDefault) { - node.Attributes["driver."+rktVolumesConfigOption] = "1" + resp.Attributes["driver."+rktVolumesConfigOption] = "1" } d.fingerprintSuccess = helper.BoolToPtr(true) - return true, nil + return nil } func (d *RktDriver) Periodic() (bool, time.Duration) { diff --git a/client/driver/rkt_nonlinux.go b/client/driver/rkt_nonlinux.go index 49ae7268ce2b..13c474def1eb 100644 --- a/client/driver/rkt_nonlinux.go +++ b/client/driver/rkt_nonlinux.go @@ -5,7 +5,6 @@ package driver import ( "time" - "github.com/hashicorp/nomad/client/config" cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -46,8 +45,8 @@ func (RktDriver) FSIsolation() cstructs.FSIsolation { panic("not implemented") } -func (RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { - return false, nil +func (RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + return nil } func (RktDriver) Periodic() (bool, time.Duration) { diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index 3b4646dbfb31..bba0037a7ed1 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -17,6 +17,7 @@ import ( "time" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/assert" @@ -57,20 +58,26 @@ func TestRktDriver_Fingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - apply, err := d.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := d.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") - } - if node.Attributes["driver.rkt"] != "1" { + + if response.Attributes["driver.rkt"] != "1" { t.Fatalf("Missing Rkt driver") } - if node.Attributes["driver.rkt.version"] == "" { + if response.Attributes["driver.rkt.version"] == "" { t.Fatalf("Missing Rkt driver version") } - if node.Attributes["driver.rkt.appc.version"] == "" { + if response.Attributes["driver.rkt.appc.version"] == "" { t.Fatalf("Missing appc version for the Rkt driver") } } diff --git a/client/fingerprint/arch.go b/client/fingerprint/arch.go index 71b5352a8916..59d498b732ef 100644 --- a/client/fingerprint/arch.go +++ b/client/fingerprint/arch.go @@ -4,8 +4,7 @@ import ( "log" "runtime" - client "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) // ArchFingerprint is used to fingerprint the architecture @@ -20,7 +19,7 @@ func NewArchFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *ArchFingerprint) Fingerprint(config *client.Config, node *structs.Node) (bool, error) { - node.Attributes["cpu.arch"] = runtime.GOARCH - return true, nil +func (f *ArchFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + resp.Attributes["cpu.arch"] = runtime.GOARCH + return nil } diff --git a/client/fingerprint/arch_test.go b/client/fingerprint/arch_test.go index 4e4b94a67869..269b141dbca9 100644 --- a/client/fingerprint/arch_test.go +++ b/client/fingerprint/arch_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -12,14 +13,20 @@ func TestArchFingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") - } - if node.Attributes["cpu.arch"] == "" { + + if response.Attributes["cpu.arch"] == "" { t.Fatalf("missing arch") } } diff --git a/client/fingerprint/cgroup.go b/client/fingerprint/cgroup.go index 1ec8d87932d3..5994e7e04c08 100644 --- a/client/fingerprint/cgroup.go +++ b/client/fingerprint/cgroup.go @@ -5,8 +5,6 @@ package fingerprint import ( "log" "time" - - "github.com/hashicorp/nomad/nomad/structs" ) const ( @@ -49,8 +47,9 @@ func NewCGroupFingerprint(logger *log.Logger) Fingerprint { // clearCGroupAttributes clears any node attributes related to cgroups that might // have been set in a previous fingerprint run. -func (f *CGroupFingerprint) clearCGroupAttributes(n *structs.Node) { - delete(n.Attributes, "unique.cgroup.mountpoint") +func (f *CGroupFingerprint) clearCGroupAttributes(n map[string]string) map[string]string { + n["unique.cgroup.mountpoint"] = "" + return n } // Periodic determines the interval at which the periodic fingerprinter will run. diff --git a/client/fingerprint/cgroup_linux.go b/client/fingerprint/cgroup_linux.go index 9abb959b5980..fd10cf382089 100644 --- a/client/fingerprint/cgroup_linux.go +++ b/client/fingerprint/cgroup_linux.go @@ -5,8 +5,7 @@ package fingerprint import ( "fmt" - client "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/opencontainers/runc/libcontainer/cgroups" ) @@ -28,30 +27,30 @@ func FindCgroupMountpointDir() (string, error) { } // Fingerprint tries to find a valid cgroup moint point -func (f *CGroupFingerprint) Fingerprint(cfg *client.Config, node *structs.Node) (bool, error) { +func (f *CGroupFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { mount, err := f.mountPointDetector.MountPoint() if err != nil { - f.clearCGroupAttributes(node) - return false, fmt.Errorf("Failed to discover cgroup mount point: %s", err) + resp.Attributes = f.clearCGroupAttributes(resp.Attributes) + return fmt.Errorf("Failed to discover cgroup mount point: %s", err) } // Check if a cgroup mount point was found if mount == "" { // Clear any attributes from the previous fingerprint. - f.clearCGroupAttributes(node) + resp.Attributes = f.clearCGroupAttributes(resp.Attributes) if f.lastState == cgroupAvailable { f.logger.Printf("[INFO] fingerprint.cgroups: cgroups are unavailable") } f.lastState = cgroupUnavailable - return true, nil + return nil } - node.Attributes["unique.cgroup.mountpoint"] = mount + resp.Attributes["unique.cgroup.mountpoint"] = mount if f.lastState == cgroupUnavailable { f.logger.Printf("[INFO] fingerprint.cgroups: cgroups are available") } f.lastState = cgroupAvailable - return true, nil + return nil } diff --git a/client/fingerprint/cgroup_test.go b/client/fingerprint/cgroup_test.go index f1237940576b..c7cf2f775668 100644 --- a/client/fingerprint/cgroup_test.go +++ b/client/fingerprint/cgroup_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -39,64 +40,85 @@ func (m *MountPointDetectorEmptyMountPoint) MountPoint() (string, error) { } func TestCGroupFingerprint(t *testing.T) { - f := &CGroupFingerprint{ - logger: testLogger(), - lastState: cgroupUnavailable, - mountPointDetector: &MountPointDetectorMountPointFail{}, + { + f := &CGroupFingerprint{ + logger: testLogger(), + lastState: cgroupUnavailable, + mountPointDetector: &MountPointDetectorMountPointFail{}, + } + + node := &structs.Node{ + Attributes: make(map[string]string), + } + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) + if err == nil { + t.Fatalf("expected an error") + } + + if a, _ := response.Attributes["unique.cgroup.mountpoint"]; a != "" { + t.Fatalf("unexpected attribute found, %s", a) + } } - node := &structs.Node{ - Attributes: make(map[string]string), + { + f := &CGroupFingerprint{ + logger: testLogger(), + lastState: cgroupUnavailable, + mountPointDetector: &MountPointDetectorValidMountPoint{}, + } + + node := &structs.Node{ + Attributes: make(map[string]string), + } + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("unexpected error, %s", err) + } + if a, ok := response.Attributes["unique.cgroup.mountpoint"]; !ok { + t.Fatalf("unable to find attribute: %s", a) + } } - ok, err := f.Fingerprint(&config.Config{}, node) - if err == nil { - t.Fatalf("expected an error") - } - if ok { - t.Fatalf("should not apply") - } - if a, ok := node.Attributes["unique.cgroup.mountpoint"]; ok { - t.Fatalf("unexpected attribute found, %s", a) - } - - f = &CGroupFingerprint{ - logger: testLogger(), - lastState: cgroupUnavailable, - mountPointDetector: &MountPointDetectorValidMountPoint{}, - } - - node = &structs.Node{ - Attributes: make(map[string]string), - } - - ok, err = f.Fingerprint(&config.Config{}, node) - if err != nil { - t.Fatalf("unexpected error, %s", err) - } - if !ok { - t.Fatalf("should apply") - } - assertNodeAttributeContains(t, node, "unique.cgroup.mountpoint") - - f = &CGroupFingerprint{ - logger: testLogger(), - lastState: cgroupUnavailable, - mountPointDetector: &MountPointDetectorEmptyMountPoint{}, - } - - node = &structs.Node{ - Attributes: make(map[string]string), - } - - ok, err = f.Fingerprint(&config.Config{}, node) - if err != nil { - t.Fatalf("unexpected error, %s", err) - } - if !ok { - t.Fatalf("should apply") - } - if a, ok := node.Attributes["unique.cgroup.mountpoint"]; ok { - t.Fatalf("unexpected attribute found, %s", a) + { + f := &CGroupFingerprint{ + logger: testLogger(), + lastState: cgroupUnavailable, + mountPointDetector: &MountPointDetectorEmptyMountPoint{}, + } + + node := &structs.Node{ + Attributes: make(map[string]string), + } + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("unexpected error, %s", err) + } + if a, _ := response.Attributes["unique.cgroup.mountpoint"]; a != "" { + t.Fatalf("unexpected attribute found, %s", a) + } } } diff --git a/client/fingerprint/consul.go b/client/fingerprint/consul.go index b3790a417eb0..6566cefd9779 100644 --- a/client/fingerprint/consul.go +++ b/client/fingerprint/consul.go @@ -8,7 +8,7 @@ import ( consul "github.com/hashicorp/consul/api" - client "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -29,23 +29,18 @@ func NewConsulFingerprint(logger *log.Logger) Fingerprint { return &ConsulFingerprint{logger: logger, lastState: consulUnavailable} } -func (f *ConsulFingerprint) Fingerprint(config *client.Config, node *structs.Node) (bool, error) { - // Guard against uninitialized Links - if node.Links == nil { - node.Links = map[string]string{} - } - +func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { // Only create the client once to avoid creating too many connections to // Consul. if f.client == nil { - consulConfig, err := config.ConsulConfig.ApiConfig() + consulConfig, err := req.Config.ConsulConfig.ApiConfig() if err != nil { - return false, fmt.Errorf("Failed to initialize the Consul client config: %v", err) + return fmt.Errorf("Failed to initialize the Consul client config: %v", err) } f.client, err = consul.NewClient(consulConfig) if err != nil { - return false, fmt.Errorf("Failed to initialize consul client: %s", err) + return fmt.Errorf("Failed to initialize consul client: %s", err) } } @@ -53,8 +48,7 @@ func (f *ConsulFingerprint) Fingerprint(config *client.Config, node *structs.Nod // If we can't hit this URL consul is probably not running on this machine. info, err := f.client.Agent().Self() if err != nil { - // Clear any attributes set by a previous fingerprint. - f.clearConsulAttributes(node) + // TODO this should set consul in the response if the error is not nil // Print a message indicating that the Consul Agent is not available // anymore @@ -62,39 +56,39 @@ func (f *ConsulFingerprint) Fingerprint(config *client.Config, node *structs.Nod f.logger.Printf("[INFO] fingerprint.consul: consul agent is unavailable") } f.lastState = consulUnavailable - return false, nil + return nil } if s, ok := info["Config"]["Server"].(bool); ok { - node.Attributes["consul.server"] = strconv.FormatBool(s) + resp.Attributes["consul.server"] = strconv.FormatBool(s) } else { f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.server") } if v, ok := info["Config"]["Version"].(string); ok { - node.Attributes["consul.version"] = v + resp.Attributes["consul.version"] = v } else { f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.version") } if r, ok := info["Config"]["Revision"].(string); ok { - node.Attributes["consul.revision"] = r + resp.Attributes["consul.revision"] = r } else { f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.revision") } if n, ok := info["Config"]["NodeName"].(string); ok { - node.Attributes["unique.consul.name"] = n + resp.Attributes["unique.consul.name"] = n } else { f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint unique.consul.name") } if d, ok := info["Config"]["Datacenter"].(string); ok { - node.Attributes["consul.datacenter"] = d + resp.Attributes["consul.datacenter"] = d } else { f.logger.Printf("[WARN] fingerprint.consul: unable to fingerprint consul.datacenter") } - if node.Attributes["consul.datacenter"] != "" || node.Attributes["unique.consul.name"] != "" { - node.Links["consul"] = fmt.Sprintf("%s.%s", - node.Attributes["consul.datacenter"], - node.Attributes["unique.consul.name"]) + if resp.Attributes["consul.datacenter"] != "" || resp.Attributes["unique.consul.name"] != "" { + resp.Links["consul"] = fmt.Sprintf("%s.%s", + resp.Attributes["consul.datacenter"], + resp.Attributes["unique.consul.name"]) } else { f.logger.Printf("[WARN] fingerprint.consul: malformed Consul response prevented linking") } @@ -105,17 +99,12 @@ func (f *ConsulFingerprint) Fingerprint(config *client.Config, node *structs.Nod f.logger.Printf("[INFO] fingerprint.consul: consul agent is available") } f.lastState = consulAvailable - return true, nil + return nil } // clearConsulAttributes removes consul attributes and links from the passed // Node. func (f *ConsulFingerprint) clearConsulAttributes(n *structs.Node) { - delete(n.Attributes, "consul.server") - delete(n.Attributes, "consul.version") - delete(n.Attributes, "consul.revision") - delete(n.Attributes, "unique.consul.name") - delete(n.Attributes, "consul.datacenter") delete(n.Links, "consul") } diff --git a/client/fingerprint/consul_test.go b/client/fingerprint/consul_test.go index e2eecf4385c0..5240b782b5a8 100644 --- a/client/fingerprint/consul_test.go +++ b/client/fingerprint/consul_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/stretchr/testify/assert" ) @@ -24,24 +25,28 @@ func TestConsulFingerprint(t *testing.T) { })) defer ts.Close() - config := config.DefaultConfig() - config.ConsulConfig.Addr = strings.TrimPrefix(ts.URL, "http://") + conf := config.DefaultConfig() + conf.ConsulConfig.Addr = strings.TrimPrefix(ts.URL, "http://") - ok, err := fp.Fingerprint(config, node) + request := &cstructs.FingerprintRequest{Config: conf, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := fp.Fingerprint(request, response) if err != nil { t.Fatalf("Failed to fingerprint: %s", err) } - if !ok { - t.Fatalf("Failed to apply node attributes") - } - assertNodeAttributeContains(t, node, "consul.server") - assertNodeAttributeContains(t, node, "consul.version") - assertNodeAttributeContains(t, node, "consul.revision") - assertNodeAttributeContains(t, node, "unique.consul.name") - assertNodeAttributeContains(t, node, "consul.datacenter") + assertNodeAttributeContains(t, response.Attributes, "consul.server") + assertNodeAttributeContains(t, response.Attributes, "consul.version") + assertNodeAttributeContains(t, response.Attributes, "consul.revision") + assertNodeAttributeContains(t, response.Attributes, "unique.consul.name") + assertNodeAttributeContains(t, response.Attributes, "consul.datacenter") - if _, ok := node.Links["consul"]; !ok { + if _, ok := response.Links["consul"]; !ok { t.Errorf("Expected a link to consul, none found") } } @@ -177,12 +182,18 @@ func TestConsulFingerprint_UnexpectedResponse(t *testing.T) { })) defer ts.Close() - config := config.DefaultConfig() - config.ConsulConfig.Addr = strings.TrimPrefix(ts.URL, "http://") + conf := config.DefaultConfig() + conf.ConsulConfig.Addr = strings.TrimPrefix(ts.URL, "http://") + + request := &cstructs.FingerprintRequest{Config: conf, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } - ok, err := fp.Fingerprint(config, node) + err := fp.Fingerprint(request, response) assert.Nil(err) - assert.True(ok) attrs := []string{ "consul.server", @@ -192,7 +203,7 @@ func TestConsulFingerprint_UnexpectedResponse(t *testing.T) { "consul.datacenter", } for _, attr := range attrs { - if v, ok := node.Attributes[attr]; ok { + if v, ok := response.Attributes[attr]; ok { t.Errorf("unexpected node attribute %q with vlaue %q", attr, v) } } diff --git a/client/fingerprint/cpu.go b/client/fingerprint/cpu.go index 0c9b4c25d0ff..69dad8956093 100644 --- a/client/fingerprint/cpu.go +++ b/client/fingerprint/cpu.go @@ -4,9 +4,8 @@ import ( "fmt" "log" - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/helper/stats" - "github.com/hashicorp/nomad/nomad/structs" ) // CPUFingerprint is used to fingerprint the CPU @@ -21,13 +20,10 @@ func NewCPUFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *CPUFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *CPUFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config setResources := func(totalCompute int) { - if node.Resources == nil { - node.Resources = &structs.Resources{} - } - - node.Resources.CPU = totalCompute + resp.Resources.CPU = totalCompute } if err := stats.Init(); err != nil { @@ -36,20 +32,20 @@ func (f *CPUFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bo if cfg.CpuCompute != 0 { setResources(cfg.CpuCompute) - return true, nil + return nil } if modelName := stats.CPUModelName(); modelName != "" { - node.Attributes["cpu.modelname"] = modelName + resp.Attributes["cpu.modelname"] = modelName } if mhz := stats.CPUMHzPerCore(); mhz > 0 { - node.Attributes["cpu.frequency"] = fmt.Sprintf("%.0f", mhz) + resp.Attributes["cpu.frequency"] = fmt.Sprintf("%.0f", mhz) f.logger.Printf("[DEBUG] fingerprint.cpu: frequency: %.0f MHz", mhz) } if numCores := stats.CPUNumCores(); numCores > 0 { - node.Attributes["cpu.numcores"] = fmt.Sprintf("%d", numCores) + resp.Attributes["cpu.numcores"] = fmt.Sprintf("%d", numCores) f.logger.Printf("[DEBUG] fingerprint.cpu: core count: %d", numCores) } @@ -62,17 +58,13 @@ func (f *CPUFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bo // Return an error if no cpu was detected or explicitly set as this // node would be unable to receive any allocations. if tt == 0 { - return false, fmt.Errorf("cannot detect cpu total compute. "+ + return fmt.Errorf("cannot detect cpu total compute. "+ "CPU compute must be set manually using the client config option %q", "cpu_total_compute") } - node.Attributes["cpu.totalcompute"] = fmt.Sprintf("%d", tt) - - if node.Resources == nil { - node.Resources = &structs.Resources{} - } + resp.Attributes["cpu.totalcompute"] = fmt.Sprintf("%d", tt) + resp.Resources.CPU = tt - node.Resources.CPU = tt - return true, nil + return nil } diff --git a/client/fingerprint/cpu_test.go b/client/fingerprint/cpu_test.go index 238d55770b16..e227d3c90013 100644 --- a/client/fingerprint/cpu_test.go +++ b/client/fingerprint/cpu_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -12,30 +13,35 @@ func TestCPUFingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") - } // CPU info - if node.Attributes["cpu.numcores"] == "" { + if response.Attributes["cpu.numcores"] == "" { t.Fatalf("Missing Num Cores") } - if node.Attributes["cpu.modelname"] == "" { + if response.Attributes["cpu.modelname"] == "" { t.Fatalf("Missing Model Name") } - if node.Attributes["cpu.frequency"] == "" { + if response.Attributes["cpu.frequency"] == "" { t.Fatalf("Missing CPU Frequency") } - if node.Attributes["cpu.totalcompute"] == "" { + if response.Attributes["cpu.totalcompute"] == "" { t.Fatalf("Missing CPU Total Compute") } - if node.Resources == nil || node.Resources.CPU == 0 { + if response.Resources == nil || response.Resources.CPU == 0 { t.Fatalf("Expected to find CPU Resources") } @@ -49,30 +55,44 @@ func TestCPUFingerprint_OverrideCompute(t *testing.T) { Attributes: make(map[string]string), } cfg := &config.Config{} - ok, err := f.Fingerprint(cfg, node) - if err != nil { - t.Fatalf("err: %v", err) - } - if !ok { - t.Fatalf("should apply") - } - - // Get actual system CPU - origCPU := node.Resources.CPU + var originalCPU int - // Override it with a setting - cfg.CpuCompute = origCPU + 123 + { + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) + } + if response.Resources.CPU == 0 { + t.Fatalf("expected fingerprint of cpu of but found 0") + } - // Make sure the Fingerprinter applies the override - ok, err = f.Fingerprint(cfg, node) - if err != nil { - t.Fatalf("err: %v", err) - } - if !ok { - t.Fatalf("should apply") + originalCPU = response.Resources.CPU } - if node.Resources.CPU != cfg.CpuCompute { - t.Fatalf("expected override cpu of %d but found %d", cfg.CpuCompute, node.Resources.CPU) + { + // Override it with a setting + cfg.CpuCompute = originalCPU + 123 + + // Make sure the Fingerprinter applies the override to the node resources + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) + } + + if response.Resources.CPU != cfg.CpuCompute { + t.Fatalf("expected override cpu of %d but found %d", cfg.CpuCompute, response.Resources.CPU) + } } } diff --git a/client/fingerprint/env_aws.go b/client/fingerprint/env_aws.go index b442f49ff30a..5d94c999886a 100644 --- a/client/fingerprint/env_aws.go +++ b/client/fingerprint/env_aws.go @@ -12,7 +12,7 @@ import ( "time" "github.com/hashicorp/go-cleanhttp" - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -63,14 +63,16 @@ func NewEnvAWSFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *EnvAWSFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *EnvAWSFingerprint) Fingerprint(request *cstructs.FingerprintRequest, response *cstructs.FingerprintResponse) error { + cfg := request.Config + // Check if we should tighten the timeout if cfg.ReadBoolDefault(TightenNetworkTimeoutsConfig, false) { f.timeout = 1 * time.Millisecond } if !f.isAWS() { - return false, nil + return nil } // newNetwork is populated and addded to the Nodes resources @@ -78,9 +80,6 @@ func (f *EnvAWSFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) Device: "eth0", } - if node.Links == nil { - node.Links = make(map[string]string) - } metadataURL := os.Getenv("AWS_ENV_URL") if metadataURL == "" { metadataURL = DEFAULT_AWS_URL @@ -115,10 +114,10 @@ func (f *EnvAWSFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) // if it's a URL error, assume we're not in an AWS environment // TODO: better way to detect AWS? Check xen virtualization? if _, ok := err.(*url.Error); ok { - return false, nil + return nil } // not sure what other errors it would return - return false, err + return err } resp, err := ioutil.ReadAll(res.Body) res.Body.Close() @@ -132,12 +131,12 @@ func (f *EnvAWSFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) key = structs.UniqueNamespace(key) } - node.Attributes[key] = strings.Trim(string(resp), "\n") + response.Attributes[key] = strings.Trim(string(resp), "\n") } // copy over network specific information - if val := node.Attributes["unique.platform.aws.local-ipv4"]; val != "" { - node.Attributes["unique.network.ip-address"] = val + if val := response.Attributes["unique.platform.aws.local-ipv4"]; val != "" { + response.Attributes["unique.network.ip-address"] = val newNetwork.IP = val newNetwork.CIDR = newNetwork.IP + "/32" } @@ -149,8 +148,8 @@ func (f *EnvAWSFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) } else if throughput == 0 { // Failed to determine speed. Check if the network fingerprint got it found := false - if node.Resources != nil && len(node.Resources.Networks) > 0 { - for _, n := range node.Resources.Networks { + if request.Node.Resources != nil && len(request.Node.Resources.Networks) > 0 { + for _, n := range request.Node.Resources.Networks { if n.IP == newNetwork.IP { throughput = n.MBits found = true @@ -165,19 +164,15 @@ func (f *EnvAWSFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) } } - // populate Node Network Resources - if node.Resources == nil { - node.Resources = &structs.Resources{} - } newNetwork.MBits = throughput - node.Resources.Networks = []*structs.NetworkResource{newNetwork} + response.Resources.Networks = []*structs.NetworkResource{newNetwork} // populate Links - node.Links["aws.ec2"] = fmt.Sprintf("%s.%s", - node.Attributes["platform.aws.placement.availability-zone"], - node.Attributes["unique.platform.aws.instance-id"]) + response.Links["aws.ec2"] = fmt.Sprintf("%s.%s", + response.Attributes["platform.aws.placement.availability-zone"], + response.Attributes["unique.platform.aws.instance-id"]) - return true, nil + return nil } func (f *EnvAWSFingerprint) isAWS() bool { diff --git a/client/fingerprint/env_aws_test.go b/client/fingerprint/env_aws_test.go index 77f7c1dc96a9..1e60516b5437 100644 --- a/client/fingerprint/env_aws_test.go +++ b/client/fingerprint/env_aws_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -19,13 +20,20 @@ func TestEnvAWSFingerprint_nonAws(t *testing.T) { Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if ok { - t.Fatalf("Should be false without test server") + if len(response.Attributes) > 0 { + t.Fatalf("Should not apply") } } @@ -51,13 +59,16 @@ func TestEnvAWSFingerprint_aws(t *testing.T) { defer ts.Close() os.Setenv("AWS_ENV_URL", ts.URL+"/latest/meta-data/") - ok, err := f.Fingerprint(&config.Config{}, node) - if err != nil { - t.Fatalf("err: %v", err) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, } - if !ok { - t.Fatalf("Expected AWS attributes and Links") + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) } keys := []string{ @@ -74,16 +85,16 @@ func TestEnvAWSFingerprint_aws(t *testing.T) { } for _, k := range keys { - assertNodeAttributeContains(t, node, k) + assertNodeAttributeContains(t, response.Attributes, k) } - if len(node.Links) == 0 { + if len(response.Links) == 0 { t.Fatalf("Empty links for Node in AWS Fingerprint test") } // confirm we have at least instance-id and ami-id for _, k := range []string{"aws.ec2"} { - assertNodeLinksContains(t, node, k) + assertNodeLinksContains(t, response.Links, k) } } @@ -171,22 +182,26 @@ func TestNetworkFingerprint_AWS(t *testing.T) { Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") - } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - if node.Resources == nil || len(node.Resources.Networks) == 0 { + if response.Resources == nil || len(response.Resources.Networks) == 0 { t.Fatal("Expected to find Network Resources") } // Test at least the first Network Resource - net := node.Resources.Networks[0] + net := response.Resources.Networks[0] if net.IP == "" { t.Fatal("Expected Network Resource to have an IP") } @@ -217,73 +232,86 @@ func TestNetworkFingerprint_AWS_network(t *testing.T) { os.Setenv("AWS_ENV_URL", ts.URL+"/latest/meta-data/") f := NewEnvAWSFingerprint(testLogger()) - node := &structs.Node{ - Attributes: make(map[string]string), - } + { + node := &structs.Node{ + Attributes: make(map[string]string), + } - cfg := &config.Config{} - ok, err := f.Fingerprint(cfg, node) - if err != nil { - t.Fatalf("err: %v", err) - } - if !ok { - t.Fatalf("should apply") - } + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) + } - if node.Resources == nil || len(node.Resources.Networks) == 0 { - t.Fatal("Expected to find Network Resources") - } + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - // Test at least the first Network Resource - net := node.Resources.Networks[0] - if net.IP == "" { - t.Fatal("Expected Network Resource to have an IP") - } - if net.CIDR == "" { - t.Fatal("Expected Network Resource to have a CIDR") - } - if net.Device == "" { - t.Fatal("Expected Network Resource to have a Device Name") - } - if net.MBits != 1000 { - t.Fatalf("Expected Network Resource to have speed %d; got %d", 1000, net.MBits) + if response.Resources == nil || len(response.Resources.Networks) == 0 { + t.Fatal("Expected to find Network Resources") + } + + // Test at least the first Network Resource + net := response.Resources.Networks[0] + if net.IP == "" { + t.Fatal("Expected Network Resource to have an IP") + } + if net.CIDR == "" { + t.Fatal("Expected Network Resource to have a CIDR") + } + if net.Device == "" { + t.Fatal("Expected Network Resource to have a Device Name") + } + if net.MBits != 1000 { + t.Fatalf("Expected Network Resource to have speed %d; got %d", 1000, net.MBits) + } } // Try again this time setting a network speed in the config - node = &structs.Node{ - Attributes: make(map[string]string), - } + { + node := &structs.Node{ + Attributes: make(map[string]string), + } - cfg.NetworkSpeed = 10 - ok, err = f.Fingerprint(cfg, node) - if err != nil { - t.Fatalf("err: %v", err) - } - if !ok { - t.Fatalf("should apply") - } + cfg := &config.Config{ + NetworkSpeed: 10, + } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) + } - if node.Resources == nil || len(node.Resources.Networks) == 0 { - t.Fatal("Expected to find Network Resources") - } + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - // Test at least the first Network Resource - net = node.Resources.Networks[0] - if net.IP == "" { - t.Fatal("Expected Network Resource to have an IP") - } - if net.CIDR == "" { - t.Fatal("Expected Network Resource to have a CIDR") - } - if net.Device == "" { - t.Fatal("Expected Network Resource to have a Device Name") - } - if net.MBits != 10 { - t.Fatalf("Expected Network Resource to have speed %d; got %d", 10, net.MBits) + if response.Resources == nil || len(response.Resources.Networks) == 0 { + t.Fatal("Expected to find Network Resources") + } + + // Test at least the first Network Resource + net := response.Resources.Networks[0] + if net.IP == "" { + t.Fatal("Expected Network Resource to have an IP") + } + if net.CIDR == "" { + t.Fatal("Expected Network Resource to have a CIDR") + } + if net.Device == "" { + t.Fatal("Expected Network Resource to have a Device Name") + } + if net.MBits != 10 { + t.Fatalf("Expected Network Resource to have speed %d; got %d", 10, net.MBits) + } } } @@ -294,11 +322,19 @@ func TestNetworkFingerprint_notAWS(t *testing.T) { Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if ok { + + if len(response.Attributes) > 0 { t.Fatalf("Should not apply") } } diff --git a/client/fingerprint/env_gce.go b/client/fingerprint/env_gce.go index 83da63486b59..132ddeba0af8 100644 --- a/client/fingerprint/env_gce.go +++ b/client/fingerprint/env_gce.go @@ -14,7 +14,7 @@ import ( "time" "github.com/hashicorp/go-cleanhttp" - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -131,18 +131,16 @@ func checkError(err error, logger *log.Logger, desc string) error { return err } -func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *EnvGCEFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config + // Check if we should tighten the timeout if cfg.ReadBoolDefault(TightenNetworkTimeoutsConfig, false) { f.client.Timeout = 1 * time.Millisecond } if !f.isGCE() { - return false, nil - } - - if node.Links == nil { - node.Links = make(map[string]string) + return nil } // Keys and whether they should be namespaced as unique. Any key whose value @@ -159,7 +157,7 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) for k, unique := range keys { value, err := f.Get(k, false) if err != nil { - return false, checkError(err, f.logger, k) + return checkError(err, f.logger, k) } // assume we want blank entries @@ -167,7 +165,7 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) if unique { key = structs.UniqueNamespace(key) } - node.Attributes[key] = strings.Trim(value, "\n") + resp.Attributes[key] = strings.Trim(value, "\n") } // These keys need everything before the final slash removed to be usable. @@ -178,14 +176,14 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) for k, unique := range keys { value, err := f.Get(k, false) if err != nil { - return false, checkError(err, f.logger, k) + return checkError(err, f.logger, k) } key := "platform.gce." + k if unique { key = structs.UniqueNamespace(key) } - node.Attributes[key] = strings.Trim(lastToken(value), "\n") + resp.Attributes[key] = strings.Trim(lastToken(value), "\n") } // Get internal and external IPs (if they exist) @@ -202,10 +200,10 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) for _, intf := range interfaces { prefix := "platform.gce.network." + lastToken(intf.Network) uniquePrefix := "unique." + prefix - node.Attributes[prefix] = "true" - node.Attributes[uniquePrefix+".ip"] = strings.Trim(intf.Ip, "\n") + resp.Attributes[prefix] = "true" + resp.Attributes[uniquePrefix+".ip"] = strings.Trim(intf.Ip, "\n") for index, accessConfig := range intf.AccessConfigs { - node.Attributes[uniquePrefix+".external-ip."+strconv.Itoa(index)] = accessConfig.ExternalIp + resp.Attributes[uniquePrefix+".external-ip."+strconv.Itoa(index)] = accessConfig.ExternalIp } } } @@ -213,7 +211,7 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) var tagList []string value, err = f.Get("tags", false) if err != nil { - return false, checkError(err, f.logger, "tags") + return checkError(err, f.logger, "tags") } if err := json.Unmarshal([]byte(value), &tagList); err != nil { f.logger.Printf("[WARN] fingerprint.env_gce: Error decoding instance tags: %s", err.Error()) @@ -231,13 +229,13 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) key = fmt.Sprintf("%s%s", attr, tag) } - node.Attributes[key] = "true" + resp.Attributes[key] = "true" } var attrDict map[string]string value, err = f.Get("attributes/", true) if err != nil { - return false, checkError(err, f.logger, "attributes/") + return checkError(err, f.logger, "attributes/") } if err := json.Unmarshal([]byte(value), &attrDict); err != nil { f.logger.Printf("[WARN] fingerprint.env_gce: Error decoding instance attributes: %s", err.Error()) @@ -255,13 +253,13 @@ func (f *EnvGCEFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) key = fmt.Sprintf("%s%s", attr, k) } - node.Attributes[key] = strings.Trim(v, "\n") + resp.Attributes[key] = strings.Trim(v, "\n") } // populate Links - node.Links["gce"] = node.Attributes["unique.platform.gce.id"] + resp.Links["gce"] = resp.Attributes["unique.platform.gce.id"] - return true, nil + return nil } func (f *EnvGCEFingerprint) isGCE() bool { diff --git a/client/fingerprint/env_gce_test.go b/client/fingerprint/env_gce_test.go index 1e339789a224..4c1febd95751 100644 --- a/client/fingerprint/env_gce_test.go +++ b/client/fingerprint/env_gce_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -19,13 +20,20 @@ func TestGCEFingerprint_nonGCE(t *testing.T) { Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if ok { - t.Fatalf("Should be false without test server") + if len(response.Attributes) > 0 { + t.Fatalf("Should have zero attributes without test server") } } @@ -76,13 +84,16 @@ func testFingerprint_GCE(t *testing.T, withExternalIp bool) { os.Setenv("GCE_ENV_URL", ts.URL+"/computeMetadata/v1/instance/") f := NewEnvGCEFingerprint(testLogger()) - ok, err := f.Fingerprint(&config.Config{}, node) - if err != nil { - t.Fatalf("err: %v", err) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, } - if !ok { - t.Fatalf("should apply") + err := f.Fingerprint(request, response) + if err != nil { + t.Fatalf("err: %v", err) } keys := []string{ @@ -100,40 +111,40 @@ func testFingerprint_GCE(t *testing.T, withExternalIp bool) { } for _, k := range keys { - assertNodeAttributeContains(t, node, k) + assertNodeAttributeContains(t, response.Attributes, k) } - if len(node.Links) == 0 { + if len(response.Links) == 0 { t.Fatalf("Empty links for Node in GCE Fingerprint test") } // Make sure Links contains the GCE ID. for _, k := range []string{"gce"} { - assertNodeLinksContains(t, node, k) + assertNodeLinksContains(t, response.Links, k) } - assertNodeAttributeEquals(t, node, "unique.platform.gce.id", "12345") - assertNodeAttributeEquals(t, node, "unique.platform.gce.hostname", "instance-1.c.project.internal") - assertNodeAttributeEquals(t, node, "platform.gce.zone", "us-central1-f") - assertNodeAttributeEquals(t, node, "platform.gce.machine-type", "n1-standard-1") - assertNodeAttributeEquals(t, node, "platform.gce.network.default", "true") - assertNodeAttributeEquals(t, node, "unique.platform.gce.network.default.ip", "10.240.0.5") + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.id", "12345") + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.hostname", "instance-1.c.project.internal") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.zone", "us-central1-f") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.machine-type", "n1-standard-1") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.network.default", "true") + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.network.default.ip", "10.240.0.5") if withExternalIp { - assertNodeAttributeEquals(t, node, "unique.platform.gce.network.default.external-ip.0", "104.44.55.66") - assertNodeAttributeEquals(t, node, "unique.platform.gce.network.default.external-ip.1", "104.44.55.67") - } else if _, ok := node.Attributes["unique.platform.gce.network.default.external-ip.0"]; ok { + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.network.default.external-ip.0", "104.44.55.66") + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.network.default.external-ip.1", "104.44.55.67") + } else if _, ok := response.Attributes["unique.platform.gce.network.default.external-ip.0"]; ok { t.Fatal("unique.platform.gce.network.default.external-ip is set without an external IP") } - assertNodeAttributeEquals(t, node, "platform.gce.scheduling.automatic-restart", "TRUE") - assertNodeAttributeEquals(t, node, "platform.gce.scheduling.on-host-maintenance", "MIGRATE") - assertNodeAttributeEquals(t, node, "platform.gce.cpu-platform", "Intel Ivy Bridge") - assertNodeAttributeEquals(t, node, "platform.gce.tag.abc", "true") - assertNodeAttributeEquals(t, node, "platform.gce.tag.def", "true") - assertNodeAttributeEquals(t, node, "unique.platform.gce.tag.foo", "true") - assertNodeAttributeEquals(t, node, "platform.gce.attr.ghi", "111") - assertNodeAttributeEquals(t, node, "platform.gce.attr.jkl", "222") - assertNodeAttributeEquals(t, node, "unique.platform.gce.attr.bar", "333") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.scheduling.automatic-restart", "TRUE") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.scheduling.on-host-maintenance", "MIGRATE") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.cpu-platform", "Intel Ivy Bridge") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.tag.abc", "true") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.tag.def", "true") + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.tag.foo", "true") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.attr.ghi", "111") + assertNodeAttributeEquals(t, response.Attributes, "platform.gce.attr.jkl", "222") + assertNodeAttributeEquals(t, response.Attributes, "unique.platform.gce.attr.bar", "333") } const GCE_routes = ` diff --git a/client/fingerprint/fingerprint.go b/client/fingerprint/fingerprint.go index 2d6d483b6c53..8a3477f51747 100644 --- a/client/fingerprint/fingerprint.go +++ b/client/fingerprint/fingerprint.go @@ -6,8 +6,7 @@ import ( "sort" "time" - "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) // EmptyDuration is to be used by fingerprinters that are not periodic. @@ -92,8 +91,8 @@ type Factory func(*log.Logger) Fingerprint // many of them can be applied on a particular host. type Fingerprint interface { // Fingerprint is used to update properties of the Node, - // and returns if the fingerprint was applicable and a potential error. - Fingerprint(*config.Config, *structs.Node) (bool, error) + // and returns a diff of updated node attributes and a potential error. + Fingerprint(*cstructs.FingerprintRequest, *cstructs.FingerprintResponse) error // Periodic is a mechanism for the fingerprinter to indicate that it should // be run periodically. The return value is a boolean indicating if it diff --git a/client/fingerprint/fingerprint_test.go b/client/fingerprint/fingerprint_test.go index 7e62440ea6c4..ba6dae096112 100644 --- a/client/fingerprint/fingerprint_test.go +++ b/client/fingerprint/fingerprint_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -15,45 +16,55 @@ func testLogger() *log.Logger { return log.New(os.Stderr, "", log.LstdFlags) } -func assertFingerprintOK(t *testing.T, fp Fingerprint, node *structs.Node) { - ok, err := fp.Fingerprint(new(config.Config), node) +func assertFingerprintOK(t *testing.T, fp Fingerprint, node *structs.Node) *cstructs.FingerprintResponse { + request := &cstructs.FingerprintRequest{Config: new(config.Config), Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := fp.Fingerprint(request, response) if err != nil { t.Fatalf("Failed to fingerprint: %s", err) } - if !ok { + + if len(response.Attributes) == 0 { t.Fatalf("Failed to apply node attributes") } + + return response } -func assertNodeAttributeContains(t *testing.T, node *structs.Node, attribute string) { - actual, found := node.Attributes[attribute] +func assertNodeAttributeContains(t *testing.T, nodeAttributes map[string]string, attribute string) { + actual, found := nodeAttributes[attribute] if !found { - t.Errorf("Expected to find Attribute `%s`\n\n[DEBUG] %#v", attribute, node) + t.Errorf("Expected to find Attribute `%s`\n\n[DEBUG] %#v", attribute, nodeAttributes) return } if actual == "" { - t.Errorf("Expected non-empty Attribute value for `%s`\n\n[DEBUG] %#v", attribute, node) + t.Errorf("Expected non-empty Attribute value for `%s`\n\n[DEBUG] %#v", attribute, nodeAttributes) } } -func assertNodeAttributeEquals(t *testing.T, node *structs.Node, attribute string, expected string) { - actual, found := node.Attributes[attribute] +func assertNodeAttributeEquals(t *testing.T, nodeAttributes map[string]string, attribute string, expected string) { + actual, found := nodeAttributes[attribute] if !found { - t.Errorf("Expected to find Attribute `%s`; unable to check value\n\n[DEBUG] %#v", attribute, node) + t.Errorf("Expected to find Attribute `%s`; unable to check value\n\n[DEBUG] %#v", attribute, nodeAttributes) return } if expected != actual { - t.Errorf("Expected `%s` Attribute to be `%s`, found `%s`\n\n[DEBUG] %#v", attribute, expected, actual, node) + t.Errorf("Expected `%s` Attribute to be `%s`, found `%s`\n\n[DEBUG] %#v", attribute, expected, actual, nodeAttributes) } } -func assertNodeLinksContains(t *testing.T, node *structs.Node, link string) { - actual, found := node.Links[link] +func assertNodeLinksContains(t *testing.T, nodeLinks map[string]string, link string) { + actual, found := nodeLinks[link] if !found { - t.Errorf("Expected to find Link `%s`\n\n[DEBUG] %#v", link, node) + t.Errorf("Expected to find Link `%s`\n\n[DEBUG]", link) return } if actual == "" { - t.Errorf("Expected non-empty Link value for `%s`\n\n[DEBUG] %#v", link, node) + t.Errorf("Expected non-empty Link value for `%s`\n\n[DEBUG]", link) } } diff --git a/client/fingerprint/host.go b/client/fingerprint/host.go index a7b8ed6c8aec..348846b3b399 100644 --- a/client/fingerprint/host.go +++ b/client/fingerprint/host.go @@ -4,8 +4,7 @@ import ( "log" "runtime" - "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/shirou/gopsutil/host" ) @@ -21,20 +20,20 @@ func NewHostFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *HostFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *HostFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { hostInfo, err := host.Info() if err != nil { f.logger.Println("[WARN] Error retrieving host information: ", err) - return false, err + return err } - node.Attributes["os.name"] = hostInfo.Platform - node.Attributes["os.version"] = hostInfo.PlatformVersion + resp.Attributes["os.name"] = hostInfo.Platform + resp.Attributes["os.version"] = hostInfo.PlatformVersion - node.Attributes["kernel.name"] = runtime.GOOS - node.Attributes["kernel.version"] = hostInfo.KernelVersion + resp.Attributes["kernel.name"] = runtime.GOOS + resp.Attributes["kernel.version"] = hostInfo.KernelVersion - node.Attributes["unique.hostname"] = hostInfo.Hostname + resp.Attributes["unique.hostname"] = hostInfo.Hostname - return true, nil + return nil } diff --git a/client/fingerprint/host_test.go b/client/fingerprint/host_test.go index 5a5f0cfbe1ed..e0ef313ab9c3 100644 --- a/client/fingerprint/host_test.go +++ b/client/fingerprint/host_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -12,16 +13,24 @@ func TestHostFingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") + + if len(response.Attributes) == 0 { + t.Fatalf("should generate a diff of node attributes") } // Host info for _, key := range []string{"os.name", "os.version", "unique.hostname", "kernel.name"} { - assertNodeAttributeContains(t, node, key) + assertNodeAttributeContains(t, response.Attributes, key) } } diff --git a/client/fingerprint/memory.go b/client/fingerprint/memory.go index b249bebf5750..b95a84d7d17c 100644 --- a/client/fingerprint/memory.go +++ b/client/fingerprint/memory.go @@ -4,8 +4,7 @@ import ( "fmt" "log" - "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/shirou/gopsutil/mem" ) @@ -23,21 +22,18 @@ func NewMemoryFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *MemoryFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *MemoryFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { memInfo, err := mem.VirtualMemory() if err != nil { f.logger.Printf("[WARN] Error reading memory information: %s", err) - return false, err + return err } if memInfo.Total > 0 { - node.Attributes["memory.totalbytes"] = fmt.Sprintf("%d", memInfo.Total) + resp.Attributes["memory.totalbytes"] = fmt.Sprintf("%d", memInfo.Total) - if node.Resources == nil { - node.Resources = &structs.Resources{} - } - node.Resources.MemoryMB = int(memInfo.Total / 1024 / 1024) + resp.Resources.MemoryMB = int(memInfo.Total / 1024 / 1024) } - return true, nil + return nil } diff --git a/client/fingerprint/memory_test.go b/client/fingerprint/memory_test.go index 44c79c0cb310..3d6b3eaed675 100644 --- a/client/fingerprint/memory_test.go +++ b/client/fingerprint/memory_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -12,20 +13,24 @@ func TestMemoryFingerprint(t *testing.T) { node := &structs.Node{ Attributes: make(map[string]string), } - ok, err := f.Fingerprint(&config.Config{}, node) + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") - } - assertNodeAttributeContains(t, node, "memory.totalbytes") + assertNodeAttributeContains(t, response.Attributes, "memory.totalbytes") - if node.Resources == nil { + if response.Resources == nil { t.Fatalf("Node Resources was nil") } - if node.Resources.MemoryMB == 0 { + if response.Resources.MemoryMB == 0 { t.Errorf("Expected node.Resources.MemoryMB to be non-zero") } diff --git a/client/fingerprint/network.go b/client/fingerprint/network.go index 287fb73593cb..58cfb1b4c160 100644 --- a/client/fingerprint/network.go +++ b/client/fingerprint/network.go @@ -6,7 +6,7 @@ import ( "net" sockaddr "github.com/hashicorp/go-sockaddr" - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -61,19 +61,17 @@ func NewNetworkFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { - if node.Resources == nil { - node.Resources = &structs.Resources{} - } +func (f *NetworkFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config // Find the named interface intf, err := f.findInterface(cfg.NetworkInterface) switch { case err != nil: - return false, fmt.Errorf("Error while detecting network interface during fingerprinting: %v", err) + return fmt.Errorf("Error while detecting network interface during fingerprinting: %v", err) case intf == nil: // No interface could be found - return false, nil + return nil } // Record the throughput of the interface @@ -94,22 +92,20 @@ func (f *NetworkFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) disallowLinkLocal := cfg.ReadBoolDefault(networkDisallowLinkLocalOption, networkDisallowLinkLocalDefault) nwResources, err := f.createNetworkResources(mbits, intf, disallowLinkLocal) if err != nil { - return false, err + return err } - // Add the network resources to the node - node.Resources.Networks = nwResources + resp.Resources.Networks = nwResources for _, nwResource := range nwResources { f.logger.Printf("[DEBUG] fingerprint.network: Detected interface %v with IP: %v", intf.Name, nwResource.IP) } // Deprecated, setting the first IP as unique IP for the node if len(nwResources) > 0 { - node.Attributes["unique.network.ip-address"] = nwResources[0].IP + resp.Attributes["unique.network.ip-address"] = nwResources[0].IP } - // return true, because we have a network connection - return true, nil + return nil } // createNetworkResources creates network resources for every IP diff --git a/client/fingerprint/network_test.go b/client/fingerprint/network_test.go index 78dca0df7cac..8d556fcb2668 100644 --- a/client/fingerprint/network_test.go +++ b/client/fingerprint/network_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -189,28 +190,35 @@ func TestNetworkFingerprint_basic(t *testing.T) { } cfg := &config.Config{NetworkSpeed: 101} - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + if len(response.Attributes) == 0 { t.Fatalf("should apply (HINT: working offline? Set env %q=y", skipOnlineTestsEnvVar) } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := response.Attributes["unique.network.ip-address"] match := net.ParseIP(ip) if match == nil { t.Fatalf("Bad IP match: %s", ip) } - if node.Resources == nil || len(node.Resources.Networks) == 0 { + if response.Resources == nil || len(response.Resources.Networks) == 0 { t.Fatal("Expected to find Network Resources") } // Test at least the first Network Resource - net := node.Resources.Networks[0] + net := response.Resources.Networks[0] if net.IP == "" { t.Fatal("Expected Network Resource to not be empty") } @@ -232,13 +240,20 @@ func TestNetworkFingerprint_default_device_absent(t *testing.T) { } cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "eth0"} - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err == nil { t.Fatalf("err: %v", err) } - if ok { - t.Fatalf("ok: %v", ok) + if len(response.Attributes) != 0 { + t.Fatalf("attributes should be zero but instead are: %v", response.Attributes) } } @@ -249,28 +264,34 @@ func TestNetworkFingerPrint_default_device(t *testing.T) { } cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "lo"} - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + if len(response.Attributes) == 0 { t.Fatalf("should apply") } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := response.Attributes["unique.network.ip-address"] match := net.ParseIP(ip) if match == nil { t.Fatalf("Bad IP match: %s", ip) } - if node.Resources == nil || len(node.Resources.Networks) == 0 { + if response.Resources == nil || len(response.Resources.Networks) == 0 { t.Fatal("Expected to find Network Resources") } // Test at least the first Network Resource - net := node.Resources.Networks[0] + net := response.Resources.Networks[0] if net.IP == "" { t.Fatal("Expected Network Resource to not be empty") } @@ -292,28 +313,31 @@ func TestNetworkFingerPrint_LinkLocal_Allowed(t *testing.T) { } cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "eth3"} - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") - } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := response.Attributes["unique.network.ip-address"] match := net.ParseIP(ip) if match == nil { t.Fatalf("Bad IP match: %s", ip) } - if node.Resources == nil || len(node.Resources.Networks) == 0 { + if response.Resources == nil || len(response.Resources.Networks) == 0 { t.Fatal("Expected to find Network Resources") } // Test at least the first Network Resource - net := node.Resources.Networks[0] + net := response.Resources.Networks[0] if net.IP == "" { t.Fatal("Expected Network Resource to not be empty") } @@ -335,28 +359,34 @@ func TestNetworkFingerPrint_LinkLocal_Allowed_MixedIntf(t *testing.T) { } cfg := &config.Config{NetworkSpeed: 100, NetworkInterface: "eth4"} - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + if len(response.Attributes) == 0 { t.Fatalf("should apply") } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, response.Attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := response.Attributes["unique.network.ip-address"] match := net.ParseIP(ip) if match == nil { t.Fatalf("Bad IP match: %s", ip) } - if node.Resources == nil || len(node.Resources.Networks) == 0 { + if response.Resources == nil || len(response.Resources.Networks) == 0 { t.Fatal("Expected to find Network Resources") } // Test at least the first Network Resource - net := node.Resources.Networks[0] + net := response.Resources.Networks[0] if net.IP == "" { t.Fatal("Expected Network Resource to not be empty") } @@ -387,11 +417,17 @@ func TestNetworkFingerPrint_LinkLocal_Disallowed(t *testing.T) { }, } - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + if len(response.Attributes) != 0 { t.Fatalf("should not apply") } } diff --git a/client/fingerprint/nomad.go b/client/fingerprint/nomad.go index 0db894196fda..21849dc9808e 100644 --- a/client/fingerprint/nomad.go +++ b/client/fingerprint/nomad.go @@ -3,8 +3,7 @@ package fingerprint import ( "log" - client "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) // NomadFingerprint is used to fingerprint the Nomad version @@ -19,8 +18,8 @@ func NewNomadFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *NomadFingerprint) Fingerprint(config *client.Config, node *structs.Node) (bool, error) { - node.Attributes["nomad.version"] = config.Version.VersionNumber() - node.Attributes["nomad.revision"] = config.Version.Revision - return true, nil +func (f *NomadFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + resp.Attributes["nomad.version"] = req.Config.Version.VersionNumber() + resp.Attributes["nomad.revision"] = req.Config.Version.Revision + return nil } diff --git a/client/fingerprint/nomad_test.go b/client/fingerprint/nomad_test.go index 730fc3c5db25..aac47a944e09 100644 --- a/client/fingerprint/nomad_test.go +++ b/client/fingerprint/nomad_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/version" ) @@ -21,17 +22,28 @@ func TestNomadFingerprint(t *testing.T) { Version: v, }, } - ok, err := f.Fingerprint(c, node) + + request := &cstructs.FingerprintRequest{Config: c, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + + err := f.Fingerprint(request, response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + + if len(response.Attributes) == 0 { t.Fatalf("should apply") } - if node.Attributes["nomad.version"] != v { + + if response.Attributes["nomad.version"] != v { t.Fatalf("incorrect version") } - if node.Attributes["nomad.revision"] != r { + + if response.Attributes["nomad.revision"] != r { t.Fatalf("incorrect revision") } } diff --git a/client/fingerprint/signal.go b/client/fingerprint/signal.go index d20cec07b025..d87e91447d86 100644 --- a/client/fingerprint/signal.go +++ b/client/fingerprint/signal.go @@ -5,8 +5,7 @@ import ( "strings" "github.com/hashicorp/consul-template/signals" - "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) // SignalFingerprint is used to fingerprint the available signals @@ -21,13 +20,13 @@ func NewSignalFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *SignalFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *SignalFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { // Build the list of available signals sigs := make([]string, 0, len(signals.SignalLookup)) for signal := range signals.SignalLookup { sigs = append(sigs, signal) } - node.Attributes["os.signals"] = strings.Join(sigs, ",") - return true, nil + resp.Attributes["os.signals"] = strings.Join(sigs, ",") + return nil } diff --git a/client/fingerprint/signal_test.go b/client/fingerprint/signal_test.go index 2157cf0c50e0..bf61f754408f 100644 --- a/client/fingerprint/signal_test.go +++ b/client/fingerprint/signal_test.go @@ -12,6 +12,6 @@ func TestSignalFingerprint(t *testing.T) { Attributes: make(map[string]string), } - assertFingerprintOK(t, fp, node) - assertNodeAttributeContains(t, node, "os.signals") + response := assertFingerprintOK(t, fp, node) + assertNodeAttributeContains(t, response.Attributes, "os.signals") } diff --git a/client/fingerprint/storage.go b/client/fingerprint/storage.go index c60f13154bce..34d9e120209c 100644 --- a/client/fingerprint/storage.go +++ b/client/fingerprint/storage.go @@ -6,8 +6,7 @@ import ( "os" "strconv" - "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) const bytesPerMegabyte = 1024 * 1024 @@ -24,15 +23,13 @@ func NewStorageFingerprint(logger *log.Logger) Fingerprint { return fp } -func (f *StorageFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { +func (f *StorageFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config // Initialize these to empty defaults - node.Attributes["unique.storage.volume"] = "" - node.Attributes["unique.storage.bytestotal"] = "" - node.Attributes["unique.storage.bytesfree"] = "" - if node.Resources == nil { - node.Resources = &structs.Resources{} - } + resp.Attributes["unique.storage.volume"] = "" + resp.Attributes["unique.storage.bytestotal"] = "" + resp.Attributes["unique.storage.bytesfree"] = "" // Guard against unset AllocDir storageDir := cfg.AllocDir @@ -40,20 +37,20 @@ func (f *StorageFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) var err error storageDir, err = os.Getwd() if err != nil { - return false, fmt.Errorf("unable to get CWD from filesystem: %s", err) + return fmt.Errorf("unable to get CWD from filesystem: %s", err) } } volume, total, free, err := f.diskFree(storageDir) if err != nil { - return false, fmt.Errorf("failed to determine disk space for %s: %v", storageDir, err) + return fmt.Errorf("failed to determine disk space for %s: %v", storageDir, err) } - node.Attributes["unique.storage.volume"] = volume - node.Attributes["unique.storage.bytestotal"] = strconv.FormatUint(total, 10) - node.Attributes["unique.storage.bytesfree"] = strconv.FormatUint(free, 10) + resp.Attributes["unique.storage.volume"] = volume + resp.Attributes["unique.storage.bytestotal"] = strconv.FormatUint(total, 10) + resp.Attributes["unique.storage.bytesfree"] = strconv.FormatUint(free, 10) - node.Resources.DiskMB = int(free / bytesPerMegabyte) + resp.Resources.DiskMB = int(free / bytesPerMegabyte) - return true, nil + return nil } diff --git a/client/fingerprint/storage_test.go b/client/fingerprint/storage_test.go index f975aec6d1dc..b244e92e813a 100644 --- a/client/fingerprint/storage_test.go +++ b/client/fingerprint/storage_test.go @@ -13,17 +13,17 @@ func TestStorageFingerprint(t *testing.T) { Attributes: make(map[string]string), } - assertFingerprintOK(t, fp, node) + response := assertFingerprintOK(t, fp, node) - assertNodeAttributeContains(t, node, "unique.storage.volume") - assertNodeAttributeContains(t, node, "unique.storage.bytestotal") - assertNodeAttributeContains(t, node, "unique.storage.bytesfree") + assertNodeAttributeContains(t, response.Attributes, "unique.storage.volume") + assertNodeAttributeContains(t, response.Attributes, "unique.storage.bytestotal") + assertNodeAttributeContains(t, response.Attributes, "unique.storage.bytesfree") - total, err := strconv.ParseInt(node.Attributes["unique.storage.bytestotal"], 10, 64) + total, err := strconv.ParseInt(response.Attributes["unique.storage.bytestotal"], 10, 64) if err != nil { t.Fatalf("Failed to parse unique.storage.bytestotal: %s", err) } - free, err := strconv.ParseInt(node.Attributes["unique.storage.bytesfree"], 10, 64) + free, err := strconv.ParseInt(response.Attributes["unique.storage.bytesfree"], 10, 64) if err != nil { t.Fatalf("Failed to parse unique.storage.bytesfree: %s", err) } @@ -32,10 +32,10 @@ func TestStorageFingerprint(t *testing.T) { t.Fatalf("unique.storage.bytesfree %d is larger than unique.storage.bytestotal %d", free, total) } - if node.Resources == nil { + if response.Resources == nil { t.Fatalf("Node Resources was nil") } - if node.Resources.DiskMB == 0 { + if response.Resources.DiskMB == 0 { t.Errorf("Expected node.Resources.DiskMB to be non-zero") } } diff --git a/client/fingerprint/vault.go b/client/fingerprint/vault.go index a8728fc98c55..d1ec1a50fbe2 100644 --- a/client/fingerprint/vault.go +++ b/client/fingerprint/vault.go @@ -7,8 +7,7 @@ import ( "strings" "time" - client "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" vapi "github.com/hashicorp/vault/api" ) @@ -29,9 +28,11 @@ func NewVaultFingerprint(logger *log.Logger) Fingerprint { return &VaultFingerprint{logger: logger, lastState: vaultUnavailable} } -func (f *VaultFingerprint) Fingerprint(config *client.Config, node *structs.Node) (bool, error) { +func (f *VaultFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + config := req.Config + if config.VaultConfig == nil || !config.VaultConfig.IsEnabled() { - return false, nil + return nil } // Only create the client once to avoid creating too many connections to @@ -39,35 +40,32 @@ func (f *VaultFingerprint) Fingerprint(config *client.Config, node *structs.Node if f.client == nil { vaultConfig, err := config.VaultConfig.ApiConfig() if err != nil { - return false, fmt.Errorf("Failed to initialize the Vault client config: %v", err) + return fmt.Errorf("Failed to initialize the Vault client config: %v", err) } f.client, err = vapi.NewClient(vaultConfig) if err != nil { - return false, fmt.Errorf("Failed to initialize Vault client: %s", err) + return fmt.Errorf("Failed to initialize Vault client: %s", err) } } // Connect to vault and parse its information status, err := f.client.Sys().SealStatus() if err != nil { - // Clear any attributes set by a previous fingerprint. - f.clearVaultAttributes(node) - // Print a message indicating that Vault is not available anymore if f.lastState == vaultAvailable { f.logger.Printf("[INFO] fingerprint.vault: Vault is unavailable") } f.lastState = vaultUnavailable - return false, nil + return nil } - node.Attributes["vault.accessible"] = strconv.FormatBool(true) + resp.Attributes["vault.accessible"] = strconv.FormatBool(true) // We strip the Vault prefix because < 0.6.2 the version looks like: // status.Version = "Vault v0.6.1" - node.Attributes["vault.version"] = strings.TrimPrefix(status.Version, "Vault ") - node.Attributes["vault.cluster_id"] = status.ClusterID - node.Attributes["vault.cluster_name"] = status.ClusterName + resp.Attributes["vault.version"] = strings.TrimPrefix(status.Version, "Vault ") + resp.Attributes["vault.cluster_id"] = status.ClusterID + resp.Attributes["vault.cluster_name"] = status.ClusterName // If Vault was previously unavailable print a message to indicate the Agent // is available now @@ -75,14 +73,7 @@ func (f *VaultFingerprint) Fingerprint(config *client.Config, node *structs.Node f.logger.Printf("[INFO] fingerprint.vault: Vault is available") } f.lastState = vaultAvailable - return true, nil -} - -func (f *VaultFingerprint) clearVaultAttributes(n *structs.Node) { - delete(n.Attributes, "vault.accessible") - delete(n.Attributes, "vault.version") - delete(n.Attributes, "vault.cluster_id") - delete(n.Attributes, "vault.cluster_name") + return nil } func (f *VaultFingerprint) Periodic() (bool, time.Duration) { diff --git a/client/fingerprint/vault_test.go b/client/fingerprint/vault_test.go index a6835b937d3a..70948a0e0407 100644 --- a/client/fingerprint/vault_test.go +++ b/client/fingerprint/vault_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" ) @@ -17,19 +18,22 @@ func TestVaultFingerprint(t *testing.T) { Attributes: make(map[string]string), } - config := config.DefaultConfig() - config.VaultConfig = tv.Config + conf := config.DefaultConfig() + conf.VaultConfig = tv.Config - ok, err := fp.Fingerprint(config, node) + request := &cstructs.FingerprintRequest{Config: conf, Node: node} + response := &cstructs.FingerprintResponse{ + Attributes: make(map[string]string, 0), + Links: make(map[string]string, 0), + Resources: &structs.Resources{}, + } + err := fp.Fingerprint(request, response) if err != nil { t.Fatalf("Failed to fingerprint: %s", err) } - if !ok { - t.Fatalf("Failed to apply node attributes") - } - assertNodeAttributeContains(t, node, "vault.accessible") - assertNodeAttributeContains(t, node, "vault.version") - assertNodeAttributeContains(t, node, "vault.cluster_id") - assertNodeAttributeContains(t, node, "vault.cluster_name") + assertNodeAttributeContains(t, response.Attributes, "vault.accessible") + assertNodeAttributeContains(t, response.Attributes, "vault.version") + assertNodeAttributeContains(t, response.Attributes, "vault.cluster_id") + assertNodeAttributeContains(t, response.Attributes, "vault.cluster_name") } diff --git a/client/structs/structs.go b/client/structs/structs.go index 0673ce951e74..1007bfd8feeb 100644 --- a/client/structs/structs.go +++ b/client/structs/structs.go @@ -4,6 +4,9 @@ import ( "crypto/md5" "io" "strconv" + + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/nomad/structs" ) // MemoryStats holds memory usage related stats @@ -184,3 +187,14 @@ func (d *DriverNetwork) Hash() []byte { } return h.Sum(nil) } + +type FingerprintRequest struct { + Config *config.Config + Node *structs.Node +} + +type FingerprintResponse struct { + Attributes map[string]string + Links map[string]string + Resources *structs.Resources +}