remove attributes from periodic fingerprints when state changes

write test for client periodic fingerprinters

client/client.go

@@ -100,6 +100,10 @@ type Client struct {
 	config *config.Config
 	start  time.Time
 
+	// fingerprinters is a list of a client's periodic fingerprinters.
+	// TODO this will later be moved to a driver manager
+	fingerprinters map[string]fingerprint.Fingerprint
+
 	// stateDB is used to efficiently store client state.
 	stateDB *bolt.DB
 
@@ -197,6 +201,7 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
 		servers:             newServerList(),
 		triggerDiscoveryCh:  make(chan struct{}),
 		serversDiscoveredCh: make(chan struct{}),
+		fingerprinters:      make(map[string]fingerprint.Fingerprint),
 	}
 
 	// Initialize the client
@@ -969,6 +974,7 @@ func (c *Client) fingerprint() error {
 			// TODO: If more periodic fingerprinters are added, then
 			// fingerprintPeriodic should be used to handle all the periodic
 			// fingerprinters by using a priority queue.
+			c.fingerprinters[name] = f
 			go c.fingerprintPeriodic(name, f, period)
 		}
 	}
@@ -1044,6 +1050,7 @@ func (c *Client) setupDrivers() error {
 
 		p, period := d.Periodic()
 		if p {
+			c.fingerprinters[name] = d
 			go c.fingerprintPeriodic(name, d, period)
 		}
 
@@ -1062,11 +1069,18 @@ func (c *Client) setupDrivers() error {
 func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) {
 	c.configLock.Lock()
 	defer c.configLock.Unlock()
+	c.logger.Printf("RESPONSE ATTRIBUTES %+v", response.GetAttributes())
 	for name, val := range response.GetAttributes() {
 		if val == "" {
+			if name == "driver.mock_driver" {
+				c.logger.Printf("REMOVING MOCK ADDR ATTRIBUTE")
+			}
 			delete(c.config.Node.Attributes, name)
 		} else {
 			c.config.Node.Attributes[name] = val
+			if name == "driver.mock_driver" {
+				c.logger.Printf("ADDING MOCK ADDR ATTRIBUTE")
+			}
 		}
 	}
 

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,49 @@ 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()
+
+	c1 := testClient(t, nil)
+	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)
+	})
+
+	// set IsShutdown to true on the mock driver
+	md := c1.fingerprinters["mock_driver"]
+	dd, ok := md.(*driver.MockDriver)
+	if !ok {
+		t.Fatalf("should be ok")
+	}
+	dd.IsShutdown = true
+
+	// Ensure that the client fingerprinter 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) {

client/driver/docker.go

@@ -482,6 +482,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
 			d.logger.Printf("[INFO] driver.docker: failed to initialize client: %s", err)
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(dockerDriverAttr)
 		return nil
 	}
 
@@ -494,6 +495,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
 			d.logger.Printf("[DEBUG] driver.docker: could not connect to docker daemon at %s: %s", client.Endpoint(), err)
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(dockerDriverAttr)
 		return nil
 	}
 

client/driver/exec_linux.go

@@ -19,12 +19,14 @@ func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 			d.logger.Printf("[DEBUG] driver.exec: cgroups unavailable, disabling")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		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")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(execDriverAttr)
 		return nil
 	}
 

client/driver/java.go

@@ -118,6 +118,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 			d.logger.Printf("[DEBUG] driver.java: root privileges and mounted cgroups required on linux, disabling")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(javaDriverAttr)
 		return nil
 	}
 
@@ -131,6 +132,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	if err != nil {
 		// assume Java wasn't found
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(javaDriverAttr)
 		return nil
 	}
 
@@ -150,6 +152,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 			d.logger.Println("[WARN] driver.java: error parsing Java version information, aborting")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(javaDriverAttr)
 		return nil
 	}
 

client/driver/mock_driver.go

@@ -16,7 +16,6 @@ import (
 	"github.com/mitchellh/mapstructure"
 
 	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"
 )
@@ -77,7 +76,10 @@ type MockDriverConfig struct {
 // MockDriver is a driver which is used for testing purposes
 type MockDriver struct {
 	DriverContext
-	fingerprint.StaticFingerprinter
+
+	// IsShutdown is Used purely for testing purposes in order to validate
+	// periodic fingerprint changes are picked up by the client and set on the node
+	IsShutdown bool
 
 	cleanupFailNum int
 }
@@ -194,7 +196,12 @@ func (m *MockDriver) Validate(map[string]interface{}) error {
 
 // Fingerprint fingerprints a node and returns if MockDriver is enabled
 func (m *MockDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
-	resp.AddAttribute("driver.mock_driver", "1")
+	switch {
+	case m.IsShutdown:
+		resp.RemoveAttribute("driver.mock_driver")
+	default:
+		resp.AddAttribute("driver.mock_driver", "1")
+	}
 	return nil
 }
 
@@ -338,3 +345,8 @@ func (h *mockDriverHandle) run() {
 		}
 	}
 }
+
+// When testing, poll for updates
+func (m *MockDriver) Periodic() (bool, time.Duration) {
+	return true, 1 * time.Second
+}

client/driver/qemu.go

@@ -163,12 +163,14 @@ func (d *QemuDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	}
 	outBytes, err := exec.Command(bin, "--version").Output()
 	if err != nil {
-		return nil
+		resp.RemoveAttribute(qemuDriverAttr)
+		return err
 	}
 	out := strings.TrimSpace(string(outBytes))
 
 	matches := reQemuVersion.FindStringSubmatch(out)
 	if len(matches) != 2 {
+		resp.RemoveAttribute(qemuDriverAttr)
 		return fmt.Errorf("Unable to parse Qemu version string: %#v", matches)
 	}
 	currentQemuVersion := matches[1]

client/driver/raw_exec.go

@@ -101,6 +101,7 @@ func (d *RawExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstr
 		return nil
 	}
 
+	resp.RemoveAttribute(rawExecDriverAttr)
 	return nil
 }
 

client/driver/rkt.go

@@ -318,6 +318,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
 			d.logger.Printf("[DEBUG] driver.rkt: must run as root user, disabling")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(rktDriverAttr)
 		return nil
 	}
 
@@ -332,6 +333,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
 	appcMatches := reAppcVersion.FindStringSubmatch(out)
 	if len(rktMatches) != 2 || len(appcMatches) != 2 {
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(rktDriverAttr)
 		return fmt.Errorf("Unable to parse Rkt version string: %#v", rktMatches)
 	}
 
@@ -345,6 +347,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
 				currentVersion, minVersion)
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
+		resp.RemoveAttribute(rktDriverAttr)
 		return nil
 	}
 

client/fingerprint/cgroup.go

@@ -5,6 +5,8 @@ package fingerprint
 import (
 	"log"
 	"time"
+
+	cstructs "github.com/hashicorp/nomad/client/structs"
 )
 
 const (
@@ -45,6 +47,12 @@ func NewCGroupFingerprint(logger *log.Logger) Fingerprint {
 	return f
 }
 
+// clearCGroupAttributes clears any node attributes related to cgroups that might
+// have been set in a previous fingerprint run.
+func (f *CGroupFingerprint) clearCGroupAttributes(r *cstructs.FingerprintResponse) {
+	r.RemoveAttribute("unique.cgroup.mountpoint")
+}
+
 // Periodic determines the interval at which the periodic fingerprinter will run.
 func (f *CGroupFingerprint) Periodic() (bool, time.Duration) {
 	return true, interval * time.Second

client/fingerprint/cgroup_linux.go

@@ -30,11 +30,15 @@ func FindCgroupMountpointDir() (string, error) {
 func (f *CGroupFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
 	mount, err := f.mountPointDetector.MountPoint()
 	if err != nil {
+		f.clearCGroupAttributes(resp)
 		return fmt.Errorf("Failed to discover cgroup mount point: %s", err)
 	}
 
 	// Check if a cgroup mount point was found
 	if mount == "" {
+
+		f.clearCGroupAttributes(resp)
+
 		if f.lastState == cgroupAvailable {
 			f.logger.Printf("[INFO] fingerprint.cgroups: cgroups are unavailable")
 		}

client/fingerprint/consul.go

@@ -47,7 +47,7 @@ func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 	// If we can't hit this URL consul is probably not running on this machine.
 	info, err := f.client.Agent().Self()
 	if err != nil {
-		// TODO this should set consul in the response if the error is not nil
+		f.clearConsulAttributes(resp)
 
 		// Print a message indicating that the Consul Agent is not available
 		// anymore
@@ -105,3 +105,14 @@ func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 func (f *ConsulFingerprint) Periodic() (bool, time.Duration) {
 	return true, 15 * time.Second
 }
+
+// clearConsulAttributes removes consul attributes and links from the passed
+// Node.
+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")
+}

client/fingerprint/vault.go

@@ -52,6 +52,7 @@ func (f *VaultFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *c
 	// Connect to vault and parse its information
 	status, err := f.client.Sys().SealStatus()
 	if err != nil {
+		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")
@@ -79,3 +80,10 @@ func (f *VaultFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *c
 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")
+}

client/structs/structs.go

@@ -210,6 +210,17 @@ func (f *FingerprintResponse) AddAttribute(name, value string) {
 	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] = ""
+}
+
 // GetAttributes fetches the attributes for the fingerprint response
 func (f *FingerprintResponse) GetAttributes() map[string]string {
 	// initialize attributes if it has not been already
@@ -230,6 +241,17 @@ func (f *FingerprintResponse) AddLink(name, value string) {
 	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] = ""
+}
+
 // GetLinks returns the links for the fingerprint response
 func (f *FingerprintResponse) GetLinks() map[string]string {
 	// initialize links if it has not been already