diff --git a/CHANGELOG.md b/CHANGELOG.md index 5602c4fae86..f1b6804dc60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ IMPROVEMENTS: * cli: Use ISO_8601 time format for cli output [[GH-3814](https://github.com/hashicorp/nomad/pull/3814)] * client: Allow '.' in environment variable names [[GH-3760](https://github.com/hashicorp/nomad/issues/3760)] + * client: Refactor client fingerprint methods to a request/response format + [[GH-3781](https://github.com/hashicorp/nomad/issues/3781)] * discovery: Allow `check_restart` to be specified in the `service` stanza. [[GH-3718](https://github.com/hashicorp/nomad/issues/3718)] * driver/docker; Support overriding image entrypoint [[GH-3788](https://github.com/hashicorp/nomad/issues/3788)] diff --git a/client/client.go b/client/client.go index 314b9dda405..4054fca6b10 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" @@ -931,33 +932,42 @@ func (c *Client) fingerprint() error { c.logger.Printf("[DEBUG] client: built-in fingerprints: %v", fingerprint.BuiltinFingerprints()) - var applied []string - var skipped []string + var detectedFingerprints []string + var skippedFingerprints []string for _, name := range fingerprint.BuiltinFingerprints() { // Skip modules that are not in the whitelist if it is enabled. if _, ok := whitelist[name]; whitelistEnabled && !ok { - skipped = append(skipped, name) + skippedFingerprints = append(skippedFingerprints, name) continue } // Skip modules that are in the blacklist if _, ok := blacklist[name]; ok { - skipped = append(skipped, name) + skippedFingerprints = append(skippedFingerprints, name) continue } + f, err := fingerprint.NewFingerprint(name, c.logger) if err != nil { return err } c.configLock.Lock() - applies, err := f.Fingerprint(c.config, c.config.Node) + request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} + var response cstructs.FingerprintResponse + err = f.Fingerprint(request, &response) c.configLock.Unlock() if err != nil { return err } - if applies { - applied = append(applied, name) + + // log the fingerprinters which have been applied + if response.Detected { + detectedFingerprints = append(detectedFingerprints, 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,9 +976,10 @@ 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) + + c.logger.Printf("[DEBUG] client: detected fingerprints %v", detectedFingerprints) + if len(skippedFingerprints) != 0 { + c.logger.Printf("[DEBUG] client: fingerprint modules skipped due to white/blacklist: %v", skippedFingerprints) } return nil } @@ -980,10 +991,17 @@ func (c *Client) fingerprintPeriodic(name string, f fingerprint.Fingerprint, d t 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) + c.configLock.Unlock() + + 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 } @@ -997,19 +1015,19 @@ func (c *Client) setupDrivers() error { whitelistEnabled := len(whitelist) > 0 blacklist := c.config.ReadStringListToMap("driver.blacklist") - var avail []string - var skipped []string + var detectedDrivers []string + var skippedDrivers []string driverCtx := driver.NewDriverContext("", "", c.config, c.config.Node, c.logger, nil) for name := range driver.BuiltinDrivers { // Skip fingerprinting drivers that are not in the whitelist if it is // enabled. if _, ok := whitelist[name]; whitelistEnabled && !ok { - skipped = append(skipped, name) + skippedDrivers = append(skippedDrivers, name) continue } // Skip fingerprinting drivers that are in the blacklist if _, ok := blacklist[name]; ok { - skipped = append(skipped, name) + skippedDrivers = append(skippedDrivers, name) continue } @@ -1017,16 +1035,23 @@ func (c *Client) setupDrivers() error { if err != nil { return err } + c.configLock.Lock() - applies, err := d.Fingerprint(c.config, c.config.Node) + request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node} + var response cstructs.FingerprintResponse + err = d.Fingerprint(request, &response) c.configLock.Unlock() if err != nil { return err } - if applies { - avail = append(avail, name) + + // log the fingerprinters which have been applied + if response.Detected { + detectedDrivers = append(detectedDrivers, name) } + c.updateNodeFromFingerprint(&response) + p, period := d.Periodic() if p { go c.fingerprintPeriodic(name, d, period) @@ -1034,15 +1059,42 @@ func (c *Client) setupDrivers() error { } - c.logger.Printf("[DEBUG] client: available drivers %v", avail) - - if len(skipped) != 0 { - c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skipped) + c.logger.Printf("[DEBUG] client: detected drivers %v", detectedDrivers) + if len(skippedDrivers) > 0 { + c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skippedDrivers) } 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 { + if val == "" { + delete(c.config.Node.Attributes, name) + } else { + c.config.Node.Attributes[name] = val + } + } + + // update node links and resources from the diff created from + // fingerprinting + for name, val := range response.Links { + if val == "" { + delete(c.config.Node.Links, name) + } else { + c.config.Node.Links[name] = val + } + } + + if response.Resources != nil { + 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/client_test.go b/client/client_test.go index 95ff480d357..9b16f93e7a1 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/consul/lib/freeport" memdb "github.com/hashicorp/go-memdb" "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/driver" "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" @@ -252,6 +253,48 @@ func TestClient_HasNodeChanged(t *testing.T) { } } +func TestClient_Fingerprint_Periodic(t *testing.T) { + if _, ok := driver.BuiltinDrivers["mock_driver"]; !ok { + t.Skip(`test requires mock_driver; run with "-tags nomad_test"`) + } + t.Parallel() + + // these constants are only defined when nomad_test is enabled, so these fail + // our linter without explicit disabling. + c1 := testClient(t, func(c *config.Config) { + c.Options = map[string]string{ + driver.ShutdownPeriodicAfter: "true", // nolint: varcheck + driver.ShutdownPeriodicDuration: "3", // nolint: varcheck + } + }) + defer c1.Shutdown() + + node := c1.config.Node + mockDriverName := "driver.mock_driver" + + // Ensure the mock driver is registered on the client + testutil.WaitForResult(func() (bool, error) { + mockDriverStatus := node.Attributes[mockDriverName] + if mockDriverStatus == "" { + return false, fmt.Errorf("mock driver attribute should be set on the client") + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) + + // Ensure that the client fingerprinter eventually removes this attribute + testutil.WaitForResult(func() (bool, error) { + mockDriverStatus := node.Attributes[mockDriverName] + if mockDriverStatus != "" { + return false, fmt.Errorf("mock driver attribute should not be set on the client") + } + return true, nil + }, func(err error) { + t.Fatalf("err: %v", err) + }) +} + func TestClient_Fingerprint_InWhitelist(t *testing.T) { t.Parallel() c := testClient(t, func(c *config.Config) { diff --git a/client/driver/docker.go b/client/driver/docker.go index 5f2dedc29d4..9168495ade8 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" @@ -477,16 +476,15 @@ 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 + resp.RemoveAttribute(dockerDriverAttr) + return nil } // This is the first operation taken on the client so we'll try to @@ -494,25 +492,26 @@ 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 + resp.RemoveAttribute(dockerDriverAttr) + return nil } - node.Attributes[dockerDriverAttr] = "1" - node.Attributes["driver.docker.version"] = env.Get("Version") + resp.AddAttribute(dockerDriverAttr, "1") + resp.AddAttribute("driver.docker.version", env.Get("Version")) + resp.Detected = true privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false) if privileged { - node.Attributes[dockerPrivilegedConfigOption] = "1" + resp.AddAttribute(dockerPrivilegedConfigOption, "1") } // Advertise if this node supports Docker volumes if d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) { - node.Attributes["driver."+dockerVolumesConfigOption] = "1" + resp.AddAttribute("driver."+dockerVolumesConfigOption, "1") } // Detect bridge IP address - #2785 @@ -530,7 +529,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.AddAttribute("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 @@ -541,7 +540,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 6dc5d97c5c1..c7191de80ff 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -171,17 +171,31 @@ 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} + var response cstructs.FingerprintResponse + err := d.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if apply != testutil.DockerIsConnected(t) { + + attributes := response.Attributes + if testutil.DockerIsConnected(t) && attributes["driver.docker"] == "" { t.Fatalf("Fingerprinter should detect when docker is available") } - if node.Attributes["driver.docker"] != "1" { + + if attributes["driver.docker"] != "1" { t.Log("Docker daemon not available. The remainder of the docker tests will be skipped.") + } else { + + // if docker is available, make sure that the response is tagged as + // applicable + if !response.Detected { + t.Fatalf("expected response to be applicable") + } } - t.Logf("Found docker version %s", node.Attributes["driver.docker.version"]) + + t.Logf("Found docker version %s", attributes["driver.docker.version"]) } // TestDockerDriver_Fingerprint_Bridge asserts that if Docker is running we set @@ -210,18 +224,31 @@ 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} + var response cstructs.FingerprintResponse + err = dd.Fingerprint(request, &response) if err != nil { t.Fatalf("error fingerprinting docker: %v", err) } - if !ok { + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + attributes := response.Attributes + if attributes == nil { + t.Fatalf("expected attributes to be set") + } + + if 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 := 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", 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 2f1e267870f..05a609e9d4c 100644 --- a/client/driver/exec_default.go +++ b/client/driver/exec_default.go @@ -3,12 +3,12 @@ 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 + resp.Detected = true + return nil } diff --git a/client/driver/exec_linux.go b/client/driver/exec_linux.go index ab3203a4911..138a92d75dc 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,31 @@ 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 { + // The exec driver will be detected in every case + resp.Detected = true + // 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.logger.Printf("[INFO] driver.exec: cgroups unavailable, disabling") } d.fingerprintSuccess = helper.BoolToPtr(false) - delete(node.Attributes, execDriverAttr) - return false, nil + resp.RemoveAttribute(execDriverAttr) + 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 + resp.RemoveAttribute(execDriverAttr) + return nil } if d.fingerprintSuccess == nil || !*d.fingerprintSuccess { d.logger.Printf("[DEBUG] driver.exec: exec driver is enabled") } - node.Attributes[execDriverAttr] = "1" + resp.AddAttribute(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 cd6365b8f15..d854c56a097 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} + var response cstructs.FingerprintResponse + err := d.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") } - if node.Attributes["driver.exec"] == "" { + + if response.Attributes == nil || response.Attributes["driver.exec"] == "" { t.Fatalf("missing driver") } } diff --git a/client/driver/java.go b/client/driver/java.go index 8c162e0cdfe..3c4b3195837 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,16 @@ 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") + d.logger.Printf("[INFO] driver.java: root privileges and mounted cgroups required on linux, disabling") } - delete(node.Attributes, "driver.java") d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + resp.RemoveAttribute(javaDriverAttr) + resp.Detected = true + return nil } // Find java version @@ -132,9 +132,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) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + resp.RemoveAttribute(javaDriverAttr) + return nil } // 'java -version' returns output on Stderr typically. @@ -152,9 +152,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) d.fingerprintSuccess = helper.BoolToPtr(false) - return false, nil + resp.RemoveAttribute(javaDriverAttr) + return nil } // Assume 'java -version' returns 3 lines: @@ -166,13 +166,14 @@ 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.AddAttribute(javaDriverAttr, "1") + resp.AddAttribute("driver.java.version", versionString) + resp.AddAttribute("driver.java.runtime", info[1]) + resp.AddAttribute("driver.java.vm", info[2]) d.fingerprintSuccess = helper.BoolToPtr(true) + resp.Detected = 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 b1f6dc3f166..f273869a7d7 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,19 @@ 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} + var response cstructs.FingerprintResponse + 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 !response.Detected { + t.Fatalf("expected response to be applicable") } - 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 +70,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 e95063d9175..5f724f98bb7 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,27 @@ 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.AddAttribute("driver.lxc.version", version) + resp.AddAttribute("driver.lxc", "1") + resp.Detected = true // Advertise if this node supports lxc volumes if d.config.ReadBoolDefault(lxcVolumesConfigOption, lxcVolumesConfigDefault) { - node.Attributes["driver."+lxcVolumesConfigOption] = "1" + resp.AddAttribute("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 b1d94cf2083..7b81c63fb47 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,34 @@ 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} + var response cstructs.FingerprintResponse + 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} + var response cstructs.FingerprintResponse + err := d.Fingerprint(request, &response) + if err != nil { + t.Fatalf("err: %v", err) + } + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + if response.Attributes["driver.lxc"] == "" { + t.Fatalf("missing driver") + } } } diff --git a/client/driver/mock_driver.go b/client/driver/mock_driver.go index 49279485426..15cc56b5b41 100644 --- a/client/driver/mock_driver.go +++ b/client/driver/mock_driver.go @@ -15,13 +15,23 @@ 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" "github.com/hashicorp/nomad/nomad/structs" ) +const ( + // ShutdownPeriodicAfter is a config key that can be used during tests to + // "stop" a previously-functioning driver, allowing for testing of periodic + // drivers and fingerprinters + ShutdownPeriodicAfter = "test.shutdown_periodic_after" + + // ShutdownPeriodicDuration is a config option that can be used during tests + // to "stop" a previously functioning driver after the specified duration + // (specified in seconds) for testing of periodic drivers and fingerprinters. + ShutdownPeriodicDuration = "test.shutdown_periodic_duration" +) + // Add the mock driver to the list of builtin drivers func init() { BuiltinDrivers["mock_driver"] = NewMockDriver @@ -78,14 +88,29 @@ type MockDriverConfig struct { // MockDriver is a driver which is used for testing purposes type MockDriver struct { DriverContext - fingerprint.StaticFingerprinter cleanupFailNum int + + // shutdownFingerprintTime is the time up to which the driver will be up + shutdownFingerprintTime time.Time } // NewMockDriver is a factory method which returns a new Mock Driver func NewMockDriver(ctx *DriverContext) Driver { - return &MockDriver{DriverContext: *ctx} + md := &MockDriver{DriverContext: *ctx} + + // if the shutdown configuration options are set, start the timer here. + // This config option defaults to false + if ctx.config != nil && ctx.config.ReadBoolDefault(ShutdownPeriodicAfter, false) { + duration, err := ctx.config.ReadInt(ShutdownPeriodicDuration) + if err != nil { + errMsg := fmt.Sprintf("unable to read config option for shutdown_periodic_duration %s, got err %s", duration, err.Error()) + panic(errMsg) + } + md.shutdownFingerprintTime = time.Now().Add(time.Second * time.Duration(duration)) + } + + return md } func (d *MockDriver) Abilities() DriverAbilities { @@ -194,9 +219,18 @@ 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 { + switch { + // If the driver is configured to shut down after a period of time, and the + // current time is after the time which the node should shut down, simulate + // driver failure + case !m.shutdownFingerprintTime.IsZero() && time.Now().After(m.shutdownFingerprintTime): + resp.RemoveAttribute("driver.mock_driver") + default: + resp.AddAttribute("driver.mock_driver", "1") + resp.Detected = true + } + return nil } // MockDriverHandle is a driver handler which supervises a mock task @@ -339,3 +373,8 @@ func (h *mockDriverHandle) run() { } } } + +// When testing, poll for updates +func (m *MockDriver) Periodic() (bool, time.Duration) { + return true, 500 * time.Millisecond +} diff --git a/client/driver/qemu.go b/client/driver/qemu.go index 3e9d6f8d93e..158628672be 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,24 @@ 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 no error, as it isn't an error to not find qemu, it just means we + // can't use it. + 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) + resp.RemoveAttribute(qemuDriverAttr) + return fmt.Errorf("Unable to parse Qemu version string: %#v", matches) } currentQemuVersion := matches[1] - node.Attributes[qemuDriverAttr] = "1" - node.Attributes[qemuDriverVersionAttr] = currentQemuVersion + resp.AddAttribute(qemuDriverAttr, "1") + resp.AddAttribute(qemuDriverVersionAttr, currentQemuVersion) + resp.Detected = true - 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 ebab5dae61f..9dbfde76a0b 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,28 @@ 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} + var response cstructs.FingerprintResponse + err := d.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + attributes := response.Attributes + if attributes == nil { + t.Fatalf("attributes should not be nil") } - if node.Attributes[qemuDriverAttr] == "" { + + if attributes[qemuDriverAttr] == "" { t.Fatalf("Missing Qemu driver") } - if node.Attributes[qemuDriverVersionAttr] == "" { + + if attributes[qemuDriverVersionAttr] == "" { t.Fatalf("Missing Qemu driver version") } } @@ -164,12 +176,15 @@ 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} + var response cstructs.FingerprintResponse + 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 86b4406e9a4..0cea2ea554e 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,19 @@ 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.AddAttribute(rawExecDriverAttr, "1") + resp.Detected = true + return nil } - delete(node.Attributes, rawExecDriverAttr) - return false, nil + resp.RemoveAttribute(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 dadaf07382e..1cec71a29fe 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,29 @@ 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} + var response cstructs.FingerprintResponse + 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 !response.Detected { + t.Fatalf("expected response to be applicable") } - 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 52c7c91f0a5..446a48eaefd 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -311,31 +311,30 @@ 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 + resp.RemoveAttribute(rktDriverAttr) + 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) + resp.RemoveAttribute(rktDriverAttr) + return fmt.Errorf("Unable to parse Rkt version string: %#v", rktMatches) } minVersion, _ := version.NewVersion(minRktVersion) @@ -347,21 +346,22 @@ 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 + resp.RemoveAttribute(rktDriverAttr) + return nil } - node.Attributes[rktDriverAttr] = "1" - node.Attributes["driver.rkt.version"] = rktMatches[1] - node.Attributes["driver.rkt.appc.version"] = appcMatches[1] + resp.AddAttribute(rktDriverAttr, "1") + resp.AddAttribute("driver.rkt.version", rktMatches[1]) + resp.AddAttribute("driver.rkt.appc.version", appcMatches[1]) + resp.Detected = true // Advertise if this node supports rkt volumes if d.config.ReadBoolDefault(rktVolumesConfigOption, rktVolumesConfigDefault) { - node.Attributes["driver."+rktVolumesConfigOption] = "1" + resp.AddAttribute("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 49ae7268ce2..13c474def1e 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 4fd5057d347..604090dc0a1 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,29 @@ 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} + var response cstructs.FingerprintResponse + err := d.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !apply { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + attributes := response.Attributes + if attributes == nil { + t.Fatalf("expected attributes to not equal nil") } - if node.Attributes["driver.rkt"] != "1" { + if attributes["driver.rkt"] != "1" { t.Fatalf("Missing Rkt driver") } - if node.Attributes["driver.rkt.version"] == "" { + if attributes["driver.rkt.version"] == "" { t.Fatalf("Missing Rkt driver version") } - if node.Attributes["driver.rkt.appc.version"] == "" { + if 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 71b5352a891..3277822bcff 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,8 @@ 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.AddAttribute("cpu.arch", runtime.GOARCH) + resp.Detected = true + return nil } diff --git a/client/fingerprint/arch_test.go b/client/fingerprint/arch_test.go index 4e4b94a6786..320ccc321f1 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,17 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") - } - if node.Attributes["cpu.arch"] == "" { - t.Fatalf("missing arch") + + if !response.Detected { + t.Fatalf("expected response to be applicable") } + + assertNodeAttributeContains(t, response.Attributes, "cpu.arch") } diff --git a/client/fingerprint/cgroup.go b/client/fingerprint/cgroup.go index 1ec8d87932d..2e6c446379a 100644 --- a/client/fingerprint/cgroup.go +++ b/client/fingerprint/cgroup.go @@ -6,7 +6,7 @@ import ( "log" "time" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) const ( @@ -49,8 +49,8 @@ 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(r *cstructs.FingerprintResponse) { + r.RemoveAttribute("unique.cgroup.mountpoint") } // 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 9abb959b598..c6f78cd354e 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,31 @@ 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) + f.clearCGroupAttributes(resp) + 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) + + f.clearCGroupAttributes(resp) 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.AddAttribute("unique.cgroup.mountpoint", mount) + resp.Detected = true 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 f1237940576..2dc1d51ecdd 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,91 @@ 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} + var response cstructs.FingerprintResponse + 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} + var response cstructs.FingerprintResponse + 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") + { + f := &CGroupFingerprint{ + logger: testLogger(), + lastState: cgroupUnavailable, + mountPointDetector: &MountPointDetectorEmptyMountPoint{}, + } + + node := &structs.Node{ + Attributes: make(map[string]string), + } + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + var response cstructs.FingerprintResponse + 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) + } } - 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: cgroupAvailable, + mountPointDetector: &MountPointDetectorValidMountPoint{}, + } + + node := &structs.Node{ + Attributes: make(map[string]string), + } + + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) + if err != nil { + t.Fatalf("unexpected error, %s", err) + } + if a, _ := response.Attributes["unique.cgroup.mountpoint"]; a == "" { + t.Fatalf("expected attribute to be found, %s", a) + } } } diff --git a/client/fingerprint/consul.go b/client/fingerprint/consul.go index b3790a417eb..84c6a97d62a 100644 --- a/client/fingerprint/consul.go +++ b/client/fingerprint/consul.go @@ -8,8 +8,7 @@ import ( consul "github.com/hashicorp/consul/api" - client "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs" + cstructs "github.com/hashicorp/nomad/client/structs" ) const ( @@ -29,23 +28,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 +47,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) + f.clearConsulAttributes(resp) // Print a message indicating that the Consul Agent is not available // anymore @@ -62,39 +55,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.AddAttribute("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.AddAttribute("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.AddAttribute("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.AddAttribute("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.AddAttribute("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 dc, ok := resp.Attributes["consul.datacenter"]; ok { + if name, ok2 := resp.Attributes["unique.consul.name"]; ok2 { + resp.AddLink("consul", fmt.Sprintf("%s.%s", dc, name)) + } } else { f.logger.Printf("[WARN] fingerprint.consul: malformed Consul response prevented linking") } @@ -105,18 +98,19 @@ 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 + resp.Detected = true + 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") +func (f *ConsulFingerprint) clearConsulAttributes(r *cstructs.FingerprintResponse) { + r.RemoveAttribute("consul.server") + r.RemoveAttribute("consul.version") + r.RemoveAttribute("consul.revision") + r.RemoveAttribute("unique.consul.name") + r.RemoveAttribute("consul.datacenter") + r.RemoveLink("consul") } func (f *ConsulFingerprint) Periodic() (bool, time.Duration) { diff --git a/client/fingerprint/consul_test.go b/client/fingerprint/consul_test.go index e2eecf4385c..400ab01f407 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,27 @@ 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} + var response cstructs.FingerprintResponse + err := fp.Fingerprint(request, &response) if err != nil { t.Fatalf("Failed to fingerprint: %s", err) } - if !ok { - t.Fatalf("Failed to apply node attributes") + + if !response.Detected { + t.Fatalf("expected response to be applicable") } - 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 +181,17 @@ 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://") - ok, err := fp.Fingerprint(config, node) + request := &cstructs.FingerprintRequest{Config: conf, Node: node} + var response cstructs.FingerprintResponse + err := fp.Fingerprint(request, &response) assert.Nil(err) - assert.True(ok) + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } attrs := []string{ "consul.server", @@ -191,13 +200,14 @@ func TestConsulFingerprint_UnexpectedResponse(t *testing.T) { "unique.consul.name", "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) } } - if v, ok := node.Links["consul"]; ok { + if v, ok := response.Links["consul"]; ok { t.Errorf("Unexpected link to consul: %v", v) } } diff --git a/client/fingerprint/cpu.go b/client/fingerprint/cpu.go index 0c9b4c25d0f..434eb5844d4 100644 --- a/client/fingerprint/cpu.go +++ b/client/fingerprint/cpu.go @@ -4,7 +4,7 @@ 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" ) @@ -21,13 +21,12 @@ func NewCPUFingerprint(logger *log.Logger) Fingerprint { return f } -func (f *CPUFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { - setResources := func(totalCompute int) { - if node.Resources == nil { - node.Resources = &structs.Resources{} +func (f *CPUFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config + setResourcesCPU := func(totalCompute int) { + resp.Resources = &structs.Resources{ + CPU: totalCompute, } - - node.Resources.CPU = totalCompute } if err := stats.Init(); err != nil { @@ -35,21 +34,21 @@ func (f *CPUFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bo } if cfg.CpuCompute != 0 { - setResources(cfg.CpuCompute) - return true, nil + setResourcesCPU(cfg.CpuCompute) + return nil } if modelName := stats.CPUModelName(); modelName != "" { - node.Attributes["cpu.modelname"] = modelName + resp.AddAttribute("cpu.modelname", modelName) } if mhz := stats.CPUMHzPerCore(); mhz > 0 { - node.Attributes["cpu.frequency"] = fmt.Sprintf("%.0f", mhz) + resp.AddAttribute("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.AddAttribute("cpu.numcores", fmt.Sprintf("%d", numCores)) f.logger.Printf("[DEBUG] fingerprint.cpu: core count: %d", numCores) } @@ -62,17 +61,14 @@ 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.AddAttribute("cpu.totalcompute", fmt.Sprintf("%d", tt)) + setResourcesCPU(tt) + resp.Detected = true - node.Resources.CPU = tt - return true, nil + return nil } diff --git a/client/fingerprint/cpu_test.go b/client/fingerprint/cpu_test.go index 238d55770b1..5bb76197036 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,33 +13,40 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") } // CPU info - if node.Attributes["cpu.numcores"] == "" { + attributes := response.Attributes + if attributes == nil { + t.Fatalf("expected attributes to be initialized") + } + if attributes["cpu.numcores"] == "" { t.Fatalf("Missing Num Cores") } - if node.Attributes["cpu.modelname"] == "" { + if attributes["cpu.modelname"] == "" { t.Fatalf("Missing Model Name") } - if node.Attributes["cpu.frequency"] == "" { + if attributes["cpu.frequency"] == "" { t.Fatalf("Missing CPU Frequency") } - if node.Attributes["cpu.totalcompute"] == "" { + if 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") } - } // TestCPUFingerprint_OverrideCompute asserts that setting cpu_total_compute in @@ -49,30 +57,41 @@ 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") - } + var originalCPU int - // Get actual system CPU - origCPU := node.Resources.CPU + { + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) + if err != nil { + t.Fatalf("err: %v", err) + } - // Override it with a setting - cfg.CpuCompute = origCPU + 123 + if !response.Detected { + t.Fatalf("expected response to be applicable") + } - // 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") + if response.Resources.CPU == 0 { + t.Fatalf("expected fingerprint of cpu of but found 0") + } + + 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} + var response cstructs.FingerprintResponse + 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 b442f49ff30..79bd6730f0d 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.AddAttribute(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, ok := response.Attributes["unique.platform.aws.local-ipv4"]; ok && val != "" { + response.AddAttribute("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,18 @@ 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 = &structs.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.AddLink("aws.ec2", fmt.Sprintf("%s.%s", + response.Attributes["platform.aws.placement.availability-zone"], + response.Attributes["unique.platform.aws.instance-id"])) + response.Detected = true - 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 77f7c1dc96a..2a94561df4c 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,15 @@ 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} + var response cstructs.FingerprintResponse + 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,15 +54,13 @@ 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) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("Expected AWS attributes and Links") - } - keys := []string{ "platform.aws.ami-id", "unique.platform.aws.hostname", @@ -74,16 +75,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 +172,21 @@ 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} + var response cstructs.FingerprintResponse + 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 +217,81 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) + if err != nil { + t.Fatalf("err: %v", err) + } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + if !response.Detected { + t.Fatalf("expected response to be applicable") + } - 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} + var response cstructs.FingerprintResponse + 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 +302,14 @@ 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} + var response cstructs.FingerprintResponse + 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 83da63486b5..280914f3df4 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.AddAttribute(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.AddAttribute(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.AddAttribute(prefix, "true") + resp.AddAttribute(uniquePrefix+".ip", strings.Trim(intf.Ip, "\n")) for index, accessConfig := range intf.AccessConfigs { - node.Attributes[uniquePrefix+".external-ip."+strconv.Itoa(index)] = accessConfig.ExternalIp + resp.AddAttribute(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.AddAttribute(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,17 @@ 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.AddAttribute(key, strings.Trim(v, "\n")) } // populate Links - node.Links["gce"] = node.Attributes["unique.platform.gce.id"] + if id, ok := resp.Attributes["unique.platform.gce.id"]; ok { + resp.AddLink("gce", id) + } + + resp.Detected = true - 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 1e339789a22..14837dca8a7 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,19 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if ok { - t.Fatalf("Should be false without test server") + if response.Detected { + t.Fatalf("expected response to not be applicable") + } + + if len(response.Attributes) > 0 { + t.Fatalf("Should have zero attributes without test server") } } @@ -76,13 +83,15 @@ 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) + request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") + if !response.Detected { + t.Fatalf("expected response to be applicable") } keys := []string{ @@ -100,40 +109,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 2d6d483b6c5..8a3477f5174 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 7e62440ea6c..41a7e6549a5 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,63 @@ 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} + var response cstructs.FingerprintResponse + 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) { + if nodeAttributes == nil { + t.Errorf("expected an initialized map for node attributes") + return + } + + 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) { + if nodeAttributes == nil { + t.Errorf("expected an initialized map for node attributes") + return + } + 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) { + if nodeLinks == nil { + t.Errorf("expected an initialized map for node links") + return + } + 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 a7b8ed6c8ae..cfeabd4acd4 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,21 @@ 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.AddAttribute("os.name", hostInfo.Platform) + resp.AddAttribute("os.version", hostInfo.PlatformVersion) - node.Attributes["kernel.name"] = runtime.GOOS - node.Attributes["kernel.version"] = hostInfo.KernelVersion + resp.AddAttribute("kernel.name", runtime.GOOS) + resp.AddAttribute("kernel.version", hostInfo.KernelVersion) - node.Attributes["unique.hostname"] = hostInfo.Hostname + resp.AddAttribute("unique.hostname", hostInfo.Hostname) + resp.Detected = true - return true, nil + return nil } diff --git a/client/fingerprint/host_test.go b/client/fingerprint/host_test.go index 5a5f0cfbe1e..79ef870c555 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + 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 b249bebf575..c24937a4300 100644 --- a/client/fingerprint/memory.go +++ b/client/fingerprint/memory.go @@ -4,7 +4,7 @@ import ( "fmt" "log" - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" "github.com/shirou/gopsutil/mem" ) @@ -23,21 +23,20 @@ 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.AddAttribute("memory.totalbytes", fmt.Sprintf("%d", memInfo.Total)) - if node.Resources == nil { - node.Resources = &structs.Resources{} + resp.Resources = &structs.Resources{ + MemoryMB: int(memInfo.Total / 1024 / 1024), } - node.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 44c79c0cb31..1b2cebb5bb8 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,21 +13,20 @@ 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} + var response cstructs.FingerprintResponse + 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 { - t.Fatalf("Node Resources was nil") + if response.Resources == nil { + t.Fatalf("response resources should not be nil") } - if node.Resources.MemoryMB == 0 { - t.Errorf("Expected node.Resources.MemoryMB to be non-zero") + if response.Resources.MemoryMB == 0 { + t.Fatalf("Expected node.Resources.MemoryMB to be non-zero") } - } diff --git a/client/fingerprint/network.go b/client/fingerprint/network.go index 287fb73593c..9634a796917 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,23 @@ 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 = &structs.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.AddAttribute("unique.network.ip-address", nwResources[0].IP) } + resp.Detected = true - // 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 78dca0df7ca..87b57bd4f38 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,36 @@ func TestNetworkFingerprint_basic(t *testing.T) { } cfg := &config.Config{NetworkSpeed: 101} - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + attributes := response.Attributes + if len(attributes) == 0 { t.Fatalf("should apply (HINT: working offline? Set env %q=y", skipOnlineTestsEnvVar) } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := 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 +241,19 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err == nil { t.Fatalf("err: %v", err) } - if ok { - t.Fatalf("ok: %v", ok) + if response.Detected { + t.Fatalf("expected response to not be applicable") + } + + if len(response.Attributes) != 0 { + t.Fatalf("attributes should be zero but instead are: %v", response.Attributes) } } @@ -249,28 +264,36 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + attributes := response.Attributes + if len(attributes) == 0 { t.Fatalf("should apply") } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := 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 +315,32 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + attributes := response.Attributes + assertNodeAttributeContains(t, attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := 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 +362,36 @@ 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} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + attributes := response.Attributes + if len(attributes) == 0 { + t.Fatalf("should apply attributes") } - assertNodeAttributeContains(t, node, "unique.network.ip-address") + assertNodeAttributeContains(t, attributes, "unique.network.ip-address") - ip := node.Attributes["unique.network.ip-address"] + ip := 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 +422,18 @@ func TestNetworkFingerPrint_LinkLocal_Disallowed(t *testing.T) { }, } - ok, err := f.Fingerprint(cfg, node) + request := &cstructs.FingerprintRequest{Config: cfg, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { - t.Fatalf("should not apply") + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + if len(response.Attributes) != 0 { + t.Fatalf("should not apply attributes") } } diff --git a/client/fingerprint/nomad.go b/client/fingerprint/nomad.go index 0db894196fd..0a00cc026d0 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,9 @@ 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.AddAttribute("nomad.version", req.Config.Version.VersionNumber()) + resp.AddAttribute("nomad.revision", req.Config.Version.Revision) + resp.Detected = true + return nil } diff --git a/client/fingerprint/nomad_test.go b/client/fingerprint/nomad_test.go index 730fc3c5db2..2060fd8e269 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,27 @@ func TestNomadFingerprint(t *testing.T) { Version: v, }, } - ok, err := f.Fingerprint(c, node) + + request := &cstructs.FingerprintRequest{Config: c, Node: node} + var response cstructs.FingerprintResponse + err := f.Fingerprint(request, &response) if err != nil { t.Fatalf("err: %v", err) } - if !ok { + + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + 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 d20cec07b02..9aac819e1b0 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,14 @@ 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.AddAttribute("os.signals", strings.Join(sigs, ",")) + resp.Detected = true + return nil } diff --git a/client/fingerprint/signal_test.go b/client/fingerprint/signal_test.go index 2157cf0c50e..bf61f754408 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 c60f13154bc..6dc72fb6df6 100644 --- a/client/fingerprint/storage.go +++ b/client/fingerprint/storage.go @@ -6,7 +6,7 @@ import ( "os" "strconv" - "github.com/hashicorp/nomad/client/config" + cstructs "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/nomad/structs" ) @@ -24,15 +24,8 @@ func NewStorageFingerprint(logger *log.Logger) Fingerprint { return fp } -func (f *StorageFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { - - // 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{} - } +func (f *StorageFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error { + cfg := req.Config // Guard against unset AllocDir storageDir := cfg.AllocDir @@ -40,20 +33,24 @@ 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.AddAttribute("unique.storage.volume", volume) + resp.AddAttribute("unique.storage.bytestotal", strconv.FormatUint(total, 10)) + resp.AddAttribute("unique.storage.bytesfree", strconv.FormatUint(free, 10)) - node.Resources.DiskMB = int(free / bytesPerMegabyte) + // set the disk size for the response + resp.Resources = &structs.Resources{ + DiskMB: int(free / bytesPerMegabyte), + } + resp.Detected = true - return true, nil + return nil } diff --git a/client/fingerprint/storage_test.go b/client/fingerprint/storage_test.go index f975aec6d1d..c4388905f49 100644 --- a/client/fingerprint/storage_test.go +++ b/client/fingerprint/storage_test.go @@ -13,17 +13,21 @@ 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") + if !response.Detected { + t.Fatalf("expected response to be applicable") + } + + 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 +36,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 a8728fc98c5..0613e98a019 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,33 @@ 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) - + f.clearVaultAttributes(resp) // 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.AddAttribute("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.AddAttribute("vault.version", strings.TrimPrefix(status.Version, "Vault ")) + resp.AddAttribute("vault.cluster_id", status.ClusterID) + resp.AddAttribute("vault.cluster_name", status.ClusterName) // If Vault was previously unavailable print a message to indicate the Agent // is available now @@ -75,16 +74,17 @@ 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") + resp.Detected = true + return nil } func (f *VaultFingerprint) Periodic() (bool, time.Duration) { return true, 15 * time.Second } + +func (f *VaultFingerprint) clearVaultAttributes(r *cstructs.FingerprintResponse) { + r.RemoveAttribute("vault.accessible") + r.RemoveAttribute("vault.version") + r.RemoveAttribute("vault.cluster_id") + r.RemoveAttribute("vault.cluster_name") +} diff --git a/client/fingerprint/vault_test.go b/client/fingerprint/vault_test.go index a6835b937d3..25e7f1386dc 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} + var response cstructs.FingerprintResponse + err := fp.Fingerprint(request, &response) if err != nil { t.Fatalf("Failed to fingerprint: %s", err) } - if !ok { - t.Fatalf("Failed to apply node attributes") + + if !response.Detected { + t.Fatalf("expected response to be applicable") } - 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 0673ce951e7..97887232de0 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,65 @@ func (d *DriverNetwork) Hash() []byte { } return h.Sum(nil) } + +// FingerprintRequest is a request which a fingerprinter accepts to fingerprint +// the node +type FingerprintRequest struct { + Config *config.Config + Node *structs.Node +} + +// FingerprintResponse is the response which a fingerprinter annotates with the +// results of the fingerprint method +type FingerprintResponse struct { + Attributes map[string]string + Links map[string]string + Resources *structs.Resources + + // Detected is a boolean indicating whether the fingerprinter detected + // if the resource was available + Detected bool +} + +// AddAttribute adds the name and value for a node attribute to the fingerprint +// response +func (f *FingerprintResponse) AddAttribute(name, value string) { + // initialize Attributes if it has not been already + if f.Attributes == nil { + f.Attributes = make(map[string]string, 0) + } + + f.Attributes[name] = value +} + +// RemoveAttribute sets the given attribute to empty, which will later remove +// it entirely from the node +func (f *FingerprintResponse) RemoveAttribute(name string) { + // initialize Attributes if it has not been already + if f.Attributes == nil { + f.Attributes = make(map[string]string, 0) + } + + f.Attributes[name] = "" +} + +// AddLink adds a link entry to the fingerprint response +func (f *FingerprintResponse) AddLink(name, value string) { + // initialize Links if it has not been already + if f.Links == nil { + f.Links = make(map[string]string, 0) + } + + f.Links[name] = value +} + +// RemoveLink removes a link entry from the fingerprint response. This will +// later remove it entirely from the node +func (f *FingerprintResponse) RemoveLink(name string) { + // initialize Links if it has not been already + if f.Links == nil { + f.Links = make(map[string]string, 0) + } + + f.Links[name] = "" +}