improve comments; update watchDriver

client/client.go

@@ -1024,11 +1024,11 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs.
 
 	if fingerprint != nil {
 		if c.config.Node.Drivers[name] == nil {
-			// if the driver info has not yet been set, do that here
+			// If the driver info has not yet been set, do that here
 			hasChanged = true
 			c.config.Node.Drivers[name] = fingerprint
 		} else {
-			// the driver info has already been set, fix it up
+			// The driver info has already been set, fix it up
 			if c.config.Node.Drivers[name].Detected != fingerprint.Detected {
 				hasChanged = true
 				c.config.Node.Drivers[name].Detected = fingerprint.Detected
@@ -1057,7 +1057,7 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs.
 		} else {
 			oldVal := c.config.Node.Drivers[name]
 			if health.HealthCheckEquals(oldVal) {
-				// make sure we accurately reflect the last time a health check has been
+				// Make sure we accurately reflect the last time a health check has been
 				// performed for the driver.
 				oldVal.UpdateTime = health.UpdateTime
 			} else {
@@ -1067,7 +1067,7 @@ func (c *Client) updateNodeFromDriver(name string, fingerprint, health *structs.
 				events := []*structs.NodeEvent{
 					{
 						Subsystem: "Driver",
-						Message:   fmt.Sprintf("Driver status changed: %s", health.HealthDescription),
+						Message:   health.HealthDescription,
 						Timestamp: time.Now().Unix(),
 					},
 				}

client/driver/docker.go

@@ -569,7 +569,7 @@ func (d *DockerDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstru
 
 	_, err = client.ListContainers(docker.ListContainersOptions{All: false})
 	if err != nil {
-		d.logger.Printf("[WARN] driver.docker: failed to list Docker containers in the process of a docker health check: %v", err)
+		d.logger.Printf("[WARN] driver.docker: failed to list Docker containers in the process of a Docker health check: %v", err)
 		resp.AddDriverInfo("docker", unhealthy)
 		return err
 	}

client/fingerprint_manager.go

@@ -25,6 +25,8 @@ type FingerprintManager struct {
 	// associated node
 	updateNodeAttributes func(*cstructs.FingerprintResponse) *structs.Node
 
+	// updateNodeFromDriver is a callback to the client to update the state of a
+	// specific driver for the node
 	updateNodeFromDriver func(string, *structs.DriverInfo, *structs.DriverInfo) *structs.Node
 	logger               *log.Logger
 }
@@ -85,29 +87,26 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error {
 
 		detected, err := fm.fingerprintDriver(name, d)
 		if err != nil {
-			fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting for %v failed: %+v", name, err)
+			fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting driver %v failed: %+v", name, err)
 			return err
 		}
 
-		if hc, ok := d.(fingerprint.HealthCheck); ok {
-			fm.runHealthCheck(name, hc)
-		} else {
-			// for drivers which are not of type health check, update them to have their
-			// health status match the status of whether they are detected or not.
-			healthInfo := &structs.DriverInfo{
-				Healthy:    detected,
-				UpdateTime: time.Now(),
-			}
-			if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil {
-				fm.nodeLock.Lock()
-				fm.node = node
-				fm.nodeLock.Unlock()
-			}
+		// For all drivers upon initialization, create a driver info which matches
+		// its fingerprint status. Later, for drivers that have the health check
+		// interface implemented, a periodic health check will be run
+		healthInfo := &structs.DriverInfo{
+			Healthy:    detected,
+			UpdateTime: time.Now(),
+		}
+		if node := fm.updateNodeFromDriver(name, nil, healthInfo); node != nil {
+			fm.nodeLock.Lock()
+			fm.node = node
+			fm.nodeLock.Unlock()
 		}
 
 		go fm.watchDriver(d, name)
 
-		// log the fingerprinters which have been applied
+		// Log the fingerprinters which have been applied
 		if detected {
 			availDrivers = append(availDrivers, name)
 		}
@@ -120,45 +119,61 @@ func (fm *FingerprintManager) setupDrivers(drivers []string) error {
 // watchDrivers facilitates the different periods between fingerprint and
 // health checking a driver
 func (fm *FingerprintManager) watchDriver(d driver.Driver, name string) {
-	_, fingerprintPeriod := d.Periodic()
+	isPeriodic, fingerprintPeriod := d.Periodic()
+	if !isPeriodic {
+		return
+	}
+
 	fm.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprinting driver %s every %v", name, fingerprintPeriod)
 
-	var healthCheckPeriod time.Duration
+	fingerprintTimer := time.NewTicker(fingerprintPeriod)
+	defer fingerprintTimer.Stop()
+
 	hc, isHealthCheck := d.(fingerprint.HealthCheck)
 	if isHealthCheck {
+		// For types that implement the health check interface, run both the
+		// fingerprint and health check periodic functions and update the node
 		req := &cstructs.HealthCheckIntervalRequest{}
 		var resp cstructs.HealthCheckIntervalResponse
-		hc.GetHealthCheckInterval(req, &resp)
-		if resp.Eligible {
-			fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking driver %s every %v", name, healthCheckPeriod)
-			healthCheckPeriod = resp.Period
+		err := hc.GetHealthCheckInterval(req, &resp)
+		if err != nil {
+			fm.logger.Printf("[ERR] client.fingerprint_manager: error when getting health check interval: %v", err)
 		}
-	}
 
-	t1 := time.NewTimer(fingerprintPeriod)
-	defer t1.Stop()
-	t2 := time.NewTimer(healthCheckPeriod)
-	defer t2.Stop()
+		fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking driver %s every %v", name, resp.Period)
+		healthCheckTimer := time.NewTicker(resp.Period)
+		defer healthCheckTimer.Stop()
 
-	for {
-		select {
-		case <-fm.shutdownCh:
-			return
-		case <-t1.C:
-			t1.Reset(fingerprintPeriod)
-			_, err := fm.fingerprintDriver(name, d)
-			if err != nil {
-				fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for driver %v failed: %+v", name, err)
-			}
-		case <-t2.C:
-			if isHealthCheck {
-				t2.Reset(healthCheckPeriod)
+		for {
+			select {
+			case <-fm.shutdownCh:
+				return
+			case <-fingerprintTimer.C:
+				_, err := fm.fingerprintDriver(name, d)
+				if err != nil {
+					fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for driver %v failed: %+v", name, err)
+				}
+			case <-healthCheckTimer.C:
 				err := fm.runHealthCheck(name, hc)
 				if err != nil {
 					fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking for %v failed: %v", name, err)
 				}
 			}
 		}
+	} else {
+		// For types that do not have a health check capacity, run only the
+		// periodic fingerprint method
+		for {
+			select {
+			case <-fm.shutdownCh:
+				return
+			case <-fingerprintTimer.C:
+				_, err := fm.fingerprintDriver(name, d)
+				if err != nil {
+					fm.logger.Printf("[DEBUG] client.fingerprint_manager: periodic fingerprinting for driver %v failed: %+v", name, err)
+				}
+			}
+		}
 	}
 }
 
@@ -239,14 +254,17 @@ func (fm *FingerprintManager) runHealthCheck(name string, hc fingerprint.HealthC
 	request := &cstructs.HealthCheckRequest{}
 	var response cstructs.HealthCheckResponse
 	err := hc.HealthCheck(request, &response)
+	if err != nil {
+		return err
+	}
 
 	if node := fm.updateNodeFromDriver(name, nil, response.Drivers[name]); node != nil {
 		fm.nodeLock.Lock()
 		fm.node = node
 		fm.nodeLock.Unlock()
 	}
 
-	return err
+	return nil
 }
 
 // setupFingerprints is used to fingerprint the node to see if these attributes are
@@ -287,7 +305,7 @@ func (fm *FingerprintManager) setupFingerprinters(fingerprints []string) error {
 // those which require periotic checking, it starts a periodic process for
 // each.
 func (fp *FingerprintManager) Run() error {
-	// first, set up all fingerprints
+	// First, set up all fingerprints
 	cfg := fp.getConfig()
 	whitelistFingerprints := cfg.ReadStringListToMap("fingerprint.whitelist")
 	whitelistFingerprintsEnabled := len(whitelistFingerprints) > 0
@@ -320,7 +338,7 @@ func (fp *FingerprintManager) Run() error {
 		fp.logger.Printf("[DEBUG] client.fingerprint_manager: fingerprint modules skipped due to white/blacklist: %v", skippedFingerprints)
 	}
 
-	// next, set up drivers
+	// Next, set up drivers
 	// Build the white/blacklists of drivers.
 	whitelistDrivers := cfg.ReadStringListToMap("driver.whitelist")
 	whitelistDriversEnabled := len(whitelistDrivers) > 0

scheduler/feasible.go

@@ -130,7 +130,7 @@ func (c *DriverChecker) hasDrivers(option *structs.Node) bool {
 	for driver := range c.drivers {
 		driverStr := fmt.Sprintf("driver.%s", driver)
 
-		// COMPAT: Remove in 0.X: As of Nomad 0.8, nodes have a DriverInfo that
+		// COMPAT: Remove in 0.10: As of Nomad 0.8, nodes have a DriverInfo that
 		// corresponds with every driver. As a Nomad server might be on a later
 		// version than a Nomad client, we need to check for compatibility here
 		// to verify the client supports this.