remove attributes from periodic fingerprints when state changes

write test for client periodic fingerprinters

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,46 @@ 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, func(c *config.Config) {
+		c.Options = map[string]string{
+			"test.shutdown_periodic_after":    "true",
+			"test.shutdown_periodic_duration": "3",
+		}
+	})
+	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) {

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

@@ -77,14 +77,30 @@ type MockDriverConfig struct {
 // MockDriver is a driver which is used for testing purposes
 type MockDriver struct {
 	DriverContext
-	fingerprint.StaticFingerprinter
+
+	// isShutdown is an internal concept to use to track whether the driver
+	// should be shut down
+	isShutdown bool
 
 	cleanupFailNum int
 }
 
 // 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(fingerprint.ShutdownPeriodicAfter, false) {
+		duration, err := ctx.config.ReadInt(fingerprint.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)
+		}
+		go md.startShutdownTimer(duration)
+	}
+
+	return md
 }
 
 func (d *MockDriver) Abilities() DriverAbilities {
@@ -165,6 +181,20 @@ func (m *MockDriver) Start(ctx *ExecContext, task *structs.Task) (*StartResponse
 	return &StartResponse{Handle: &h, Network: net}, nil
 }
 
+// startShutdownTimer sets a timer, after which the mock driver will no loger be
+// responsive. This is used for testing periodic fingerprinting functionality
+func (m *MockDriver) startShutdownTimer(duration int) {
+	timer := time.NewTimer(time.Duration(duration) * time.Second)
+	for {
+		select {
+		case <-timer.C:
+			m.isShutdown = true
+		default:
+			time.Sleep(100 * time.Millisecond)
+		}
+	}
+}
+
 // Cleanup deletes all keys except for Config.Options["cleanup_fail_on"] for
 // Config.Options["cleanup_fail_num"] times. For failures it will return a
 // recoverable error.
@@ -194,7 +224,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 +373,8 @@ func (h *mockDriverHandle) run() {
 		}
 	}
 }
+
+// When testing, poll for updates
+func (m *MockDriver) Periodic() (bool, time.Duration) {
+	return true, 500 * time.Millisecond
+}

client/driver/qemu.go

@@ -163,12 +163,15 @@ func (d *QemuDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	}
 	outBytes, err := exec.Command(bin, "--version").Output()
 	if err != 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 {
+		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
@@ -102,6 +102,17 @@ func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 	return nil
 }
 
+// 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")
+}
+
 func (f *ConsulFingerprint) Periodic() (bool, time.Duration) {
 	return true, 15 * time.Second
 }

client/fingerprint/fingerprint.go

@@ -16,6 +16,16 @@ const (
 	// TightenNetworkTimeoutsConfig is a config key that can be used during
 	// tests to tighten the timeouts for fingerprinters that make network calls.
 	TightenNetworkTimeoutsConfig = "test.tighten_network_timeouts"
+
+	// 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"
 )
 
 func init() {

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