Change Docker ENV behavior

client/driver/docker.go

@@ -47,32 +47,24 @@ func NewDockerDriver(ctx *DriverContext) Driver {
 // to connect to the docker daemon. In production mode we will read
 // docker.endpoint from the config file.
 func (d *DockerDriver) dockerClient() (*docker.Client, error) {
-	// In dev mode, read DOCKER_* environment variables DOCKER_HOST,
-	// DOCKER_TLS_VERIFY, and DOCKER_CERT_PATH. This allows you to run tests and
-	// demo against boot2docker or a VM on OSX and Windows. This falls back on
-	// the default unix socket on linux if tests are run on linux.
-	//
-	// Also note that we need to turn on DevMode in the test configs.
-	if d.config.DevMode {
-		return docker.NewClientFromEnv()
-	}
-
-	// In prod mode we'll read the docker.endpoint configuration and fall back
-	// on the host-specific default. We do not read from the environment.
-	defaultEndpoint, err := docker.DefaultDockerHost()
-	if err != nil {
-		return nil, fmt.Errorf("Unable to determine default docker endpoint: %s", err)
+	// Default to using whatever is configured in docker.endpoint. If this is
+	// not specified we'll fall back on NewClientFromEnv which reads config from
+	// the DOCKER_* environment variables DOCKER_HOST, DOCKER_TLS_VERIFY, and
+	// DOCKER_CERT_PATH. This allows us to lock down the config in production
+	// but also accept the standard ENV configs for dev and test.
+	dockerEndpoint := d.config.Read("docker.endpoint")
+	if dockerEndpoint != "" {
+		return docker.NewClient(dockerEndpoint)
 	}
-	dockerEndpoint := d.config.ReadDefault("docker.endpoint", defaultEndpoint)
 
-	return docker.NewClient(dockerEndpoint)
+	return docker.NewClientFromEnv()
 }
 
 func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) {
 	// Initialize docker API client
 	client, err := d.dockerClient()
 	if err != nil {
-		d.logger.Printf("[DEBUG] driver.docker: could not connect to docker daemon: %v", err)
+		d.logger.Printf("[DEBUG] driver.docker: could not connect to docker daemon: %s", err)
 		return false, nil
 	}
 
@@ -94,17 +86,13 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool
 		return false, fmt.Errorf("Unable to parse docker.cleanup.image: %s", err)
 	}
 
+	// This is the first operation taken on the client so we'll try to
+	// establish a connection to the Docker daemon. If this fails it means
+	// Docker isn't available so we'll simply disable the docker driver.
 	env, err := client.Version()
 	if err != nil {
-		d.logger.Printf("[DEBUG] driver.docker: could not read version from daemon: %v", err)
-		// Check the "no such file" error if the unix file is missing
-		if strings.Contains(err.Error(), "no such file") {
-			return false, nil
-		}
-
-		// We connected to the daemon but couldn't read the version so something
-		// is broken.
-		return false, err
+		d.logger.Printf("[INFO] driver.docker: connection to daemon failed: %s", err)
+		return false, nil
 	}
 	node.Attributes["driver.docker"] = "1"
 	node.Attributes["driver.docker.version"] = env.Get("Version")

client/driver/docker_test.go

@@ -3,12 +3,12 @@ package driver
 import (
 	"fmt"
 	"io/ioutil"
-	"os/exec"
 	"path/filepath"
 	"reflect"
 	"testing"
 	"time"
 
+	docker "github.com/fsouza/go-dockerclient"
 	"github.com/hashicorp/nomad/client/config"
 	"github.com/hashicorp/nomad/client/driver/environment"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -20,11 +20,37 @@ func testDockerDriverContext(task string) *DriverContext {
 	return NewDriverContext(task, cfg, cfg.Node, testLogger())
 }
 
-// dockerLocated looks to see whether docker is available on this system before
-// we try to run tests. We'll keep it simple and just check for the CLI.
-func dockerLocated() bool {
-	_, err := exec.Command("docker", "-v").CombinedOutput()
-	return err == nil
+// dockerIsConnected checks to see if a docker daemon is available (local or remote)
+func dockerIsConnected(t *testing.T) bool {
+	client, err := docker.NewClientFromEnv()
+	if err != nil {
+		return false
+	}
+
+	// Creating a client doesn't actually connect, so make sure we do something
+	// like call Version() on it.
+	env, err := client.Version()
+	if err != nil {
+		t.Logf("Failed to connect to docker daemon: %s", err)
+		return false
+	}
+
+	t.Logf("Successfully connected to docker daemon running version %s", env.Get("Version"))
+	return true
+}
+
+func dockerIsRemote(t *testing.T) bool {
+	client, err := docker.NewClientFromEnv()
+	if err != nil {
+		return false
+	}
+
+	// Technically this could be a local tcp socket but for testing purposes
+	// we'll just assume that tcp is only used for remote connections.
+	if client.Endpoint()[0:3] == "tcp" {
+		return true
+	}
+	return false
 }
 
 func TestDockerDriver_Handle(t *testing.T) {
@@ -42,7 +68,7 @@ func TestDockerDriver_Handle(t *testing.T) {
 	}
 }
 
-// The fingerprinter test should always pass, even if Docker is not installed.
+// This test should always pass, even if docker daemon is not available
 func TestDockerDriver_Fingerprint(t *testing.T) {
 	d := NewDockerDriver(testDockerDriverContext(""))
 	node := &structs.Node{
@@ -52,17 +78,17 @@ func TestDockerDriver_Fingerprint(t *testing.T) {
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	if apply != dockerLocated() {
-		t.Fatalf("Fingerprinter should detect Docker when it is installed")
+	if apply != dockerIsConnected(t) {
+		t.Fatalf("Fingerprinter should detect when docker is available")
 	}
 	if node.Attributes["driver.docker"] != "1" {
-		t.Log("Docker not found. The remainder of the docker tests will be skipped.")
+		t.Log("Docker daemon not available. The remainder of the docker tests will be skipped.")
 	}
 	t.Logf("Found docker version %s", node.Attributes["driver.docker.version"])
 }
 
 func TestDockerDriver_StartOpen_Wait(t *testing.T) {
-	if !dockerLocated() {
+	if !dockerIsConnected(t) {
 		t.SkipNow()
 	}
 
@@ -99,7 +125,7 @@ func TestDockerDriver_StartOpen_Wait(t *testing.T) {
 }
 
 func TestDockerDriver_Start_Wait(t *testing.T) {
-	if !dockerLocated() {
+	if !dockerIsConnected(t) {
 		t.SkipNow()
 	}
 
@@ -147,7 +173,10 @@ func TestDockerDriver_Start_Wait(t *testing.T) {
 }
 
 func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) {
-	if !dockerLocated() {
+	// This test requires that the alloc dir be mounted into docker as a volume.
+	// Because this cannot happen when docker is run remotely, e.g. when running
+	// docker in a VM, we skip this when we detect Docker is being run remotely.
+	if !dockerIsConnected(t) || dockerIsRemote(t) {
 		t.SkipNow()
 	}
 
@@ -202,7 +231,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) {
 }
 
 func TestDockerDriver_Start_Kill_Wait(t *testing.T) {
-	if !dockerLocated() {
+	if !dockerIsConnected(t) {
 		t.SkipNow()
 	}
 
@@ -269,7 +298,7 @@ func taskTemplate() *structs.Task {
 }
 
 func TestDocker_StartN(t *testing.T) {
-	if !dockerLocated() {
+	if !dockerIsConnected(t) {
 		t.SkipNow()
 	}
 
@@ -320,7 +349,7 @@ func TestDocker_StartN(t *testing.T) {
 }
 
 func TestDocker_StartNVersions(t *testing.T) {
-	if !dockerLocated() {
+	if !dockerIsConnected(t) {
 		t.SkipNow()
 	}
 
@@ -374,7 +403,7 @@ func TestDocker_StartNVersions(t *testing.T) {
 }
 
 func TestDockerHostNet(t *testing.T) {
-	if !dockerLocated() {
+	if !dockerIsConnected(t) {
 		t.SkipNow()
 	}