refactor Fingerprint to request/response construct

client/client.go

@@ -23,6 +23,7 @@ import (
 	"github.com/hashicorp/nomad/client/driver"
 	"github.com/hashicorp/nomad/client/fingerprint"
 	"github.com/hashicorp/nomad/client/stats"
+	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/client/vaultclient"
 	"github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/helper"
@@ -949,15 +950,31 @@ func (c *Client) fingerprint() error {
 			return err
 		}
 
-		c.configLock.Lock()
-		applies, err := f.Fingerprint(c.config, c.config.Node)
-		c.configLock.Unlock()
+		request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
+		response := &cstructs.FingerprintResponse{
+			Attributes: make(map[string]string, 0),
+			Links:      make(map[string]string, 0),
+			Resources:  &structs.Resources{},
+		}
+
+		err = f.Fingerprint(request, response)
 		if err != nil {
 			return err
 		}
-		if applies {
+
+		// if an attribute should be skipped, remove it from the list which we will
+		// later apply to the node
+		for _, e := range skipped {
+			delete(response.Attributes, e)
+		}
+
+		for name := range response.Attributes {
 			applied = append(applied, name)
 		}
+
+		// add the diff found from each fingerprinter
+		c.updateNodeFromFingerprint(response)
+
 		p, period := f.Periodic()
 		if p {
 			// TODO: If more periodic fingerprinters are added, then
@@ -966,6 +983,7 @@ func (c *Client) fingerprint() error {
 			go c.fingerprintPeriodic(name, f, period)
 		}
 	}
+
 	c.logger.Printf("[DEBUG] client: applied fingerprints %v", applied)
 	if len(skipped) != 0 {
 		c.logger.Printf("[DEBUG] client: fingerprint modules skipped due to white/blacklist: %v", skipped)
@@ -979,11 +997,21 @@ func (c *Client) fingerprintPeriodic(name string, f fingerprint.Fingerprint, d t
 	for {
 		select {
 		case <-time.After(d):
-			c.configLock.Lock()
-			if _, err := f.Fingerprint(c.config, c.config.Node); err != nil {
+			request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
+			response := &cstructs.FingerprintResponse{
+				Attributes: make(map[string]string, 0),
+				Links:      make(map[string]string, 0),
+				Resources:  &structs.Resources{},
+			}
+
+			err := f.Fingerprint(request, response)
+
+			if err != nil {
 				c.logger.Printf("[DEBUG] client: periodic fingerprinting for %v failed: %v", name, err)
+			} else {
+				c.updateNodeFromFingerprint(response)
 			}
-			c.configLock.Unlock()
+
 		case <-c.shutdownCh:
 			return
 		}
@@ -1017,16 +1045,30 @@ func (c *Client) setupDrivers() error {
 		if err != nil {
 			return err
 		}
-		c.configLock.Lock()
-		applies, err := d.Fingerprint(c.config, c.config.Node)
-		c.configLock.Unlock()
+
+		request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
+		response := &cstructs.FingerprintResponse{
+			Attributes: make(map[string]string, 0),
+			Links:      make(map[string]string, 0),
+			Resources:  &structs.Resources{},
+		}
+
+		err = d.Fingerprint(request, response)
 		if err != nil {
 			return err
 		}
-		if applies {
+
+		// remove attributes we are supposed to skip
+		for _, attr := range skipped {
+			delete(response.Attributes, attr)
+		}
+
+		for name := range response.Attributes {
 			avail = append(avail, name)
 		}
 
+		c.updateNodeFromFingerprint(response)
+
 		p, period := d.Periodic()
 		if p {
 			go c.fingerprintPeriodic(name, d, period)
@@ -1035,6 +1077,7 @@ func (c *Client) setupDrivers() error {
 	}
 
 	c.logger.Printf("[DEBUG] client: available drivers %v", avail)
+	c.logger.Printf("[DEBUG] client: skipped attributes %v", skipped)
 
 	if len(skipped) != 0 {
 		c.logger.Printf("[DEBUG] client: drivers skipped due to white/blacklist: %v", skipped)
@@ -1043,6 +1086,24 @@ func (c *Client) setupDrivers() error {
 	return nil
 }
 
+// updateNodeFromFingerprint updates the node with the result of
+// fingerprinting the node from the diff that was created
+func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) {
+	c.configLock.Lock()
+	defer c.configLock.Unlock()
+	for name, val := range response.Attributes {
+		c.config.Node.Attributes[name] = val
+	}
+
+	// update node links and resources from the diff created from
+	// fingerprinting
+	for name, val := range response.Links {
+		c.config.Node.Links[name] = val
+	}
+
+	c.config.Node.Resources.Merge(response.Resources)
+}
+
 // retryIntv calculates a retry interval value given the base
 func (c *Client) retryIntv(base time.Duration) time.Duration {
 	if c.config.DevMode {

client/driver/docker.go

@@ -26,7 +26,6 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/go-plugin"
 	"github.com/hashicorp/nomad/client/allocdir"
-	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/driver/env"
 	"github.com/hashicorp/nomad/client/driver/executor"
 	dstructs "github.com/hashicorp/nomad/client/driver/structs"
@@ -476,42 +475,39 @@ func NewDockerDriver(ctx *DriverContext) Driver {
 	return &DockerDriver{DriverContext: *ctx}
 }
 
-func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) {
-	// Initialize docker API clients
+func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
 	client, _, err := d.dockerClients()
 	if err != nil {
 		if d.fingerprintSuccess == nil || *d.fingerprintSuccess {
 			d.logger.Printf("[INFO] driver.docker: failed to initialize client: %s", err)
 		}
-		delete(node.Attributes, dockerDriverAttr)
 		d.fingerprintSuccess = helper.BoolToPtr(false)
-		return false, nil
+		return nil
 	}
 
 	// This is the first operation taken on the client so we'll try to
 	// establish a connection to the Docker daemon. If this fails it means
 	// Docker isn't available so we'll simply disable the docker driver.
 	env, err := client.Version()
 	if err != nil {
-		delete(node.Attributes, dockerDriverAttr)
 		if d.fingerprintSuccess == nil || *d.fingerprintSuccess {
 			d.logger.Printf("[DEBUG] driver.docker: could not connect to docker daemon at %s: %s", client.Endpoint(), err)
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
-		return false, nil
+		return nil
 	}
 
-	node.Attributes[dockerDriverAttr] = "1"
-	node.Attributes["driver.docker.version"] = env.Get("Version")
+	resp.Attributes[dockerDriverAttr] = "1"
+	resp.Attributes["driver.docker.version"] = env.Get("Version")
 
 	privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false)
 	if privileged {
-		node.Attributes[dockerPrivilegedConfigOption] = "1"
+		resp.Attributes[dockerPrivilegedConfigOption] = "1"
 	}
 
 	// Advertise if this node supports Docker volumes
 	if d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) {
-		node.Attributes["driver."+dockerVolumesConfigOption] = "1"
+		resp.Attributes["driver."+dockerVolumesConfigOption] = "1"
 	}
 
 	// Detect bridge IP address - #2785
@@ -529,7 +525,7 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool
 			}
 
 			if n.IPAM.Config[0].Gateway != "" {
-				node.Attributes["driver.docker.bridge_ip"] = n.IPAM.Config[0].Gateway
+				resp.Attributes["driver.docker.bridge_ip"] = n.IPAM.Config[0].Gateway
 			} else if d.fingerprintSuccess == nil {
 				// Docker 17.09.0-ce dropped the Gateway IP from the bridge network
 				// See https://github.com/moby/moby/issues/32648
@@ -540,7 +536,7 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool
 	}
 
 	d.fingerprintSuccess = helper.BoolToPtr(true)
-	return true, nil
+	return nil
 }
 
 // Validate is used to validate the driver configuration

client/driver/docker_test.go

@@ -171,17 +171,28 @@ func TestDockerDriver_Fingerprint(t *testing.T) {
 	node := &structs.Node{
 		Attributes: make(map[string]string),
 	}
-	apply, err := d.Fingerprint(&config.Config{}, node)
+
+	request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node}
+	response := &cstructs.FingerprintResponse{
+		Attributes: make(map[string]string, 0),
+		Links:      make(map[string]string, 0),
+		Resources:  &structs.Resources{},
+	}
+
+	err := d.Fingerprint(request, response)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	if apply != testutil.DockerIsConnected(t) {
+
+	if testutil.DockerIsConnected(t) && response.Attributes["driver.docker"] == "" {
 		t.Fatalf("Fingerprinter should detect when docker is available")
 	}
-	if node.Attributes["driver.docker"] != "1" {
+
+	if response.Attributes["driver.docker"] != "1" {
 		t.Log("Docker daemon not available. The remainder of the docker tests will be skipped.")
 	}
-	t.Logf("Found docker version %s", node.Attributes["driver.docker.version"])
+
+	t.Logf("Found docker version %s", response.Attributes["driver.docker.version"])
 }
 
 // TestDockerDriver_Fingerprint_Bridge asserts that if Docker is running we set
@@ -210,18 +221,26 @@ func TestDockerDriver_Fingerprint_Bridge(t *testing.T) {
 	conf := testConfig(t)
 	conf.Node = mock.Node()
 	dd := NewDockerDriver(NewDriverContext("", "", conf, conf.Node, testLogger(), nil))
-	ok, err := dd.Fingerprint(conf, conf.Node)
+
+	request := &cstructs.FingerprintRequest{Config: conf, Node: conf.Node}
+	response := &cstructs.FingerprintResponse{
+		Attributes: make(map[string]string, 0),
+		Links:      make(map[string]string, 0),
+		Resources:  &structs.Resources{},
+	}
+
+	err = dd.Fingerprint(request, response)
 	if err != nil {
 		t.Fatalf("error fingerprinting docker: %v", err)
 	}
-	if !ok {
+	if response.Attributes["driver.docker"] == "" {
 		t.Fatalf("expected Docker to be enabled but false was returned")
 	}
 
-	if found := conf.Node.Attributes["driver.docker.bridge_ip"]; found != expectedAddr {
+	if found := response.Attributes["driver.docker.bridge_ip"]; found != expectedAddr {
 		t.Fatalf("expected bridge ip %q but found: %q", expectedAddr, found)
 	}
-	t.Logf("docker bridge ip: %q", conf.Node.Attributes["driver.docker.bridge_ip"])
+	t.Logf("docker bridge ip: %q", response.Attributes["driver.docker.bridge_ip"])
 }
 
 func TestDockerDriver_StartOpen_Wait(t *testing.T) {

client/driver/exec_default.go

@@ -3,12 +3,11 @@
 package driver
 
 import (
-	"github.com/hashicorp/nomad/client/config"
+	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/helper"
-	"github.com/hashicorp/nomad/nomad/structs"
 )
 
-func (d *ExecDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) {
+func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
 	d.fingerprintSuccess = helper.BoolToPtr(false)
-	return false, nil
+	return nil
 }

client/driver/exec_linux.go

@@ -1,9 +1,8 @@
 package driver
 
 import (
-	"github.com/hashicorp/nomad/client/config"
+	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/helper"
-	"github.com/hashicorp/nomad/nomad/structs"
 	"golang.org/x/sys/unix"
 )
 
@@ -13,28 +12,26 @@ const (
 	execDriverAttr = "driver.exec"
 )
 
-func (d *ExecDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) {
+func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
 	// Only enable if cgroups are available and we are root
-	if !cgroupsMounted(node) {
+	if !cgroupsMounted(req.Node) {
 		if d.fingerprintSuccess == nil || *d.fingerprintSuccess {
 			d.logger.Printf("[DEBUG] driver.exec: cgroups unavailable, disabling")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
-		delete(node.Attributes, execDriverAttr)
-		return false, nil
+		return nil
 	} else if unix.Geteuid() != 0 {
 		if d.fingerprintSuccess == nil || *d.fingerprintSuccess {
 			d.logger.Printf("[DEBUG] driver.exec: must run as root user, disabling")
 		}
-		delete(node.Attributes, execDriverAttr)
 		d.fingerprintSuccess = helper.BoolToPtr(false)
-		return false, nil
+		return nil
 	}
 
 	if d.fingerprintSuccess == nil || !*d.fingerprintSuccess {
 		d.logger.Printf("[DEBUG] driver.exec: exec driver is enabled")
 	}
-	node.Attributes[execDriverAttr] = "1"
+	resp.Attributes[execDriverAttr] = "1"
 	d.fingerprintSuccess = helper.BoolToPtr(true)
-	return true, nil
+	return nil
 }

client/driver/exec_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/driver/env"
+	cstructs "github.com/hashicorp/nomad/client/structs"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/testutil"
 
@@ -37,14 +38,19 @@ func TestExecDriver_Fingerprint(t *testing.T) {
 			"unique.cgroup.mountpoint": "/sys/fs/cgroup",
 		},
 	}
-	apply, err := d.Fingerprint(&config.Config{}, node)
+
+	request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node}
+	response := &cstructs.FingerprintResponse{
+		Attributes: make(map[string]string, 0),
+		Links:      make(map[string]string, 0),
+		Resources:  &structs.Resources{},
+	}
+
+	err := d.Fingerprint(request, response)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	if !apply {
-		t.Fatalf("should apply")
-	}
-	if node.Attributes["driver.exec"] == "" {
+	if response.Attributes["driver.exec"] == "" {
 		t.Fatalf("missing driver")
 	}
 }