code review fixup

client/client.go

@@ -961,7 +961,7 @@ func (c *Client) fingerprint() error {
 		}
 
 		// log the fingerprinters which have been applied
-		if response.Applicable {
+		if response.Detected {
 			appliedFingerprints = append(appliedFingerprints, name)
 		}
 
@@ -1039,13 +1039,14 @@ func (c *Client) setupDrivers() error {
 		request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
 		var response cstructs.FingerprintResponse
 		c.configLock.Lock()
-		if err := d.Fingerprint(request, &response); err != nil {
+		err = d.Fingerprint(request, &response)
+		c.configLock.Unlock()
+		if err != nil {
 			return err
 		}
-		c.configLock.Unlock()
 
 		// log the fingerprinters which have been applied
-		if response.Applicable {
+		if response.Detected {
 			availDrivers = append(availDrivers, name)
 		}
 

client/client_test.go

@@ -261,8 +261,8 @@ func TestClient_Fingerprint_Periodic(t *testing.T) {
 
 	c1 := testClient(t, func(c *config.Config) {
 		c.Options = map[string]string{
-			"test.shutdown_periodic_after":    "true",
-			"test.shutdown_periodic_duration": "3",
+			driver.ShutdownPeriodicAfter:    "true",
+			driver.ShutdownPeriodicDuration: "3",
 		}
 	})
 	defer c1.Shutdown()

client/driver/docker.go

@@ -501,7 +501,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
 
 	resp.AddAttribute(dockerDriverAttr, "1")
 	resp.AddAttribute("driver.docker.version", env.Get("Version"))
-	resp.Applicable = true
+	resp.Detected = true
 
 	privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false)
 	if privileged {

client/driver/docker_test.go

@@ -190,7 +190,7 @@ func TestDockerDriver_Fingerprint(t *testing.T) {
 
 		// if docker is available, make sure that the response is tagged as
 		// applicable
-		if !response.Applicable {
+		if !response.Detected {
 			t.Fatalf("expected response to be applicable")
 		}
 	}
@@ -232,7 +232,7 @@ func TestDockerDriver_Fingerprint_Bridge(t *testing.T) {
 		t.Fatalf("error fingerprinting docker: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/driver/exec_default.go

@@ -9,6 +9,6 @@ import (
 
 func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
 	d.fingerprintSuccess = helper.BoolToPtr(false)
-	resp.Applicable = true
+	resp.Detected = true
 	return nil
 }

client/driver/exec_linux.go

@@ -16,7 +16,7 @@ func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	// Only enable if cgroups are available and we are root
 	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)
 		resp.RemoveAttribute(execDriverAttr)
@@ -35,6 +35,6 @@ func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	}
 	resp.AddAttribute(execDriverAttr, "1")
 	d.fingerprintSuccess = helper.BoolToPtr(true)
-	resp.Applicable = true
+	resp.Detected = true
 	return nil
 }

client/driver/exec_test.go

@@ -46,7 +46,7 @@ func TestExecDriver_Fingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/driver/java.go

@@ -115,7 +115,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	// Only enable if we are root and cgroups are mounted when running on linux systems.
 	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")
 		}
 		d.fingerprintSuccess = helper.BoolToPtr(false)
 		resp.RemoveAttribute(javaDriverAttr)
@@ -170,7 +170,7 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 	resp.AddAttribute("driver.java.runtime", info[1])
 	resp.AddAttribute("driver.java.vm", info[2])
 	d.fingerprintSuccess = helper.BoolToPtr(true)
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/driver/java_test.go

@@ -58,7 +58,7 @@ func TestJavaDriver_Fingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/driver/lxc.go

@@ -196,7 +196,7 @@ func (d *LxcDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
 	}
 	resp.AddAttribute("driver.lxc.version", version)
 	resp.AddAttribute("driver.lxc", "1")
-	resp.Applicable = true
+	resp.Detected = true
 
 	// Advertise if this node supports lxc volumes
 	if d.config.ReadBoolDefault(lxcVolumesConfigOption, lxcVolumesConfigDefault) {

client/driver/lxc_test.go

@@ -60,7 +60,7 @@ func TestLxcDriver_Fingerprint(t *testing.T) {
 			t.Fatalf("err: %v", err)
 		}
 
-		if !response.Applicable {
+		if !response.Detected {
 			t.Fatalf("expected response to be applicable")
 		}
 

client/driver/mock_driver.go

@@ -16,11 +16,23 @@ 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"
 )
 
+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,11 +90,10 @@ type MockDriverConfig struct {
 type MockDriver struct {
 	DriverContext
 
-	// isShutdown is an internal concept to use to track whether the driver
-	// should be shut down
-	isShutdown bool
-
 	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
@@ -91,13 +102,13 @@ func NewMockDriver(ctx *DriverContext) Driver {
 
 	// 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 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)
 		}
-		go md.startShutdownTimer(duration)
+		md.shutdownFingerprintTime = time.Now().Add(time.Second * time.Duration(duration))
 	}
 
 	return md
@@ -181,20 +192,6 @@ 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.
@@ -225,11 +222,14 @@ 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 {
 	switch {
-	case m.isShutdown:
+	// 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.Applicable = true
+		resp.Detected = true
 	}
 	return nil
 }

client/driver/qemu.go

@@ -178,7 +178,7 @@ func (d *QemuDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 
 	resp.AddAttribute(qemuDriverAttr, "1")
 	resp.AddAttribute(qemuDriverVersionAttr, currentQemuVersion)
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/driver/qemu_test.go

@@ -43,7 +43,7 @@ func TestQemuDriver_Fingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/driver/raw_exec.go

@@ -98,7 +98,7 @@ func (d *RawExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstr
 	if enabled || req.Config.DevMode {
 		d.logger.Printf("[WARN] driver.raw_exec: raw exec is enabled. Only enable if needed")
 		resp.AddAttribute(rawExecDriverAttr, "1")
-		resp.Applicable = true
+		resp.Detected = true
 		return nil
 	}
 

client/driver/raw_exec_test.go

@@ -53,7 +53,7 @@ func TestRawExecDriver_Fingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/driver/rkt.go

@@ -354,7 +354,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
 	resp.AddAttribute(rktDriverAttr, "1")
 	resp.AddAttribute("driver.rkt.version", rktMatches[1])
 	resp.AddAttribute("driver.rkt.appc.version", appcMatches[1])
-	resp.Applicable = true
+	resp.Detected = true
 
 	// Advertise if this node supports rkt volumes
 	if d.config.ReadBoolDefault(rktVolumesConfigOption, rktVolumesConfigDefault) {

client/driver/rkt_test.go

@@ -66,7 +66,7 @@ func TestRktDriver_Fingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/arch.go

@@ -21,6 +21,6 @@ func NewArchFingerprint(logger *log.Logger) Fingerprint {
 
 func (f *ArchFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
 	resp.AddAttribute("cpu.arch", runtime.GOARCH)
-	resp.Applicable = true
+	resp.Detected = true
 	return nil
 }

client/fingerprint/arch_test.go

@@ -21,7 +21,7 @@ func TestArchFingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/cgroup_linux.go

@@ -47,7 +47,7 @@ func (f *CGroupFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 	}
 
 	resp.AddAttribute("unique.cgroup.mountpoint", mount)
-	resp.Applicable = true
+	resp.Detected = true
 
 	if f.lastState == cgroupUnavailable {
 		f.logger.Printf("[INFO] fingerprint.cgroups: cgroups are available")

client/fingerprint/consul.go

@@ -98,7 +98,7 @@ func (f *ConsulFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 		f.logger.Printf("[INFO] fingerprint.consul: consul agent is available")
 	}
 	f.lastState = consulAvailable
-	resp.Applicable = true
+	resp.Detected = true
 	return nil
 }
 

client/fingerprint/consul_test.go

@@ -35,7 +35,7 @@ func TestConsulFingerprint(t *testing.T) {
 		t.Fatalf("Failed to fingerprint: %s", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 
@@ -189,7 +189,7 @@ func TestConsulFingerprint_UnexpectedResponse(t *testing.T) {
 	err := fp.Fingerprint(request, &response)
 	assert.Nil(err)
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/cpu.go

@@ -68,7 +68,7 @@ func (f *CPUFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cst
 
 	resp.AddAttribute("cpu.totalcompute", fmt.Sprintf("%d", tt))
 	setResourcesCPU(tt)
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/fingerprint/cpu_test.go

@@ -21,7 +21,7 @@ func TestCPUFingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 
@@ -67,7 +67,7 @@ func TestCPUFingerprint_OverrideCompute(t *testing.T) {
 			t.Fatalf("err: %v", err)
 		}
 
-		if !response.Applicable {
+		if !response.Detected {
 			t.Fatalf("expected response to be applicable")
 		}
 

client/fingerprint/env_aws.go

@@ -173,7 +173,7 @@ func (f *EnvAWSFingerprint) Fingerprint(request *cstructs.FingerprintRequest, re
 	response.AddLink("aws.ec2", fmt.Sprintf("%s.%s",
 		response.Attributes["platform.aws.placement.availability-zone"],
 		response.Attributes["unique.platform.aws.instance-id"]))
-	response.Applicable = true
+	response.Detected = true
 
 	return nil
 }

client/fingerprint/env_aws_test.go

@@ -229,7 +229,7 @@ func TestNetworkFingerprint_AWS_network(t *testing.T) {
 			t.Fatalf("err: %v", err)
 		}
 
-		if !response.Applicable {
+		if !response.Detected {
 			t.Fatalf("expected response to be applicable")
 		}
 

client/fingerprint/env_gce.go

@@ -261,7 +261,7 @@ func (f *EnvGCEFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 		resp.AddLink("gce", id)
 	}
 
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/fingerprint/env_gce_test.go

@@ -27,7 +27,7 @@ func TestGCEFingerprint_nonGCE(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if response.Applicable {
+	if response.Detected {
 		t.Fatalf("expected response to not be applicable")
 	}
 
@@ -90,7 +90,7 @@ func testFingerprint_GCE(t *testing.T, withExternalIp bool) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/fingerprint.go

@@ -16,16 +16,6 @@ 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/host.go

@@ -34,7 +34,7 @@ func (f *HostFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *cs
 	resp.AddAttribute("kernel.version", hostInfo.KernelVersion)
 
 	resp.AddAttribute("unique.hostname", hostInfo.Hostname)
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/fingerprint/host_test.go

@@ -21,7 +21,7 @@ func TestHostFingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/network.go

@@ -106,7 +106,7 @@ func (f *NetworkFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp
 	if len(nwResources) > 0 {
 		resp.AddAttribute("unique.network.ip-address", nwResources[0].IP)
 	}
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/fingerprint/network_test.go

@@ -197,7 +197,7 @@ func TestNetworkFingerprint_basic(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 
@@ -248,7 +248,7 @@ func TestNetworkFingerprint_default_device_absent(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if response.Applicable {
+	if response.Detected {
 		t.Fatalf("expected response to not be applicable")
 	}
 
@@ -271,7 +271,7 @@ func TestNetworkFingerPrint_default_device(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 
@@ -322,7 +322,7 @@ func TestNetworkFingerPrint_LinkLocal_Allowed(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 
@@ -369,7 +369,7 @@ func TestNetworkFingerPrint_LinkLocal_Allowed_MixedIntf(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 
@@ -429,7 +429,7 @@ func TestNetworkFingerPrint_LinkLocal_Disallowed(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/nomad.go

@@ -21,6 +21,6 @@ func NewNomadFingerprint(logger *log.Logger) Fingerprint {
 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.Applicable = true
+	resp.Detected = true
 	return nil
 }

client/fingerprint/nomad_test.go

@@ -30,7 +30,7 @@ func TestNomadFingerprint(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/signal.go

@@ -28,6 +28,6 @@ func (f *SignalFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *
 	}
 
 	resp.AddAttribute("os.signals", strings.Join(sigs, ","))
-	resp.Applicable = true
+	resp.Detected = true
 	return nil
 }

client/fingerprint/storage.go

@@ -50,7 +50,7 @@ func (f *StorageFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp
 	resp.Resources = &structs.Resources{
 		DiskMB: int(free / bytesPerMegabyte),
 	}
-	resp.Applicable = true
+	resp.Detected = true
 
 	return nil
 }

client/fingerprint/storage_test.go

@@ -15,7 +15,7 @@ func TestStorageFingerprint(t *testing.T) {
 
 	response := assertFingerprintOK(t, fp, node)
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/fingerprint/vault.go

@@ -74,7 +74,7 @@ func (f *VaultFingerprint) Fingerprint(req *cstructs.FingerprintRequest, resp *c
 		f.logger.Printf("[INFO] fingerprint.vault: Vault is available")
 	}
 	f.lastState = vaultAvailable
-	resp.Applicable = true
+	resp.Detected = true
 	return nil
 }
 

client/fingerprint/vault_test.go

@@ -28,7 +28,7 @@ func TestVaultFingerprint(t *testing.T) {
 		t.Fatalf("Failed to fingerprint: %s", err)
 	}
 
-	if !response.Applicable {
+	if !response.Detected {
 		t.Fatalf("expected response to be applicable")
 	}
 

client/structs/structs.go

@@ -198,9 +198,9 @@ type FingerprintResponse struct {
 	Links      map[string]string
 	Resources  *structs.Resources
 
-	// Applicable is a boolean indicating whether the fingerprint should be
-	// applied
-	Applicable bool
+	// Detected is a boolean indicating whether the fingerprinter detected
+	// if the resource was avaialble
+	Detected bool
 }
 
 // AddAttribute adds the name and value for a node attribute to the fingerprint