From acb8f1debde55213708c88cc3cf82987478da406 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 10 Nov 2015 15:54:31 -0800 Subject: [PATCH 1/4] Changed behavior for Docker ENV - Docker ENV variables now work the same way in production, dev, and test - Docker ENV variables are *ignored* if docker.endpoint is present in the Nomad config file - Remote tests now work correctly --- client/driver/docker.go | 26 ++++++----------- client/driver/docker_test.go | 55 ++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index a2d614a8999..eb31c4ada42 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -47,25 +47,17 @@ 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) { diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 53052bd7e70..ec35ef5b1e5 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -3,12 +3,13 @@ package driver import ( "fmt" "io/ioutil" - "os/exec" + "log" "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 +21,35 @@ 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() bool { + client, err := docker.NewClientFromEnv() + if err != nil { + return false + } + + env, err := client.Version() + if err != nil { + log.Printf("[TEST] Failed") + return false + } + + log.Printf("[TEST] Successfully connected to docker daemon running version %s", env.Get("Version")) + return true +} + +func dockerIsRemote() 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 +67,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,7 +77,7 @@ func TestDockerDriver_Fingerprint(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - if apply != dockerLocated() { + if apply != dockerIsConnected() { t.Fatalf("Fingerprinter should detect Docker when it is installed") } if node.Attributes["driver.docker"] != "1" { @@ -62,7 +87,7 @@ func TestDockerDriver_Fingerprint(t *testing.T) { } func TestDockerDriver_StartOpen_Wait(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() { t.SkipNow() } @@ -99,7 +124,7 @@ func TestDockerDriver_StartOpen_Wait(t *testing.T) { } func TestDockerDriver_Start_Wait(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() { t.SkipNow() } @@ -147,7 +172,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { } func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() || dockerIsRemote() { t.SkipNow() } @@ -202,7 +227,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { } func TestDockerDriver_Start_Kill_Wait(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() { t.SkipNow() } @@ -269,7 +294,7 @@ func taskTemplate() *structs.Task { } func TestDocker_StartN(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() { t.SkipNow() } @@ -320,7 +345,7 @@ func TestDocker_StartN(t *testing.T) { } func TestDocker_StartNVersions(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() { t.SkipNow() } @@ -374,7 +399,7 @@ func TestDocker_StartNVersions(t *testing.T) { } func TestDockerHostNet(t *testing.T) { - if !dockerLocated() { + if !dockerIsConnected() { t.SkipNow() } From 4b071f66135a012aebe953383db4258316db0d6c Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 10 Nov 2015 16:18:52 -0800 Subject: [PATCH 2/4] Change Docker fingerprinter to INFO and not error when the connection to the daemon fails; we simply assume docker isn't there. --- client/driver/docker.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index eb31c4ada42..862587dd232 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -64,7 +64,7 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool // 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 } @@ -86,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") From fdd21d30012edc62119e014b74d3312fab7c8877 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 10 Nov 2015 17:43:08 -0800 Subject: [PATCH 3/4] Change dockerIs* to accept *testing.T for logging --- client/driver/docker_test.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index ec35ef5b1e5..76c8d3ad211 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -3,7 +3,6 @@ package driver import ( "fmt" "io/ioutil" - "log" "path/filepath" "reflect" "testing" @@ -22,7 +21,7 @@ func testDockerDriverContext(task string) *DriverContext { } // dockerIsConnected checks to see if a docker daemon is available (local or remote) -func dockerIsConnected() bool { +func dockerIsConnected(t *testing.T) bool { client, err := docker.NewClientFromEnv() if err != nil { return false @@ -30,15 +29,15 @@ func dockerIsConnected() bool { env, err := client.Version() if err != nil { - log.Printf("[TEST] Failed") + t.Logf("Failed to connect to docker daemon: %s", err) return false } - log.Printf("[TEST] Successfully connected to docker daemon running version %s", env.Get("Version")) + t.Logf("Successfully connected to docker daemon running version %s", env.Get("Version")) return true } -func dockerIsRemote() bool { +func dockerIsRemote(t *testing.T) bool { client, err := docker.NewClientFromEnv() if err != nil { return false @@ -77,17 +76,17 @@ func TestDockerDriver_Fingerprint(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } - if apply != dockerIsConnected() { - 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 !dockerIsConnected() { + if !dockerIsConnected(t) { t.SkipNow() } @@ -124,7 +123,7 @@ func TestDockerDriver_StartOpen_Wait(t *testing.T) { } func TestDockerDriver_Start_Wait(t *testing.T) { - if !dockerIsConnected() { + if !dockerIsConnected(t) { t.SkipNow() } @@ -172,7 +171,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { } func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { - if !dockerIsConnected() || dockerIsRemote() { + if !dockerIsConnected(t) || dockerIsRemote(t) { t.SkipNow() } @@ -227,7 +226,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { } func TestDockerDriver_Start_Kill_Wait(t *testing.T) { - if !dockerIsConnected() { + if !dockerIsConnected(t) { t.SkipNow() } @@ -294,7 +293,7 @@ func taskTemplate() *structs.Task { } func TestDocker_StartN(t *testing.T) { - if !dockerIsConnected() { + if !dockerIsConnected(t) { t.SkipNow() } @@ -345,7 +344,7 @@ func TestDocker_StartN(t *testing.T) { } func TestDocker_StartNVersions(t *testing.T) { - if !dockerIsConnected() { + if !dockerIsConnected(t) { t.SkipNow() } @@ -399,7 +398,7 @@ func TestDocker_StartNVersions(t *testing.T) { } func TestDockerHostNet(t *testing.T) { - if !dockerIsConnected() { + if !dockerIsConnected(t) { t.SkipNow() } From 9fd9e33766def1f926ca2013a05398c7c24ab364 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 10 Nov 2015 17:48:06 -0800 Subject: [PATCH 4/4] Added some comments to the test to explain why we're doing stuff this way --- client/driver/docker_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 76c8d3ad211..872c2419b72 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -27,6 +27,8 @@ func dockerIsConnected(t *testing.T) bool { 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) @@ -171,6 +173,9 @@ func TestDockerDriver_Start_Wait(t *testing.T) { } func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { + // 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() }