Driver health detection cleanups

This PR does:

1. Health message based on detection has format "Driver XXX detected"
and "Driver XXX not detected"
2. Set initial health description based on detection status and don't
wait for the first health check.
3. Combine updating attributes on the node, fingerprint and health
checking update for drivers into a single call back.
4. Condensed driver info in `node status` only shows detected drivers
and make the output less wide by removing spaces.

client/client.go

@@ -1023,11 +1023,15 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs.
 
 	var hasChanged bool
 
+	hadDriver := c.config.Node.Drivers[name] != nil
 	if fingerprint != nil {
-		if c.config.Node.Drivers[name] == nil {
+		if !hadDriver {
 			// If the driver info has not yet been set, do that here
 			hasChanged = true
 			c.config.Node.Drivers[name] = fingerprint
+			for attrName, newVal := range fingerprint.Attributes {
+				c.config.Node.Attributes[attrName] = newVal
+			}
 		} else {
 			// The driver info has already been set, fix it up
 			if c.config.Node.Drivers[name].Detected != fingerprint.Detected {
@@ -1052,9 +1056,13 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs.
 	}
 
 	if health != nil {
-		if c.config.Node.Drivers[name] == nil {
+		if !hadDriver {
 			hasChanged = true
-			c.config.Node.Drivers[name] = health
+			if info, ok := c.config.Node.Drivers[name]; !ok {
+				c.config.Node.Drivers[name] = health
+			} else {
+				info.MergeHealthCheck(health)
+			}
 		} else {
 			oldVal := c.config.Node.Drivers[name]
 			if health.HealthCheckEquals(oldVal) {

client/driver/rkt.go

@@ -370,7 +370,7 @@ func (d *RktDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
 	}
 
 	// Output version information when the fingerprinter first sees rkt
-	if req.Node.Attributes[rktDriverAttr] != "1" {
+	if info, ok := req.Node.Drivers["rkt"]; ok && info != nil && !info.Detected {
 		d.logger.Printf("[DEBUG] driver.rkt: detect version: %s", strings.Replace(out, "\n", " ", -1))
 	}
 	resp.AddAttribute(rktDriverAttr, "1")

client/fingerprint_manager.go

@@ -187,17 +187,6 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error {
 			return err
 		}
 
-		// Set the initial health check status to be the driver detected status.
-		// Later, the periodic health checker will update this value for drivers
-		// where health checks are enabled.
-		healthInfo := &structs.DriverInfo{
-			Healthy:    detected,
-			UpdateTime: time.Now(),
-		}
-		if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil {
-			fm.setNode(node)
-		}
-
 		// Start a periodic watcher to detect changes to a drivers health and
 		// attributes.
 		go fm.watchDriver(d, name)
@@ -336,6 +325,18 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge
 	var response cstructs.FingerprintResponse
 
 	fm.nodeLock.Lock()
+
+	// Determine if the driver has been detected before.
+	originalNode, haveDriver := fm.node.Drivers[name]
+	firstDetection := !haveDriver
+
+	// Determine if the driver is healthy
+	var driverIsHealthy bool
+	if haveDriver && originalNode.Healthy {
+		driverIsHealthy = true
+	}
+
+	// Fingerprint the driver.
 	request := &cstructs.FingerprintRequest{Config: fm.getConfig(), Node: fm.node}
 	err := f.Fingerprint(request, &response)
 	fm.nodeLock.Unlock()
@@ -344,45 +345,39 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge
 		return false, err
 	}
 
-	if node := fm.updateNodeAttributes(&response); node != nil {
-		fm.setNode(node)
-	}
-
-	di := &structs.DriverInfo{
+	fingerprintInfo := &structs.DriverInfo{
 		Attributes: response.Attributes,
 		Detected:   response.Detected,
 	}
 
 	// Remove the attribute indicating the status of the driver, as the overall
 	// driver info object should indicate this.
-	driverKey := fmt.Sprintf("driver.%s", name)
-	delete(di.Attributes, driverKey)
-
-	if node := fm.updateNodeFromDriver(name, di, nil); node != nil {
-		fm.setNode(node)
-	}
-
-	// Get a copy of the current node
-	var driverExists, driverIsHealthy bool
-	fm.nodeLock.Lock()
-	driverInfo, driverExists := fm.node.Drivers[name]
-	if driverExists {
-		driverIsHealthy = driverInfo.Healthy
-	}
-	fm.nodeLock.Unlock()
+	delete(fingerprintInfo.Attributes, fmt.Sprintf("driver.%s", name))
+
+	// We set the health status based on the detection state of the driver if:
+	// * It is the first time we are fingerprinting the driver. This gives all
+	// drivers an initial health.
+	// * If the driver becomes undetected. This gives us an immediate unhealthy
+	// state and description when it transistions from detected and healthy to
+	// undetected.
+	// * If the driver does not have its own health checks. Then we always
+	// couple the states.
+	var healthInfo *structs.DriverInfo
+	if firstDetection || !hasPeriodicHealthCheck || !response.Detected && driverIsHealthy {
+		state := " "
+		if !response.Detected {
+			state = " not "
+		}
 
-	// If either 1) the driver is undetected or 2) if the driver does not have
-	// periodic health checks enabled, set the health status to the match that
-	// of the fingerprinter
-	if !hasPeriodicHealthCheck || !response.Detected && driverExists && driverIsHealthy {
-		healthInfo := &structs.DriverInfo{
+		healthInfo = &structs.DriverInfo{
 			Healthy:           response.Detected,
-			HealthDescription: fmt.Sprintf("Driver %s is detected: %t", name, response.Detected),
+			HealthDescription: fmt.Sprintf("Driver %s is%sdetected", name, state),
 			UpdateTime:        time.Now(),
 		}
-		if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil {
-			fm.setNode(node)
-		}
+	}
+
+	if node := fm.updateNodeFromDriver(name, fingerprintInfo, healthInfo); node != nil {
+		fm.setNode(node)
 	}
 
 	return response.Detected, nil