-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change Docker ENV behavior #407
Changes from 2 commits
acb8f1d
4b071f6
fdd21d3
9fd9e33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test fails with a remote docker host because the alloc dir cannot be mounted into the remote daemon. Since remote docker is only supported for dev/test I think the skip is OK here. |
||
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() | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe pass the testing variable in and then instead of returning true/false you can use
t.Skip
and even put the reason. Alsot.Logf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good suggestion, but skipping here would change the semantics for the
TestDockerDriver_Fingerprint
case. I will passt
for logging.