driver info should be set by both fingerprint and health check

client/client.go

@@ -952,6 +952,18 @@ func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintRespons
 		c.config.Node.Resources.Merge(response.Resources)
 	}
 
+	for name, new_val := range response.Drivers {
+		old_val := c.config.Node.Drivers[name]
+		if new_val.Equals(old_val) {
+			continue
+		}
+		if old_val == nil {
+			c.config.Node.Drivers[name] = new_val
+		} else {
+			c.config.Node.Drivers[name].MergeFingerprintInfo(new_val)
+		}
+	}
+
 	return c.config.Node
 }
 
@@ -965,7 +977,11 @@ func (c *Client) updateNodeFromHealthCheck(response *cstructs.HealthCheckRespons
 		if new_val.Equals(old_val) {
 			continue
 		}
-		c.config.Node.Drivers[name] = new_val
+		if old_val == nil {
+			c.config.Node.Drivers[name] = new_val
+		} else {
+			c.config.Node.Drivers[name].MergeHealthCheck(new_val)
+		}
 	}
 
 	return c.config.Node

client/client_test.go

@@ -278,6 +278,21 @@ func TestClient_Fingerprint_Periodic(t *testing.T) {
 		if mockDriverStatus == "" {
 			return false, fmt.Errorf("mock driver attribute should be set on the client")
 		}
+
+		// assert that the Driver information for the node is also set correctly
+		mockDriverInfo := node.Drivers["mock_driver"]
+		if mockDriverInfo == nil {
+			return false, fmt.Errorf("mock driver is nil when it should be set on node Drivers")
+		}
+		if !mockDriverInfo.Detected {
+			return false, fmt.Errorf("mock driver should be set as healthy")
+		}
+		if !mockDriverInfo.Healthy {
+			return false, fmt.Errorf("mock driver should be set as healthy")
+		}
+		if mockDriverInfo.HealthDescription == "" {
+			return false, fmt.Errorf("mock driver description should not be empty")
+		}
 		return true, nil
 	}, func(err error) {
 		t.Fatalf("err: %v", err)

client/driver/docker.go

@@ -551,6 +551,9 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
 	return nil
 }
 
+// HealthChck implements the interface for the HealthCheck interface. This
+// performs a health check on the docker driver, asserting whether the docker
+// driver is responsive to a `docker ps` command.
 func (d *DockerDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstructs.HealthCheckResponse) error {
 	unhealthy := &structs.DriverInfo{
 		HealthDescription: "Docker driver is available but unresponsive",
@@ -560,7 +563,7 @@ func (d *DockerDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstru
 	_, _, err := d.dockerClients()
 	if err != nil {
 		d.logger.Printf("[WARN] driver.docker: docker driver is available but is unresponsive to `docker ps`")
-		resp.AddDriverInfo("driver.docker", unhealthy)
+		resp.AddDriverInfo("docker", unhealthy)
 		return err
 	}
 
@@ -570,10 +573,13 @@ func (d *DockerDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstru
 		HealthDescription: "Docker driver is available and responsive",
 		UpdateTime:        time.Now(),
 	}
-	resp.AddDriverInfo("driver.docker", healthy)
+	resp.AddDriverInfo("docker", healthy)
 	return nil
 }
 
+// GetHealthChecks implements the interface for the HealthCheck interface. This
+// sets whether the driver is eligible for periodic health checks and the
+// interval at which to do them.
 func (d *DockerDriver) GetHealthCheckInterval(req *cstructs.HealthCheckIntervalRequest, resp *cstructs.HealthCheckIntervalResponse) error {
 	resp.Eligible = true
 	resp.Period = 1 * time.Minute

client/driver/docker_test.go

@@ -289,7 +289,7 @@ func TestDockerDriver_Check_DockerHealthStatus(t *testing.T) {
 	err = dc.HealthCheck(request, &response)
 	require.Nil(err)
 
-	driverInfo := response.Drivers["driver.docker"]
+	driverInfo := response.Drivers["docker"]
 	require.NotNil(driverInfo)
 	require.True(driverInfo.Healthy)
 }

client/driver/mock_driver.go

@@ -238,22 +238,24 @@ func (m *MockDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
 // HealthCheck implements the interface for HealthCheck, and indicates the current
 // health status of the mock driver.
 func (m *MockDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstructs.HealthCheckResponse) error {
-	if !m.shutdownFingerprintTime.IsZero() && time.Now().After(m.shutdownFingerprintTime) {
+	switch {
+	case !m.shutdownFingerprintTime.IsZero() && time.Now().After(m.shutdownFingerprintTime):
 		notHealthy := &structs.DriverInfo{
 			Healthy:           false,
 			HealthDescription: "not running",
 			UpdateTime:        time.Now(),
 		}
-		resp.AddDriverInfo(mockDriverName, notHealthy)
+		resp.AddDriverInfo("mock_driver", notHealthy)
+		return nil
+	default:
+		healthy := &structs.DriverInfo{
+			Healthy:           true,
+			HealthDescription: "running",
+			UpdateTime:        time.Now(),
+		}
+		resp.AddDriverInfo("mock_driver", healthy)
 		return nil
 	}
-	healthy := &structs.DriverInfo{
-		Healthy:           true,
-		HealthDescription: "running",
-		UpdateTime:        time.Now(),
-	}
-	resp.AddDriverInfo(mockDriverName, healthy)
-	return nil
 }
 
 // GetHealthCheckInterval implements the interface for HealthCheck and indicates

client/fingerprint_manager.go

@@ -114,7 +114,10 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error {
 			go fm.runFingerprint(d, period, name)
 		}
 
-		if hc, ok := d.(fingerprint.HealthCheck); ok {
+		// We should only run the health check task if the driver is detected
+		// Note that if the driver is detected later in a periodic health check,
+		// this won't automateically trigger the periodic health check.
+		if hc, ok := d.(fingerprint.HealthCheck); ok && detected {
 			req := &cstructs.HealthCheckIntervalRequest{}
 			resp := &cstructs.HealthCheckIntervalResponse{}
 			hc.GetHealthCheckInterval(req, resp)
@@ -139,38 +142,37 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge
 		return false, err
 	}
 
-	fm.nodeLock.Lock()
+	// TODO This is a temporary measure, as eventually all drivers will need to
+	// support this. Doing this so that we can enable this iteratively and also
+	// in a backwards compatible way, where node attributes for drivers will
+	// eventually be phased out.
+	di := &structs.DriverInfo{
+		Attributes: response.Attributes,
+		Detected:   response.Detected,
+	}
+	response.AddDriver(name, di)
+
 	if node := fm.updateNodeAttributes(&response); node != nil {
+		fm.nodeLock.Lock()
 		fm.node = node
+		fm.nodeLock.Unlock()
 	}
-	fm.nodeLock.Unlock()
 
 	if hc, ok := f.(fingerprint.HealthCheck); ok {
 		fm.healthCheck(name, hc)
 	} else {
-		// This is a temporary measure, as eventually all drivers will need to
-		// support this. Doing this so that we can enable this iteratively and also
-		// in a backwards compatible way, where node attributes for drivers will
-		// eventually be phased out.
-
-		di := &structs.DriverInfo{
-			Attributes: response.Attributes,
-			Detected:   response.Detected,
-			Healthy:    response.Detected,
-			UpdateTime: time.Now(),
-		}
 
 		resp := &cstructs.HealthCheckResponse{
 			Drivers: map[string]*structs.DriverInfo{
 				name: di,
 			},
 		}
 
-		fm.nodeLock.Lock()
 		if node := fm.updateHealthCheck(resp); node != nil {
+			fm.nodeLock.Lock()
 			fm.node = node
+			fm.nodeLock.Unlock()
 		}
-		fm.nodeLock.Unlock()
 	}
 
 	return response.Detected, nil

client/fingerprint_manager_test.go

@@ -56,6 +56,7 @@ func TestFingerprintManager_Fingerprint_Run(t *testing.T) {
 
 	node := &structs.Node{
 		Attributes: make(map[string]string, 0),
+		Drivers:    make(map[string]*structs.DriverInfo, 0),
 	}
 
 	conf := config.DefaultConfig()
@@ -68,6 +69,9 @@ func TestFingerprintManager_Fingerprint_Run(t *testing.T) {
 		for k, v := range r.Attributes {
 			node.Attributes[k] = v
 		}
+		for k, v := range r.Drivers {
+			node.Drivers[k] = v
+		}
 		return node
 	}
 	updateHealthCheck := func(resp *cstructs.HealthCheckResponse) *structs.Node {
@@ -87,6 +91,7 @@ func TestFingerprintManager_Fingerprint_Run(t *testing.T) {
 	require.Nil(err)
 
 	require.NotEqual("", node.Attributes["driver.raw_exec"])
+	require.True(node.Drivers["raw_exec"].Detected)
 }
 
 func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) {
@@ -183,6 +188,7 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) {
 	}
 	conf := config.DefaultConfig()
 	conf.Options = map[string]string{
+		"driver.raw_exec.enable":          "1",
 		"test.shutdown_periodic_after":    "true",
 		"test.shutdown_periodic_duration": "2",
 	}
@@ -213,7 +219,7 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) {
 		if mockDriverAttribute == "" {
 			return false, fmt.Errorf("mock driver info should be set on the client attributes")
 		}
-		mockDriverInfo := node.Drivers["driver.mock_driver"]
+		mockDriverInfo := node.Drivers["mock_driver"]
 		if mockDriverInfo == nil {
 			return false, fmt.Errorf("mock driver info should be set on the client")
 		}
@@ -222,27 +228,26 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) {
 		}
 		return true, nil
 	}, func(err error) {
-
-		// Ensure that a default driver without health checks enabled is registered and healthy on the client
-		testutil.WaitForResult(func() (bool, error) {
-			rawExecAttribute := node.Attributes["driver.raw_exec"]
-			if rawExecAttribute == "" {
-				return false, fmt.Errorf("raw exec info should be set on the client attributes")
-			}
-			rawExecInfo := node.Drivers["driver.raw_exec"]
-			if rawExecInfo == nil {
-				return false, fmt.Errorf("raw exec info should be set on the client")
-			}
-			if !rawExecInfo.Healthy {
-				return false, fmt.Errorf("raw exec info should be healthy")
-			}
-			return true, nil
-		}, func(err error) {
-			t.Fatalf("err: %v", err)
-		})
 		t.Fatalf("err: %v", err)
 	})
 
+	// Ensure that a default driver without health checks enabled is registered and healthy on the client
+	testutil.WaitForResult(func() (bool, error) {
+		rawExecAttribute := node.Attributes["driver.raw_exec"]
+		if rawExecAttribute == "" {
+			return false, fmt.Errorf("raw exec info should be set on the client attributes")
+		}
+		rawExecInfo := node.Drivers["raw_exec"]
+		if rawExecInfo == nil {
+			return false, fmt.Errorf("raw exec driver info should be set on the client")
+		}
+		if !rawExecInfo.Detected {
+			return false, fmt.Errorf("raw exec driver should be detected")
+		}
+		return true, nil
+	}, func(err error) {
+		t.Fatalf("err: %v", err)
+	})
 }
 
 func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) {
@@ -252,12 +257,23 @@ func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) {
 	node := &structs.Node{
 		Drivers: make(map[string]*structs.DriverInfo, 0),
 	}
-	updateNode := func(r *cstructs.FingerprintResponse) *structs.Node {
+	updateNode := func(resp *cstructs.FingerprintResponse) *structs.Node {
+		for k, v := range resp.Drivers {
+			if node.Drivers[k] == nil {
+				node.Drivers[k] = v
+			} else {
+				node.Drivers[k].MergeFingerprintInfo(v)
+			}
+		}
 		return node
 	}
 	updateHealthCheck := func(resp *cstructs.HealthCheckResponse) *structs.Node {
 		for k, v := range resp.Drivers {
-			node.Drivers[k] = v
+			if node.Drivers[k] == nil {
+				node.Drivers[k] = v
+			} else {
+				node.Drivers[k].MergeHealthCheck(v)
+			}
 		}
 		return node
 	}
@@ -289,10 +305,13 @@ func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) {
 
 	// Ensure the mock driver is registered and healthy on the client
 	testutil.WaitForResult(func() (bool, error) {
-		mockDriverInfo := node.Drivers["driver.mock_driver"]
+		mockDriverInfo := node.Drivers["mock_driver"]
 		if mockDriverInfo == nil {
 			return false, fmt.Errorf("mock driver info should be set on the client")
 		}
+		if !mockDriverInfo.Detected {
+			return false, fmt.Errorf("mock driver info should be detected")
+		}
 		if !mockDriverInfo.Healthy {
 			return false, fmt.Errorf("mock driver info should be healthy")
 		}
@@ -304,12 +323,16 @@ func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) {
 	// Ensure that the client health check eventually removes this attribute and
 	// marks the driver as unhealthy
 	testutil.WaitForResult(func() (bool, error) {
-		mockDriverInfo := node.Drivers["driver.mock_driver"]
+
+		mockDriverInfo := node.Drivers["mock_driver"]
 		if mockDriverInfo == nil {
 			return false, fmt.Errorf("mock driver info should be set on the client")
 		}
+		if !mockDriverInfo.Detected {
+			return false, fmt.Errorf("mock driver should be detected")
+		}
 		if mockDriverInfo.Healthy {
-			return false, fmt.Errorf("mock driver info should not be healthy")
+			return false, fmt.Errorf("mock driver should not be healthy")
 		}
 		return true, nil
 	}, func(err error) {

client/structs/structs.go

@@ -206,6 +206,19 @@ type FingerprintResponse struct {
 	// Detected is a boolean indicating whether the fingerprinter detected
 	// if the resource was available
 	Detected bool
+
+	// Drivers is a map of driver names to driver info. This allows the
+	// fingerprint method of each driver to set whether the driver is enabled or
+	// not, as well as its attributes
+	Drivers map[string]*structs.DriverInfo
+}
+
+func (f *FingerprintResponse) AddDriver(name string, value *structs.DriverInfo) {
+	if f.Drivers == nil {
+		f.Drivers = make(map[string]*structs.DriverInfo, 0)
+	}
+
+	f.Drivers[name] = value
 }
 
 // AddAttribute adds the name and value for a node attribute to the fingerprint

nomad/structs/structs.go

@@ -1072,6 +1072,17 @@ type DriverInfo struct {
 	UpdateTime        time.Time
 }
 
+func (di *DriverInfo) MergeHealthCheck(other *DriverInfo) {
+	di.Healthy = other.Healthy
+	di.HealthDescription = other.HealthDescription
+	di.UpdateTime = other.UpdateTime
+}
+
+func (di *DriverInfo) MergeFingerprintInfo(other *DriverInfo) {
+	di.Detected = other.Detected
+	di.Attributes = other.Attributes
+}
+
 func (di *DriverInfo) Equals(other *DriverInfo) bool {
 	if di == nil && other == nil {
 		return true