From 240fee48486ff4c90b28e44c84ce8177fa78cc69 Mon Sep 17 00:00:00 2001
From: Chelsea Holland Komlo <me@chelseakomlo.com>
Date: Tue, 6 Mar 2018 16:03:24 -0500
Subject: [PATCH] fix up codereview feedback

---
 client/driver/docker.go       |  6 +++---
 client/fingerprint_manager.go | 13 ++++++++++---
 nomad/structs/node.go         |  3 +--
 scheduler/feasible.go         |  8 ++++----
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/client/driver/docker.go b/client/driver/docker.go
index 2812a6fbe..c2fc516ce 100644
--- a/client/driver/docker.go
+++ b/client/driver/docker.go
@@ -551,7 +551,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
 	return nil
 }
 
-// HealthChck implements the interface for the HealthCheck interface. This
+// HealthCheck 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 {
@@ -562,14 +562,14 @@ func (d *DockerDriver) HealthCheck(req *cstructs.HealthCheckRequest, resp *cstru
 
 	client, _, err := d.dockerClients()
 	if err != nil {
-		d.logger.Printf("[WARN] driver.docker: docker driver is available but is unresponsive to `docker ps`")
+		d.logger.Printf("[WARN] driver.docker: failed to retrieve Docker client in the process of a docker health check: %v", err)
 		resp.AddDriverInfo("docker", unhealthy)
 		return err
 	}
 
 	_, err = client.ListContainers(docker.ListContainersOptions{All: false})
 	if err != nil {
-		d.logger.Printf("[WARN] driver.docker: docker driver is available but is unresponsive to `docker ps`")
+		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
 	}
diff --git a/client/fingerprint_manager.go b/client/fingerprint_manager.go
index 2db0704e6..f73bdc9c7 100644
--- a/client/fingerprint_manager.go
+++ b/client/fingerprint_manager.go
@@ -2,6 +2,7 @@ package client
 
 import (
 	"log"
+	"strings"
 	"sync"
 	"time"
 
@@ -24,7 +25,7 @@ type FingerprintManager struct {
 	// associated node
 	updateNodeAttributes func(*cstructs.FingerprintResponse) *structs.Node
 
-	// UpdateHealthCheck is a callback to the client to update the state of the
+	// updateHealthCheck is a callback to the client to update the state of the
 	// node for resources that require a health check
 	updateHealthCheck func(*cstructs.HealthCheckResponse) *structs.Node
 	logger            *log.Logger
@@ -76,7 +77,7 @@ func (fm *FingerprintManager) runHealthCheck(hc fingerprint.HealthCheck, period
 		case <-time.After(period):
 			err := fm.healthCheck(name, hc)
 			if err != nil {
-				fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking for %v failed: %+v", name, err)
+				fm.logger.Printf("[DEBUG] client.fingerprint_manager: health checking for %v failed: %v", name, err)
 				continue
 			}
 
@@ -151,8 +152,14 @@ func (fm *FingerprintManager) fingerprintDriver(name string, f fingerprint.Finge
 	// 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.
+	strippedAttributes := make(map[string]string, 0)
+	for k, v := range response.Attributes {
+		copy := k
+		strings.Replace(copy, "driver.", "", 1)
+		strippedAttributes[k] = v
+	}
 	di := &structs.DriverInfo{
-		Attributes: response.Attributes,
+		Attributes: strippedAttributes,
 		Detected:   response.Detected,
 	}
 	response.AddDriver(name, di)
diff --git a/nomad/structs/node.go b/nomad/structs/node.go
index 7910e1ba7..d8b30bf8f 100644
--- a/nomad/structs/node.go
+++ b/nomad/structs/node.go
@@ -1,7 +1,6 @@
 package structs
 
 import (
-	"strings"
 	"time"
 )
 
@@ -42,7 +41,7 @@ func (di *DriverInfo) HealthCheckEquals(other *DriverInfo) bool {
 		return false
 	}
 
-	if strings.Compare(di.HealthDescription, other.HealthDescription) != 0 {
+	if di.HealthDescription == other.HealthDescription {
 		return false
 	}
 
diff --git a/scheduler/feasible.go b/scheduler/feasible.go
index 359591c80..35582b484 100644
--- a/scheduler/feasible.go
+++ b/scheduler/feasible.go
@@ -130,10 +130,10 @@ func (c *DriverChecker) hasDrivers(option *structs.Node) bool {
 	for driver := range c.drivers {
 		driverStr := fmt.Sprintf("driver.%s", driver)
 
-		// TODO this is a compatibility mechanism- 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.
+		// COMPAT: Remove in 0.X: 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.
 		if option.Drivers != nil {
 			driverInfo := option.Drivers[driver]
 			if driverInfo == nil {