From eab62662de10d0a6fe3c4b8cf0f716f0a2578f3b Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Tue, 8 Jan 2019 18:13:50 +0100 Subject: [PATCH 01/37] chore: Setup appveyor for windows test execution --- GNUmakefile | 2 +- appveyor.yml | 33 +++++++++++++++++++++++---------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index d1f64d27e..71f34a4fc 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -12,7 +12,7 @@ GO_TEST_CMD = $(if $(shell which gotestsum),gotestsum --,go test) default: help -ifeq (,$(findstring $(THIS_OS),Darwin Linux FreeBSD)) +ifeq (,$(findstring $(THIS_OS),Darwin Linux FreeBSD Windows)) $(error Building Nomad is currently only supported on Darwin and Linux.) endif diff --git a/appveyor.yml b/appveyor.yml index a6ae2926d..cb43c0415 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,15 +1,12 @@ version: "build-{branch}-{build}" image: Visual Studio 2017 + clone_folder: c:\gopath\src\github.com\hashicorp\nomad environment: GOPATH: c:\gopath GOBIN: c:\gopath\bin - - matrix: - - RUN_UI_TESTS: 1 - SKIP_NOMAD_TESTS: 1 - - {} + GOTESTSUM_JUNITFILE: c:\results.xml install: - cmd: set PATH=%GOBIN%;c:\go\bin;%PATH% @@ -17,11 +14,27 @@ install: - cmd: go version - cmd: go env - ps: mkdir C:\gopath\bin - - ps: appveyor DownloadFile "https://releases.hashicorp.com/vault/0.7.0/vault_0.7.0_windows_amd64.zip" -FileName "C:\\gopath\\bin\\vault.zip" + - ps: appveyor DownloadFile "https://releases.hashicorp.com/vault/0.10.2/vault_0.10.2_windows_amd64.zip" -FileName "C:\\gopath\\bin\\vault.zip" - ps: Expand-Archive C:\gopath\bin\vault.zip -DestinationPath C:\gopath\bin - - ps: appveyor DownloadFile "https://releases.hashicorp.com/consul/0.7.0/consul_0.7.0_windows_amd64.zip" -FileName "C:\\gopath\\bin\\consul.zip" + - ps: appveyor DownloadFile "https://releases.hashicorp.com/consul/1.0.0/consul_1.0.0_windows_amd64.zip" -FileName "C:\\gopath\\bin\\consul.zip" - ps: Expand-Archive C:\gopath\bin\consul.zip -DestinationPath C:\gopath\bin - #- cmd: go install + - ps: choco install make + - ps: | + go get -u github.com/kardianos/govendor + go get -u github.com/ugorji/go/codec/codecgen + go get -u github.com/hashicorp/go-bindata/go-bindata + go get -u github.com/elazarl/go-bindata-assetfs/go-bindata-assetfs + go get -u github.com/a8m/tree/cmd/tree + go get -u github.com/magiconair/vendorfmt/cmd/vendorfmt + go get -u github.com/golang/protobuf/protoc-gen-go + go get -u gotest.tools/gotestsum build_script: - #- cmd: go test ./... - - cmd: go install + - cmd: | + mkdir -p $GOPATH\bin + go build -o $GOPATH\bin\nomad +test_script: + - cmd: gotestsum -f short-verbose +on_finish: + - ps: | + $wc = New-Object 'System.Net.WebClient' + $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path "c:\\results.xml")) From a51aeb224f21d0aa992da36d9181a15c48b9fc8c Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 9 Jan 2019 00:34:02 +0100 Subject: [PATCH 02/37] testtask: Build on windows --- helper/testtask/testtask.go | 14 +------------- helper/testtask/testtask_unix.go | 23 +++++++++++++++++++++++ helper/testtask/testtask_windows.go | 13 +++++++++++++ 3 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 helper/testtask/testtask_unix.go create mode 100644 helper/testtask/testtask_windows.go diff --git a/helper/testtask/testtask.go b/helper/testtask/testtask.go index 0152e3be2..fa8bc0cd0 100644 --- a/helper/testtask/testtask.go +++ b/helper/testtask/testtask.go @@ -7,8 +7,6 @@ import ( "io/ioutil" "os" "os/exec" - "strconv" - "syscall" "time" "github.com/hashicorp/nomad/nomad/structs" @@ -115,21 +113,11 @@ func execute() { ioutil.WriteFile(file, []byte(msg), 0666) case "pgrp": - // pgrp puts the pid in a new process group if len(args) < 1 { fmt.Fprintln(os.Stderr, "expected process group number for pgrp") os.Exit(1) } - num := popArg() - grp, err := strconv.Atoi(num) - if err != nil { - fmt.Fprintf(os.Stderr, "failed to convert process group number %q: %v\n", num, err) - os.Exit(1) - } - if err := syscall.Setpgid(0, grp); err != nil { - fmt.Fprintf(os.Stderr, "failed to set process group: %v\n", err) - os.Exit(1) - } + executeProcessGroup(popArg()) case "fork/exec": // fork/exec forks execs the helper process diff --git a/helper/testtask/testtask_unix.go b/helper/testtask/testtask_unix.go new file mode 100644 index 000000000..6b17a7a43 --- /dev/null +++ b/helper/testtask/testtask_unix.go @@ -0,0 +1,23 @@ +// +build unix + +package testtask + +import ( + "fmt" + "os" + "strconv" + "syscall" +) + +func executeProcessGroup(gid string) { + // pgrp puts the pid in a new process group + grp, err := strconv.Atoi(gid) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to convert process group number %q: %v\n", gid, err) + os.Exit(1) + } + if err := syscall.Setpgid(0, grp); err != nil { + fmt.Fprintf(os.Stderr, "failed to set process group: %v\n", err) + os.Exit(1) + } +} diff --git a/helper/testtask/testtask_windows.go b/helper/testtask/testtask_windows.go new file mode 100644 index 000000000..64717665f --- /dev/null +++ b/helper/testtask/testtask_windows.go @@ -0,0 +1,13 @@ +// +build windows + +package testtask + +import ( + "fmt" + "os" +) + +func executeProcessGroup(gid string) { + fmt.Fprintf(os.Stderr, "process groups are not supported on windows\n") + os.Exit(1) +} From b05d362eb15ca430d867142c66f8db565aed09c6 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 9 Jan 2019 00:45:27 +0100 Subject: [PATCH 03/37] fingerprinter: Use HCLogger for windows --- client/fingerprint/network_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/fingerprint/network_windows_test.go b/client/fingerprint/network_windows_test.go index 6eedf5010..96482bc74 100644 --- a/client/fingerprint/network_windows_test.go +++ b/client/fingerprint/network_windows_test.go @@ -7,7 +7,7 @@ import ( ) func TestNetworkFingerPrint_linkspeed_parse(t *testing.T) { - f := &NetworkFingerprint{logger: testlog.Logger(t), interfaceDetector: &DefaultNetworkInterfaceDetector{}} + f := &NetworkFingerprint{logger: testlog.HCLogger(t), interfaceDetector: &DefaultNetworkInterfaceDetector{}} var outputTests = []struct { in string From 3078c24f79d255b0ee7d52d44bfcb529498da703 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 9 Jan 2019 20:30:47 +0000 Subject: [PATCH 04/37] vaultclient: Update tests for vault 1.0 --- client/vaultclient/vaultclient_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/vaultclient/vaultclient_test.go b/client/vaultclient/vaultclient_test.go index d9d334d5d..c46328965 100644 --- a/client/vaultclient/vaultclient_test.go +++ b/client/vaultclient/vaultclient_test.go @@ -275,7 +275,8 @@ func TestVaultClient_RenewNonexistentLease(t *testing.T) { _, err = c.RenewToken(c.client.Token(), 10) if err == nil { t.Fatalf("expected error, got nil") - } else if !strings.Contains(err.Error(), "lease not found") { - t.Fatalf("expected \"%s\" in error message, got \"%v\"", "lease not found", err) + // The Vault error message changed between 0.10.2 and 1.0.1 + } else if !strings.Contains(err.Error(), "lease not found") && !strings.Contains(err.Error(), "lease is not renewable") { + t.Fatalf("expected \"%s\" or \"%s\" in error message, got \"%v\"", "lease not found", "lease is not renewable", err.Error()) } } From d33809b88634a6858c935ea9786d205514839dab Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 9 Jan 2019 13:36:58 +0100 Subject: [PATCH 05/37] fingerprint: Limit vault shutdown waiting When vault is installed through chocolatey, it also installs a shim that will not pass kill signals to the child. This means the process will never actually terminate, and we lose the process handle. Here, rather than waiting forever, we timeout fast. --- testutil/vault.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/testutil/vault.go b/testutil/vault.go index aa7f69bd5..b2b353c5f 100644 --- a/testutil/vault.go +++ b/testutil/vault.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/structs/config" vapi "github.com/hashicorp/vault/api" - "github.com/mitchellh/go-testing-interface" + testing "github.com/mitchellh/go-testing-interface" ) // TestVault is a test helper. It uses a fork/exec model to create a test Vault @@ -198,7 +198,12 @@ func (tv *TestVault) Stop() { tv.t.Errorf("err: %s", err) } if tv.waitCh != nil { - <-tv.waitCh + select { + case <-tv.waitCh: + return + case <-time.After(1 * time.Second): + tv.t.Error("Timed out waiting for vault to terminate") + } } } From dcce2d72478a1ea987c64dac723c24a8ea3524a7 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 9 Jan 2019 20:26:14 +0000 Subject: [PATCH 06/37] vaultclient: use require for error assertions --- client/vaultclient/vaultclient_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/vaultclient/vaultclient_test.go b/client/vaultclient/vaultclient_test.go index c46328965..73cf662d2 100644 --- a/client/vaultclient/vaultclient_test.go +++ b/client/vaultclient/vaultclient_test.go @@ -9,10 +9,12 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/testutil" vaultapi "github.com/hashicorp/vault/api" + "github.com/stretchr/testify/require" ) func TestVaultClient_TokenRenewals(t *testing.T) { t.Parallel() + require := require.New(t) v := testutil.NewTestVault(t) defer v.Stop() @@ -67,9 +69,7 @@ func TestVaultClient_TokenRenewals(t *testing.T) { for { select { case err := <-errCh: - if err != nil { - t.Fatalf("error while renewing the token: %v", err) - } + require.NoError(err, "unexpected error while renewing vault token") } } }(errCh) @@ -83,7 +83,7 @@ func TestVaultClient_TokenRenewals(t *testing.T) { for i := 0; i < num; i++ { if err := c.StopRenewToken(tokens[i]); err != nil { - t.Fatal(err) + require.NoError(err) } } From d81e632157c36cb2df001f60597b670b0b7ea53f Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 9 Jan 2019 20:26:54 +0000 Subject: [PATCH 07/37] testutil: Start vault in the same routine as waiting This is a workaround for the windows process model. Go os/exec does not pass the parent process handle to the child processes STARTUPINFO struct, this means that unless we wait in the _same_ execution context as Starting the process, the handle will be lost, and we cannot kill it without regaining a handle. A better long term solution would be a higher level process abstraction that uses windows.CreateProcess on windows. --- testutil/vault.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/testutil/vault.go b/testutil/vault.go index b2b353c5f..6719b6610 100644 --- a/testutil/vault.go +++ b/testutil/vault.go @@ -167,13 +167,15 @@ func NewTestVaultDelayed(t testing.T) *TestVault { // Start starts the test Vault server and waits for it to respond to its HTTP // API func (tv *TestVault) Start() error { - if err := tv.cmd.Start(); err != nil { - tv.t.Fatalf("failed to start vault: %v", err) - } - // Start the waiter tv.waitCh = make(chan error, 1) + go func() { + if err := tv.cmd.Start(); err != nil { + tv.waitCh <- err + return + } + err := tv.cmd.Wait() tv.waitCh <- err }() From e9a7978367fa6004e663a224ce0574d0814cebd2 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 09:00:41 +0000 Subject: [PATCH 08/37] DROP: disable consul tests on windows --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index cb43c0415..908bfb761 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -16,8 +16,8 @@ install: - ps: mkdir C:\gopath\bin - ps: appveyor DownloadFile "https://releases.hashicorp.com/vault/0.10.2/vault_0.10.2_windows_amd64.zip" -FileName "C:\\gopath\\bin\\vault.zip" - ps: Expand-Archive C:\gopath\bin\vault.zip -DestinationPath C:\gopath\bin - - ps: appveyor DownloadFile "https://releases.hashicorp.com/consul/1.0.0/consul_1.0.0_windows_amd64.zip" -FileName "C:\\gopath\\bin\\consul.zip" - - ps: Expand-Archive C:\gopath\bin\consul.zip -DestinationPath C:\gopath\bin + # - ps: appveyor DownloadFile "https://releases.hashicorp.com/consul/1.0.0/consul_1.0.0_windows_amd64.zip" -FileName "C:\\gopath\\bin\\consul.zip" + # - ps: Expand-Archive C:\gopath\bin\consul.zip -DestinationPath C:\gopath\bin - ps: choco install make - ps: | go get -u github.com/kardianos/govendor From 85474288076cdd247fb00c5732ec029913f9b6b1 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 09:59:04 +0000 Subject: [PATCH 09/37] plugins: Load plugins on windows --- plugins/shared/loader/init.go | 14 ++++++++++---- plugins/shared/loader/loader_test.go | 10 +++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 7af5c8e53..30d2d94fc 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "sort" + "runtime" multierror "github.com/hashicorp/go-multierror" plugin "github.com/hashicorp/go-plugin" @@ -250,10 +251,15 @@ func (l *PluginLoader) scan() ([]os.FileInfo, error) { continue } - // Check if it is executable by anyone - if s.Mode().Perm()&0111 == 0 { - l.logger.Debug("skipping un-executable file in plugin folder", "file", f) - continue + // Check if it is executable by anyone. On windows, an executable is any file with any + // extension and the file begins with MZ, however, there is no easy way for us to + // actually validate the executability of a file, so here we skip executability checks + // for windows systems. + if runtime.GOOS != "windows" { + if s.Mode().Perm()&0111 == 0 { + l.logger.Debug("skipping un-executable file in plugin folder", "file", f) + continue + } } plugins = append(plugins, s) } diff --git a/plugins/shared/loader/loader_test.go b/plugins/shared/loader/loader_test.go index f8b31aea8..51e3cf71c 100644 --- a/plugins/shared/loader/loader_test.go +++ b/plugins/shared/loader/loader_test.go @@ -8,6 +8,7 @@ import ( "sort" "strings" "testing" + "runtime" log "github.com/hashicorp/go-hclog" version "github.com/hashicorp/go-version" @@ -55,8 +56,12 @@ func newHarness(t *testing.T, plugins []string) *harness { t.Fatalf("failed to get self executable path: %v", err) } + exeSuffix := "" + if runtime.GOOS == "windows" { + exeSuffix = ".exe" + } for _, p := range plugins { - dest := filepath.Join(h.tmpDir, p) + dest := strings.Join([]string{filepath.Join(h.tmpDir, p), exeSuffix}, "") if err := copyFile(selfExe, dest); err != nil { t.Fatalf("failed to copy file: %v", err) } @@ -1217,6 +1222,9 @@ func TestPluginLoader_Bad_Executable(t *testing.T) { // Test that we skip directories, non-executables and follow symlinks func TestPluginLoader_External_SkipBadFiles(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows currently does not skip non exe files") + } t.Parallel() require := require.New(t) From c9edd612ef32fa5ac6bff99271bcff0908c992a9 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 11:45:56 +0100 Subject: [PATCH 10/37] appveyor: Fix env access? --- appveyor.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 908bfb761..a2fd7b516 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -6,7 +6,6 @@ clone_folder: c:\gopath\src\github.com\hashicorp\nomad environment: GOPATH: c:\gopath GOBIN: c:\gopath\bin - GOTESTSUM_JUNITFILE: c:\results.xml install: - cmd: set PATH=%GOBIN%;c:\go\bin;%PATH% @@ -33,8 +32,8 @@ build_script: mkdir -p $GOPATH\bin go build -o $GOPATH\bin\nomad test_script: - - cmd: gotestsum -f short-verbose + - cmd: gotestsum -f short-verbose --junitfile results.xml on_finish: - ps: | $wc = New-Object 'System.Net.WebClient' - $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path "c:\\results.xml")) + $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\results.xml)) From 4ce7cc1ee483299c3b07c943ab8846e30c4cb2c5 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 13:14:07 +0100 Subject: [PATCH 11/37] appveyor: Push test results as an artifact --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index a2fd7b516..c8753f045 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -35,5 +35,6 @@ test_script: - cmd: gotestsum -f short-verbose --junitfile results.xml on_finish: - ps: | + Push-AppveyorArtifact (Resolve-Path .\results.xml) $wc = New-Object 'System.Net.WebClient' $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\results.xml)) From 4014ee30ae8ca5bb9550c1225d60765e644cc861 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 13:21:01 +0100 Subject: [PATCH 12/37] appveyor: Add gopath/bin to PATH --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index c8753f045..3729b1c68 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -29,6 +29,7 @@ install: go get -u gotest.tools/gotestsum build_script: - cmd: | + set PATH=%GOPATH%/bin;%PATH% mkdir -p $GOPATH\bin go build -o $GOPATH\bin\nomad test_script: From cf08f85413488242e3c98e9cec4e538268af43bd Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 13:43:51 +0100 Subject: [PATCH 13/37] api: Validate the slash variant of a given path This validates the slash variant of a RelativeDest, rather than the platform native version, to support test execution on windows. --- api/tasks_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/tasks_test.go b/api/tasks_test.go index da66aac18..14a151717 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -1,6 +1,7 @@ package api import ( + "path/filepath" "reflect" "testing" "time" @@ -364,7 +365,7 @@ func TestTask_Artifact(t *testing.T) { if *a.GetterMode != "file" { t.Errorf("expected file but found %q", *a.GetterMode) } - if *a.RelativeDest != "local/foo.txt" { + if filepath.ToSlash(*a.RelativeDest) != "local/foo.txt" { t.Errorf("expected local/foo.txt but found %q", *a.RelativeDest) } } From 20f40eada8020fd2ae2dbbfdf230d4579fce753e Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 13:57:29 +0100 Subject: [PATCH 14/37] client/fs: windows error message for not found --- client/fs_endpoint_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/fs_endpoint_test.go b/client/fs_endpoint_test.go index b2ca33f23..f0d6f53c0 100644 --- a/client/fs_endpoint_test.go +++ b/client/fs_endpoint_test.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "strings" "testing" "time" @@ -1523,7 +1524,11 @@ func TestFS_streamFile_NoFile(t *testing.T) { err := c.endpoints.FileSystem.streamFile( context.Background(), 0, "foo", 0, ad, framer, nil) require.NotNil(err) - require.Contains(err.Error(), "no such file") + if runtime.GOOS == "windows" { + require.Contains(err.Error(), "cannot find the file") + } else { + require.Contains(err.Error(), "no such file") + } } func TestFS_streamFile_Modify(t *testing.T) { From 041c01d58362784bb81459d0f975077c03cd35f8 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 14:11:47 +0100 Subject: [PATCH 15/37] client/fs: Skip delete-while-streaming test on win --- client/fs_endpoint_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/fs_endpoint_test.go b/client/fs_endpoint_test.go index f0d6f53c0..43aa790dc 100644 --- a/client/fs_endpoint_test.go +++ b/client/fs_endpoint_test.go @@ -1706,6 +1706,9 @@ func TestFS_streamFile_Truncate(t *testing.T) { } func TestFS_streamImpl_Delete(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not allow us to delete a file while it is open") + } t.Parallel() c, cleanup := TestClient(t, nil) @@ -1730,7 +1733,11 @@ func TestFS_streamImpl_Delete(t *testing.T) { frames := make(chan *sframer.StreamFrame, 4) go func() { for { - frame := <-frames + frame, ok := <-frames + if !ok { + return + } + if frame.IsHeartbeat() { continue } From 08267885abc966f4d4b1fcea7ccc19897e405054 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 14:49:02 +0100 Subject: [PATCH 16/37] appveyor: GOMAXPROCS: 1 --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index 3729b1c68..737aab913 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -6,6 +6,7 @@ clone_folder: c:\gopath\src\github.com\hashicorp\nomad environment: GOPATH: c:\gopath GOBIN: c:\gopath\bin + GOMAXPROCS: 1 install: - cmd: set PATH=%GOBIN%;c:\go\bin;%PATH% From 347b4ad24754e96be05f80b14b80eac815d2a216 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 16:04:34 +0100 Subject: [PATCH 17/37] Expand unix build definition --- drivers/docker/driver_unix_test.go | 98 ------------------------------ helper/testtask/testtask_unix.go | 2 +- 2 files changed, 1 insertion(+), 99 deletions(-) delete mode 100644 drivers/docker/driver_unix_test.go diff --git a/drivers/docker/driver_unix_test.go b/drivers/docker/driver_unix_test.go deleted file mode 100644 index 2ba6cec26..000000000 --- a/drivers/docker/driver_unix_test.go +++ /dev/null @@ -1,98 +0,0 @@ -// +build !windows - -package docker - -import ( - "context" - "fmt" - "io/ioutil" - "os" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/hashicorp/nomad/client/testutil" - tu "github.com/hashicorp/nomad/testutil" - "github.com/stretchr/testify/require" -) - -func TestDockerDriver_Signal(t *testing.T) { - if !tu.IsTravis() { - t.Parallel() - } - if !testutil.DockerIsConnected(t) { - t.Skip("Docker not connected") - } - - task, cfg, _ := dockerTask(t) - cfg.Command = "/bin/sh" - cfg.Args = []string{"local/test.sh"} - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - driver := dockerDriverHarness(t, nil) - cleanup := driver.MkAllocDir(task, true) - defer cleanup() - - // Copy the image into the task's directory - copyImage(t, task.TaskDir(), "busybox.tar") - - testFile := filepath.Join(task.TaskDir().LocalDir, "test.sh") - testData := []byte(` -at_term() { - echo 'Terminated.' > $NOMAD_TASK_DIR/output - exit 3 -} -trap at_term INT -while true; do - echo 'sleeping' - sleep 0.2 -done - `) - require.NoError(t, ioutil.WriteFile(testFile, testData, 0777)) - _, _, err := driver.StartTask(task) - require.NoError(t, err) - defer driver.DestroyTask(task.ID, true) - require.NoError(t, driver.WaitUntilStarted(task.ID, time.Duration(tu.TestMultiplier()*5)*time.Second)) - handle, ok := driver.Impl().(*Driver).tasks.Get(task.ID) - require.True(t, ok) - - waitForExist(t, newTestDockerClient(t), handle.containerID) - require.NoError(t, handle.Kill(time.Duration(tu.TestMultiplier()*5)*time.Second, os.Interrupt)) - - waitCh, err := driver.WaitTask(context.Background(), task.ID) - require.NoError(t, err) - select { - case res := <-waitCh: - if res.Successful() { - require.Fail(t, "should err: %v", res) - } - case <-time.After(time.Duration(tu.TestMultiplier()*5) * time.Second): - require.Fail(t, "timeout") - } - - // Check the log file to see it exited because of the signal - outputFile := filepath.Join(task.TaskDir().LocalDir, "output") - act, err := ioutil.ReadFile(outputFile) - if err != nil { - t.Fatalf("Couldn't read expected output: %v", err) - } - - exp := "Terminated." - if strings.TrimSpace(string(act)) != exp { - t.Fatalf("Command outputted %v; want %v", act, exp) - } -} - -func TestDockerDriver_containerBinds(t *testing.T) { - task, cfg, _ := dockerTask(t) - driver := dockerDriverHarness(t, nil) - cleanup := driver.MkAllocDir(task, false) - defer cleanup() - - binds, err := driver.Impl().(*Driver).containerBinds(task, cfg) - require.NoError(t, err) - require.Contains(t, binds, fmt.Sprintf("%s:/alloc", task.TaskDir().SharedAllocDir)) - require.Contains(t, binds, fmt.Sprintf("%s:/local", task.TaskDir().LocalDir)) - require.Contains(t, binds, fmt.Sprintf("%s:/secrets", task.TaskDir().SecretsDir)) -} diff --git a/helper/testtask/testtask_unix.go b/helper/testtask/testtask_unix.go index 6b17a7a43..c3d3ceb68 100644 --- a/helper/testtask/testtask_unix.go +++ b/helper/testtask/testtask_unix.go @@ -1,4 +1,4 @@ -// +build unix +// +build darwin dragonfly freebsd linux netbsd openbsd solaris package testtask From a587be44a77b3efbfda1d76227bacf9bed42157c Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 16:04:23 +0100 Subject: [PATCH 18/37] dockerlogger: Fix tests on windows Uses the home directory and windows path expansion, as c:\tmp doesn't necessarily exist, and mktemp would involve unnecessarily complicating the commands. --- drivers/docker/docklog/docker_logger_test.go | 14 +++++++------- drivers/docker/docklog/testing_unix.go | 9 +++++++++ drivers/docker/docklog/testing_windows.go | 9 +++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 drivers/docker/docklog/testing_unix.go create mode 100644 drivers/docker/docklog/testing_windows.go diff --git a/drivers/docker/docklog/docker_logger_test.go b/drivers/docker/docklog/docker_logger_test.go index a3159feaf..16373c5f9 100644 --- a/drivers/docker/docklog/docker_logger_test.go +++ b/drivers/docker/docklog/docker_logger_test.go @@ -24,11 +24,11 @@ func TestDockerLogger(t *testing.T) { t.Skip("docker unavailable:", err) } - if img, err := client.InspectImage("busybox:1"); err != nil || img == nil { + if img, err := client.InspectImage(containerImage); err != nil || img == nil { t.Log("image not found locally, downloading...") err = client.PullImage(docker.PullImageOptions{ - Repository: "busybox", - Tag: "1", + Repository: containerImageName, + Tag: containerImageTag, }, docker.AuthConfiguration{}) if err != nil { t.Fatalf("failed to pull image: %v", err) @@ -38,9 +38,9 @@ func TestDockerLogger(t *testing.T) { containerConf := docker.CreateContainerOptions{ Config: &docker.Config{ Cmd: []string{ - "/bin/sh", "-c", "touch /tmp/docklog; tail -f /tmp/docklog", + "sh", "-c", "touch ~/docklog; tail -f ~/docklog", }, - Image: "busybox:1", + Image: containerImage, }, Context: context.Background(), } @@ -98,8 +98,8 @@ func echoToContainer(t *testing.T, client *docker.Client, id string, line string op := docker.CreateExecOptions{ Container: id, Cmd: []string{ - "/bin/ash", "-c", - fmt.Sprintf("echo %s >>/tmp/docklog", line), + "ash", "-c", + fmt.Sprintf("echo %s >>~/docklog", line), }, } diff --git a/drivers/docker/docklog/testing_unix.go b/drivers/docker/docklog/testing_unix.go new file mode 100644 index 000000000..42c319d83 --- /dev/null +++ b/drivers/docker/docklog/testing_unix.go @@ -0,0 +1,9 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package docklog + +var ( + containerImage = "busybox:1" + containerImageName = "busybox" + containerImageTag = "1" +) diff --git a/drivers/docker/docklog/testing_windows.go b/drivers/docker/docklog/testing_windows.go new file mode 100644 index 000000000..4ec6b73c7 --- /dev/null +++ b/drivers/docker/docklog/testing_windows.go @@ -0,0 +1,9 @@ +// +build windows + +package docklog + +var ( + containerImage = "dantoml/busybox-windows:08012019" + containerImageName = "dantoml/busybox-windows" + containerImageTag = "08012019" +) From 8a4ffea94a9a6904aad64f1b186d04090e28bce7 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 16:24:29 +0100 Subject: [PATCH 19/37] chore: Cleanup formatting --- client/vaultclient/vaultclient_test.go | 2 +- plugins/shared/loader/init.go | 2 +- plugins/shared/loader/loader_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/vaultclient/vaultclient_test.go b/client/vaultclient/vaultclient_test.go index 73cf662d2..ab5a7474e 100644 --- a/client/vaultclient/vaultclient_test.go +++ b/client/vaultclient/vaultclient_test.go @@ -275,7 +275,7 @@ func TestVaultClient_RenewNonexistentLease(t *testing.T) { _, err = c.RenewToken(c.client.Token(), 10) if err == nil { t.Fatalf("expected error, got nil") - // The Vault error message changed between 0.10.2 and 1.0.1 + // The Vault error message changed between 0.10.2 and 1.0.1 } else if !strings.Contains(err.Error(), "lease not found") && !strings.Contains(err.Error(), "lease is not renewable") { t.Fatalf("expected \"%s\" or \"%s\" in error message, got \"%v\"", "lease not found", "lease is not renewable", err.Error()) } diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 30d2d94fc..de94dfc47 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -5,8 +5,8 @@ import ( "os" "os/exec" "path/filepath" - "sort" "runtime" + "sort" multierror "github.com/hashicorp/go-multierror" plugin "github.com/hashicorp/go-plugin" diff --git a/plugins/shared/loader/loader_test.go b/plugins/shared/loader/loader_test.go index 51e3cf71c..2fd9004d1 100644 --- a/plugins/shared/loader/loader_test.go +++ b/plugins/shared/loader/loader_test.go @@ -5,10 +5,10 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "sort" "strings" "testing" - "runtime" log "github.com/hashicorp/go-hclog" version "github.com/hashicorp/go-version" From 76ee0240abc47e299efc991e2ba33ec4d8066ab9 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 10 Jan 2019 16:52:22 +0100 Subject: [PATCH 20/37] docker: ExpandPath tests validate slashpath --- drivers/docker/utils_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/docker/utils_test.go b/drivers/docker/utils_test.go index edc10dd0f..8f196f321 100644 --- a/drivers/docker/utils_test.go +++ b/drivers/docker/utils_test.go @@ -1,6 +1,7 @@ package docker import ( + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -55,7 +56,7 @@ func TestExpandPath(t *testing.T) { for _, c := range cases { t.Run(c.expected, func(t *testing.T) { - require.Equal(t, c.expected, expandPath(c.base, c.target)) + require.Equal(t, c.expected, filepath.ToSlash(expandPath(c.base, c.target))) }) } } From 1e0825388ecc04de519430653df10bf8a388a307 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Fri, 11 Jan 2019 14:28:40 +0100 Subject: [PATCH 21/37] docker: Test cleanup for windows * Docker for Windows does not support ulimits * Use filepath.ToSlash to test workdir * Convert expected mount paths to system style * Skip security-opt test on windows - Windows does not support seccomp, and it's unclear which options are available. * Skip StartN due to lack of sigint * docker: Use api to get image info on windows * No bridge on windows * Stop hardcoding /bin/ --- drivers/docker/driver_test.go | 875 +++------------------------ drivers/docker/driver_unix_test.go | 643 ++++++++++++++++++++ drivers/docker/testing_unix.go | 61 ++ drivers/docker/testing_windows.go | 25 + drivers/docker/utils_test.go | 55 -- drivers/docker/utils_unix_test.go | 63 ++ drivers/docker/utils_windows_test.go | 34 ++ 7 files changed, 925 insertions(+), 831 deletions(-) create mode 100644 drivers/docker/driver_unix_test.go create mode 100644 drivers/docker/testing_unix.go create mode 100644 drivers/docker/testing_windows.go create mode 100644 drivers/docker/utils_unix_test.go create mode 100644 drivers/docker/utils_windows_test.go diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index e2d681a82..e695f9f3a 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -3,16 +3,12 @@ package docker import ( "context" "fmt" - "io" "io/ioutil" "math/rand" - "os" "path/filepath" "reflect" "runtime" "runtime/debug" - "sort" - "strconv" "strings" "testing" "time" @@ -20,7 +16,6 @@ import ( docker "github.com/fsouza/go-dockerclient" "github.com/hashicorp/consul/lib/freeport" hclog "github.com/hashicorp/go-hclog" - "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/devices/gpu/nvidia" @@ -32,7 +27,6 @@ import ( dtestutil "github.com/hashicorp/nomad/plugins/drivers/testutils" "github.com/hashicorp/nomad/plugins/shared/loader" tu "github.com/hashicorp/nomad/testutil" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -68,18 +62,9 @@ func dockerIsRemote(t *testing.T) bool { } var ( - // busyboxImageID is the ID stored in busybox.tar - busyboxImageID = "busybox:1.29.3" - - // busyboxImageID is the ID stored in busybox_glibc.tar - busyboxGlibcImageID = "busybox:1.29.3-glibc" - - // busyboxImageID is the ID stored in busybox_musl.tar - busyboxMuslImageID = "busybox:1.29.3-musl" - // busyboxLongRunningCmd is a busybox command that runs indefinitely, and // ideally responds to SIGINT/SIGTERM. Sadly, busybox:1.29.3 /bin/sleep doesn't. - busyboxLongRunningCmd = []string{"/bin/nc", "-l", "-p", "3000", "127.0.0.1"} + busyboxLongRunningCmd = []string{"nc", "-l", "-p", "3000", "127.0.0.1"} ) // Returns a task with a reserved and dynamic port. The ports are returned @@ -89,12 +74,7 @@ func dockerTask(t *testing.T) (*drivers.TaskConfig, *TaskConfig, []int) { dockerReserved := ports[0] dockerDynamic := ports[1] - cfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: busyboxLongRunningCmd[0], - Args: busyboxLongRunningCmd[1:], - } + cfg := newTaskConfig("", busyboxLongRunningCmd) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "redis-demo", @@ -341,12 +321,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { } testutil.DockerCompatible(t) - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: busyboxLongRunningCmd[0], - Args: busyboxLongRunningCmd[1:], - } + taskCfg := newTaskConfig("", busyboxLongRunningCmd) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "nc-demo", @@ -381,12 +356,7 @@ func TestDockerDriver_Start_WaitFinish(t *testing.T) { } testutil.DockerCompatible(t) - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "/bin/echo", - Args: []string{"hello"}, - } + taskCfg := newTaskConfig("", []string{"echo", "hello"}) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "nc-demo", @@ -428,12 +398,7 @@ func TestDockerDriver_Start_StoppedContainer(t *testing.T) { } testutil.DockerCompatible(t) - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "sleep", - Args: []string{"9001"}, - } + taskCfg := newTaskConfig("", []string{"sleep", "9001"}) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "nc-demo", @@ -447,7 +412,19 @@ func TestDockerDriver_Start_StoppedContainer(t *testing.T) { copyImage(t, task.TaskDir(), "busybox.tar") client := newTestDockerClient(t) - imageID, err := d.Impl().(*Driver).loadImage(task, &taskCfg, client) + + var imageID string + var err error + + if runtime.GOOS != "windows" { + imageID, err = d.Impl().(*Driver).loadImage(task, &taskCfg, client) + } else { + image, lErr := client.InspectImage("dantoml/busybox-windows:08012019") + err = lErr + if image != nil { + imageID = image.ID + } + } require.NoError(t, err) require.NotEmpty(t, imageID) @@ -457,7 +434,7 @@ func TestDockerDriver_Start_StoppedContainer(t *testing.T) { opts := docker.CreateContainerOptions{ Name: strings.Replace(task.ID, "/", "_", -1), Config: &docker.Config{ - Image: busyboxImageID, + Image: taskCfg.Image, Cmd: []string{"sleep", "9000"}, }, } @@ -480,15 +457,7 @@ func TestDockerDriver_Start_LoadImage(t *testing.T) { } testutil.DockerCompatible(t) - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "/bin/sh", - Args: []string{ - "-c", - "echo hello > $NOMAD_TASK_DIR/output", - }, - } + taskCfg := newTaskConfig("", []string{"sh", "-c", "echo hello > $NOMAD_TASK_DIR/output"}) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "busybox-demo", @@ -539,7 +508,7 @@ func TestDockerDriver_Start_BadPull_Recoverable(t *testing.T) { taskCfg := TaskConfig{ Image: "127.0.0.1:32121/foo", // bad path - Command: "/bin/echo", + Command: "echo", Args: []string{ "hello", }, @@ -580,16 +549,13 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { exp := []byte{'w', 'i', 'n'} file := "output.txt" - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "/bin/sh", - Args: []string{ - "-c", - fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, - string(exp), taskenv.AllocDir, file), - }, - } + + taskCfg := newTaskConfig("", []string{ + "sh", + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, + string(exp), taskenv.AllocDir, file), + }) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "busybox-demo", @@ -638,12 +604,7 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { } testutil.DockerCompatible(t) - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: busyboxLongRunningCmd[0], - Args: busyboxLongRunningCmd[1:], - } + taskCfg := newTaskConfig("", busyboxLongRunningCmd) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "busybox-demo", @@ -663,7 +624,11 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { go func(t *testing.T) { time.Sleep(100 * time.Millisecond) - require.NoError(t, d.StopTask(task.ID, time.Second, "SIGINT")) + signal := "SIGINT" + if runtime.GOOS == "windows" { + signal = "SIGKILL" + } + require.NoError(t, d.StopTask(task.ID, time.Second, signal)) }(t) // Attempt to wait @@ -686,14 +651,7 @@ func TestDockerDriver_Start_KillTimeout(t *testing.T) { } testutil.DockerCompatible(t) timeout := 2 * time.Second - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "/bin/sleep", - Args: []string{ - "10", - }, - } + taskCfg := newTaskConfig("", []string{"sleep", "10"}) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "busybox-demo", @@ -734,6 +692,9 @@ func TestDockerDriver_Start_KillTimeout(t *testing.T) { } func TestDockerDriver_StartN(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows Docker does not support SIGINT") + } if !tu.IsTravis() { t.Parallel() } @@ -778,6 +739,9 @@ func TestDockerDriver_StartN(t *testing.T) { } func TestDockerDriver_StartNVersions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipped on windows, we don't have image variants available") + } if !tu.IsTravis() { t.Parallel() } @@ -785,18 +749,21 @@ func TestDockerDriver_StartNVersions(t *testing.T) { require := require.New(t) task1, cfg1, _ := dockerTask(t) - cfg1.Image = busyboxImageID - cfg1.LoadImage = "busybox.tar" + tcfg1 := newTaskConfig("", []string{"echo", "hello"}) + cfg1.Image = tcfg1.Image + cfg1.LoadImage = tcfg1.LoadImage require.NoError(task1.EncodeConcreteDriverConfig(cfg1)) task2, cfg2, _ := dockerTask(t) - cfg2.Image = busyboxMuslImageID - cfg2.LoadImage = "busybox_musl.tar" + tcfg2 := newTaskConfig("musl", []string{"echo", "hello"}) + cfg2.Image = tcfg2.Image + cfg2.LoadImage = tcfg2.LoadImage require.NoError(task2.EncodeConcreteDriverConfig(cfg2)) task3, cfg3, _ := dockerTask(t) - cfg3.Image = busyboxGlibcImageID - cfg3.LoadImage = "busybox_glibc.tar" + tcfg3 := newTaskConfig("glibc", []string{"echo", "hello"}) + cfg3.Image = tcfg3.Image + cfg3.LoadImage = tcfg3.LoadImage require.NoError(task3.EncodeConcreteDriverConfig(cfg3)) taskList := []*drivers.TaskConfig{task1, task2, task3} @@ -836,195 +803,6 @@ func TestDockerDriver_StartNVersions(t *testing.T) { t.Log("Test complete!") } -func TestDockerDriver_NetworkMode_Host(t *testing.T) { - if !tu.IsTravis() { - t.Parallel() - } - testutil.DockerCompatible(t) - expected := "host" - - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: busyboxLongRunningCmd[0], - Args: busyboxLongRunningCmd[1:], - NetworkMode: expected, - } - task := &drivers.TaskConfig{ - ID: uuid.Generate(), - Name: "busybox-demo", - Resources: basicResources, - } - require.NoError(t, task.EncodeConcreteDriverConfig(&taskCfg)) - - d := dockerDriverHarness(t, nil) - cleanup := d.MkAllocDir(task, true) - defer cleanup() - copyImage(t, task.TaskDir(), "busybox.tar") - - _, _, err := d.StartTask(task) - require.NoError(t, err) - - require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second)) - - defer d.DestroyTask(task.ID, true) - - dockerDriver, ok := d.Impl().(*Driver) - require.True(t, ok) - - handle, ok := dockerDriver.tasks.Get(task.ID) - require.True(t, ok) - - container, err := client.InspectContainer(handle.containerID) - if err != nil { - t.Fatalf("err: %v", err) - } - - actual := container.HostConfig.NetworkMode - require.Equal(t, expected, actual) -} - -func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { - if !tu.IsTravis() { - t.Parallel() - } - testutil.DockerCompatible(t) - require := require.New(t) - - // Because go-dockerclient doesn't provide api for query network aliases, just check that - // a container can be created with a 'network_aliases' property - - // Create network, network-scoped alias is supported only for containers in user defined networks - client := newTestDockerClient(t) - networkOpts := docker.CreateNetworkOptions{Name: "foobar", Driver: "bridge"} - network, err := client.CreateNetwork(networkOpts) - require.NoError(err) - defer client.RemoveNetwork(network.ID) - - expected := []string{"foobar"} - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: busyboxLongRunningCmd[0], - Args: busyboxLongRunningCmd[1:], - NetworkMode: network.Name, - NetworkAliases: expected, - } - task := &drivers.TaskConfig{ - ID: uuid.Generate(), - Name: "busybox", - Resources: basicResources, - } - require.NoError(task.EncodeConcreteDriverConfig(&taskCfg)) - - d := dockerDriverHarness(t, nil) - cleanup := d.MkAllocDir(task, true) - defer cleanup() - copyImage(t, task.TaskDir(), "busybox.tar") - - _, _, err = d.StartTask(task) - require.NoError(err) - require.NoError(d.WaitUntilStarted(task.ID, 5*time.Second)) - - defer d.DestroyTask(task.ID, true) - - dockerDriver, ok := d.Impl().(*Driver) - require.True(ok) - - handle, ok := dockerDriver.tasks.Get(task.ID) - require.True(ok) - - _, err = client.InspectContainer(handle.containerID) - require.NoError(err) -} - -func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { - testutil.DockerCompatible(t) - - task, cfg, _ := dockerTask(t) - expectedUlimits := map[string]string{ - "nproc": "4242", - "nofile": "2048:4096", - } - cfg.Sysctl = map[string]string{ - "net.core.somaxconn": "16384", - } - cfg.Ulimit = expectedUlimits - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - client, d, handle, cleanup := dockerSetup(t, task) - defer cleanup() - require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second)) - - container, err := client.InspectContainer(handle.containerID) - assert.Nil(t, err, "unexpected error: %v", err) - - want := "16384" - got := container.HostConfig.Sysctls["net.core.somaxconn"] - assert.Equal(t, want, got, "Wrong net.core.somaxconn config for docker job. Expect: %s, got: %s", want, got) - - expectedUlimitLen := 2 - actualUlimitLen := len(container.HostConfig.Ulimits) - assert.Equal(t, want, got, "Wrong number of ulimit configs for docker job. Expect: %d, got: %d", expectedUlimitLen, actualUlimitLen) - - for _, got := range container.HostConfig.Ulimits { - if expectedStr, ok := expectedUlimits[got.Name]; !ok { - t.Errorf("%s config unexpected for docker job.", got.Name) - } else { - if !strings.Contains(expectedStr, ":") { - expectedStr = expectedStr + ":" + expectedStr - } - - splitted := strings.SplitN(expectedStr, ":", 2) - soft, _ := strconv.Atoi(splitted[0]) - hard, _ := strconv.Atoi(splitted[1]) - assert.Equal(t, int64(soft), got.Soft, "Wrong soft %s ulimit for docker job. Expect: %d, got: %d", got.Name, soft, got.Soft) - assert.Equal(t, int64(hard), got.Hard, "Wrong hard %s ulimit for docker job. Expect: %d, got: %d", got.Name, hard, got.Hard) - - } - } -} - -func TestDockerDriver_Sysctl_Ulimit_Errors(t *testing.T) { - testutil.DockerCompatible(t) - - brokenConfigs := []map[string]string{ - { - "nofile": "", - }, - { - "nofile": "abc:1234", - }, - { - "nofile": "1234:abc", - }, - } - - testCases := []struct { - ulimitConfig map[string]string - err error - }{ - {brokenConfigs[0], fmt.Errorf("Malformed ulimit specification nofile: \"\", cannot be empty")}, - {brokenConfigs[1], fmt.Errorf("Malformed soft ulimit nofile: abc:1234")}, - {brokenConfigs[2], fmt.Errorf("Malformed hard ulimit nofile: 1234:abc")}, - } - - for _, tc := range testCases { - task, cfg, _ := dockerTask(t) - cfg.Ulimit = tc.ulimitConfig - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - d := dockerDriverHarness(t, nil) - cleanup := d.MkAllocDir(task, true) - defer cleanup() - copyImage(t, task.TaskDir(), "busybox.tar") - - _, _, err := d.StartTask(task) - require.NotNil(t, err, "Expected non nil error") - require.Contains(t, err.Error(), tc.err.Error()) - } -} - func TestDockerDriver_Labels(t *testing.T) { if !tu.IsTravis() { t.Parallel() @@ -1075,6 +853,10 @@ func TestDockerDriver_ForcePull(t *testing.T) { } func TestDockerDriver_ForcePull_RepoDigest(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("TODO: Skipped digest test on Windows") + } + if !tu.IsTravis() { t.Parallel() } @@ -1099,6 +881,9 @@ func TestDockerDriver_ForcePull_RepoDigest(t *testing.T) { } func TestDockerDriver_SecurityOpt(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not support seccomp") + } if !tu.IsTravis() { t.Parallel() } @@ -1385,8 +1170,7 @@ func TestDockerWorkDir(t *testing.T) { container, err := client.InspectContainer(handle.containerID) require.NoError(t, err) - - require.Equal(t, cfg.WorkDir, container.Config.WorkingDir) + require.Equal(t, cfg.WorkDir, filepath.ToSlash(container.Config.WorkingDir)) } func inSlice(needle string, haystack []string) bool { @@ -1425,12 +1209,17 @@ func TestDockerDriver_PortsNoMap(t *testing.T) { require.Exactly(t, expectedExposedPorts, container.Config.ExposedPorts) + hostIP := "127.0.0.1" + if runtime.GOOS == "windows" { + hostIP = "" + } + // Verify that the correct ports are FORWARDED expectedPortBindings := map[docker.Port][]docker.PortBinding{ - docker.Port(fmt.Sprintf("%d/tcp", res)): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", res)}}, - docker.Port(fmt.Sprintf("%d/udp", res)): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", res)}}, - docker.Port(fmt.Sprintf("%d/tcp", dyn)): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", dyn)}}, - docker.Port(fmt.Sprintf("%d/udp", dyn)): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", dyn)}}, + docker.Port(fmt.Sprintf("%d/tcp", res)): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", res)}}, + docker.Port(fmt.Sprintf("%d/udp", res)): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", res)}}, + docker.Port(fmt.Sprintf("%d/tcp", dyn)): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", dyn)}}, + docker.Port(fmt.Sprintf("%d/udp", dyn)): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", dyn)}}, } require.Exactly(t, expectedPortBindings, container.HostConfig.PortBindings) @@ -1468,43 +1257,21 @@ func TestDockerDriver_PortsMapping(t *testing.T) { require.Exactly(t, expectedExposedPorts, container.Config.ExposedPorts) + hostIP := "127.0.0.1" + if runtime.GOOS == "windows" { + hostIP = "" + } + // Verify that the correct ports are FORWARDED expectedPortBindings := map[docker.Port][]docker.PortBinding{ - docker.Port("8080/tcp"): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", res)}}, - docker.Port("8080/udp"): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", res)}}, - docker.Port("6379/tcp"): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", dyn)}}, - docker.Port("6379/udp"): {{HostIP: "127.0.0.1", HostPort: fmt.Sprintf("%d", dyn)}}, + docker.Port("8080/tcp"): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", res)}}, + docker.Port("8080/udp"): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", res)}}, + docker.Port("6379/tcp"): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", dyn)}}, + docker.Port("6379/udp"): {{HostIP: hostIP, HostPort: fmt.Sprintf("%d", dyn)}}, } require.Exactly(t, expectedPortBindings, container.HostConfig.PortBindings) } -func TestDockerDriver_User(t *testing.T) { - if !tu.IsTravis() { - t.Parallel() - } - testutil.DockerCompatible(t) - task, cfg, _ := dockerTask(t) - task.User = "alice" - cfg.Command = "/bin/sleep" - cfg.Args = []string{"10000"} - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - d := dockerDriverHarness(t, nil) - cleanup := d.MkAllocDir(task, true) - defer cleanup() - copyImage(t, task.TaskDir(), "busybox.tar") - - _, _, err := d.StartTask(task) - if err == nil { - d.DestroyTask(task.ID, true) - t.Fatalf("Should've failed") - } - - if !strings.Contains(err.Error(), "alice") { - t.Fatalf("Expected failure string not found, found %q instead", err.Error()) - } -} - func TestDockerDriver_CleanupContainer(t *testing.T) { if !tu.IsTravis() { t.Parallel() @@ -1512,7 +1279,7 @@ func TestDockerDriver_CleanupContainer(t *testing.T) { testutil.DockerCompatible(t) task, cfg, _ := dockerTask(t) - cfg.Command = "/bin/echo" + cfg.Command = "echo" cfg.Args = []string{"hello"} require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) @@ -1547,7 +1314,7 @@ func TestDockerDriver_Stats(t *testing.T) { testutil.DockerCompatible(t) task, cfg, _ := dockerTask(t) - cfg.Command = "/bin/sleep" + cfg.Command = "sleep" cfg.Args = []string{"1000"} require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) @@ -1586,16 +1353,17 @@ func setupDockerVolumes(t *testing.T, cfg map[string]interface{}, hostpath strin randfn := fmt.Sprintf("test-%d", rand.Int()) hostfile := filepath.Join(hostpath, randfn) - containerPath := "/mnt/vol" + var containerPath string + if runtime.GOOS == "windows" { + containerPath = "C:\\data" + } else { + containerPath = "/mnt/vol" + } containerFile := filepath.Join(containerPath, randfn) - taskCfg := &TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "touch", - Args: []string{containerFile}, - Volumes: []string{fmt.Sprintf("%s:%s", hostpath, containerPath)}, - } + taskCfg := newTaskConfig("", []string{"touch", containerFile}) + taskCfg.Volumes = []string{fmt.Sprintf("%s:%s", hostpath, containerPath)} + task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "ls", @@ -1609,7 +1377,7 @@ func setupDockerVolumes(t *testing.T, cfg map[string]interface{}, hostpath strin copyImage(t, task.TaskDir(), "busybox.tar") - return task, d, taskCfg, hostfile, cleanup + return task, d, &taskCfg, hostfile, cleanup } func TestDockerDriver_VolumesDisabled(t *testing.T) { @@ -1678,144 +1446,6 @@ func TestDockerDriver_VolumesDisabled(t *testing.T) { require.Fail(t, "Started driver successfully when volume drivers should have been disabled.") } } - -} - -func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) { - t.Parallel() - - allocDir := "/tmp/nomad/alloc-dir" - - cases := []struct { - name string - requiresVolumes bool - - volumeDriver string - volumes []string - - expectedVolumes []string - }{ - { - name: "basic plugin", - requiresVolumes: true, - volumeDriver: "nfs", - volumes: []string{"test-path:/tmp/taskpath"}, - expectedVolumes: []string{"test-path:/tmp/taskpath"}, - }, - { - name: "absolute default driver", - requiresVolumes: true, - volumeDriver: "", - volumes: []string{"/abs/test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/abs/test-path:/tmp/taskpath"}, - }, - { - name: "absolute local driver", - requiresVolumes: true, - volumeDriver: "local", - volumes: []string{"/abs/test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/abs/test-path:/tmp/taskpath"}, - }, - { - name: "relative default driver", - requiresVolumes: false, - volumeDriver: "", - volumes: []string{"test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"}, - }, - { - name: "relative local driver", - requiresVolumes: false, - volumeDriver: "local", - volumes: []string{"test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"}, - }, - { - name: "relative outside task-dir default driver", - requiresVolumes: false, - volumeDriver: "", - volumes: []string{"../test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"}, - }, - { - name: "relative outside task-dir local driver", - requiresVolumes: false, - volumeDriver: "local", - volumes: []string{"../test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"}, - }, - { - name: "relative outside alloc-dir default driver", - requiresVolumes: true, - volumeDriver: "", - volumes: []string{"../../test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"}, - }, - { - name: "relative outside task-dir local driver", - requiresVolumes: true, - volumeDriver: "local", - volumes: []string{"../../test-path:/tmp/taskpath"}, - expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"}, - }, - } - - t.Run("with volumes enabled", func(t *testing.T) { - dh := dockerDriverHarness(t, nil) - driver := dh.Impl().(*Driver) - driver.config.Volumes.Enabled = true - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - task, cfg, _ := dockerTask(t) - cfg.VolumeDriver = c.volumeDriver - cfg.Volumes = c.volumes - - task.AllocDir = allocDir - task.Name = "demo" - - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") - require.NoError(t, err) - - for _, v := range c.expectedVolumes { - require.Contains(t, cc.HostConfig.Binds, v) - } - }) - } - }) - - t.Run("with volumes disabled", func(t *testing.T) { - dh := dockerDriverHarness(t, nil) - driver := dh.Impl().(*Driver) - driver.config.Volumes.Enabled = false - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - task, cfg, _ := dockerTask(t) - cfg.VolumeDriver = c.volumeDriver - cfg.Volumes = c.volumes - - task.AllocDir = allocDir - task.Name = "demo" - - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") - if c.requiresVolumes { - require.Error(t, err, "volumes are not enabled") - } else { - require.NoError(t, err) - - for _, v := range c.expectedVolumes { - require.Contains(t, cc.HostConfig.Binds, v) - } - } - }) - } - }) - } func TestDockerDriver_VolumesEnabled(t *testing.T) { @@ -1872,6 +1502,10 @@ func TestDockerDriver_Mounts(t *testing.T) { Source: "test", } + if runtime.GOOS == "windows" { + goodMount.Target = "C:\\nomad" + } + cases := []struct { Name string Mounts []DockerMount @@ -1894,7 +1528,7 @@ func TestDockerDriver_Mounts(t *testing.T) { d := dockerDriverHarness(t, nil) // Build the task task, cfg, _ := dockerTask(t) - cfg.Command = "/bin/sleep" + cfg.Command = "sleep" cfg.Args = []string{"10000"} cfg.Mounts = c.Mounts require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) @@ -1918,273 +1552,18 @@ func TestDockerDriver_Mounts(t *testing.T) { } } -func TestDockerDriver_MountsSerialization(t *testing.T) { - t.Parallel() - - allocDir := "/tmp/nomad/alloc-dir" - - cases := []struct { - name string - requiresVolumes bool - passedMounts []DockerMount - expectedMounts []docker.HostMount - }{ - { - name: "basic volume", - passedMounts: []DockerMount{ - { - Target: "/nomad", - ReadOnly: true, - Source: "test", - }, - }, - expectedMounts: []docker.HostMount{ - { - Type: "volume", - Target: "/nomad", - Source: "test", - ReadOnly: true, - VolumeOptions: &docker.VolumeOptions{}, - }, - }, - }, - { - name: "basic bind", - passedMounts: []DockerMount{ - { - Type: "bind", - Target: "/nomad", - Source: "test", - }, - }, - expectedMounts: []docker.HostMount{ - { - Type: "bind", - Target: "/nomad", - Source: "/tmp/nomad/alloc-dir/demo/test", - BindOptions: &docker.BindOptions{}, - }, - }, - }, - { - name: "basic absolute bind", - requiresVolumes: true, - passedMounts: []DockerMount{ - { - Type: "bind", - Target: "/nomad", - Source: "/tmp/test", - }, - }, - expectedMounts: []docker.HostMount{ - { - Type: "bind", - Target: "/nomad", - Source: "/tmp/test", - BindOptions: &docker.BindOptions{}, - }, - }, - }, - { - name: "bind relative outside", - requiresVolumes: true, - passedMounts: []DockerMount{ - { - Type: "bind", - Target: "/nomad", - Source: "../../test", - }, - }, - expectedMounts: []docker.HostMount{ - { - Type: "bind", - Target: "/nomad", - Source: "/tmp/nomad/test", - BindOptions: &docker.BindOptions{}, - }, - }, - }, - { - name: "basic tmpfs", - requiresVolumes: false, - passedMounts: []DockerMount{ - { - Type: "tmpfs", - Target: "/nomad", - TmpfsOptions: DockerTmpfsOptions{ - SizeBytes: 321, - Mode: 0666, - }, - }, - }, - expectedMounts: []docker.HostMount{ - { - Type: "tmpfs", - Target: "/nomad", - TempfsOptions: &docker.TempfsOptions{ - SizeBytes: 321, - Mode: 0666, - }, - }, - }, - }, - } - - t.Run("with volumes enabled", func(t *testing.T) { - dh := dockerDriverHarness(t, nil) - driver := dh.Impl().(*Driver) - driver.config.Volumes.Enabled = true - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - task, cfg, _ := dockerTask(t) - cfg.Mounts = c.passedMounts - - task.AllocDir = allocDir - task.Name = "demo" - - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") - require.NoError(t, err) - require.EqualValues(t, c.expectedMounts, cc.HostConfig.Mounts) - }) - } - }) - - t.Run("with volumes disabled", func(t *testing.T) { - dh := dockerDriverHarness(t, nil) - driver := dh.Impl().(*Driver) - driver.config.Volumes.Enabled = false - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - task, cfg, _ := dockerTask(t) - cfg.Mounts = c.passedMounts - - task.AllocDir = allocDir - task.Name = "demo" - - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") - if c.requiresVolumes { - require.Error(t, err, "volumes are not enabled") - } else { - require.NoError(t, err) - require.EqualValues(t, c.expectedMounts, cc.HostConfig.Mounts) - } - }) - } - }) - -} - -// TestDockerDriver_CreateContainerConfig_MountsCombined asserts that -// devices and mounts set by device managers/plugins are honored -// and present in docker.CreateContainerOptions, and that it is appended -// to any devices/mounts a user sets in the task config. -func TestDockerDriver_CreateContainerConfig_MountsCombined(t *testing.T) { - t.Parallel() - - task, cfg, _ := dockerTask(t) - - task.Devices = []*drivers.DeviceConfig{ - { - HostPath: "/dev/fuse", - TaskPath: "/container/dev/task-fuse", - Permissions: "rw", - }, - } - task.Mounts = []*drivers.MountConfig{ - { - HostPath: "/tmp/task-mount", - TaskPath: "/container/tmp/task-mount", - Readonly: true, - }, - } - - cfg.Devices = []DockerDevice{ - { - HostPath: "/dev/stdout", - ContainerPath: "/container/dev/cfg-stdout", - CgroupPermissions: "rwm", - }, - } - cfg.Mounts = []DockerMount{ - { - Type: "bind", - Source: "/tmp/cfg-mount", - Target: "/container/tmp/cfg-mount", - ReadOnly: false, - }, - } - - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - dh := dockerDriverHarness(t, nil) - driver := dh.Impl().(*Driver) - - c, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") - require.NoError(t, err) - - expectedMounts := []docker.HostMount{ - { - Type: "bind", - Source: "/tmp/cfg-mount", - Target: "/container/tmp/cfg-mount", - ReadOnly: false, - BindOptions: &docker.BindOptions{}, - }, - { - Type: "bind", - Source: "/tmp/task-mount", - Target: "/container/tmp/task-mount", - ReadOnly: true, - }, - } - foundMounts := c.HostConfig.Mounts - sort.Slice(foundMounts, func(i, j int) bool { - return foundMounts[i].Target < foundMounts[j].Target - }) - require.EqualValues(t, expectedMounts, foundMounts) - - expectedDevices := []docker.Device{ - { - PathOnHost: "/dev/stdout", - PathInContainer: "/container/dev/cfg-stdout", - CgroupPermissions: "rwm", - }, - { - PathOnHost: "/dev/fuse", - PathInContainer: "/container/dev/task-fuse", - CgroupPermissions: "rw", - }, - } - - foundDevices := c.HostConfig.Devices - sort.Slice(foundDevices, func(i, j int) bool { - return foundDevices[i].PathInContainer < foundDevices[j].PathInContainer - }) - require.EqualValues(t, expectedDevices, foundDevices) -} - // TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images. func TestDockerDriver_Cleanup(t *testing.T) { testutil.DockerCompatible(t) // using a small image and an specific point release to avoid accidental conflicts with other tasks - imageName := "busybox:1.27.1" + cfg := newTaskConfig("", []string{"sleep", "100"}) + cfg.LoadImage = "" task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "cleanup_test", Resources: basicResources, } - cfg := &TaskConfig{ - Image: imageName, - Command: "/bin/sleep", - Args: []string{"100"}, - } require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) @@ -2197,8 +1576,8 @@ func TestDockerDriver_Cleanup(t *testing.T) { // Ensure image was removed tu.WaitForResult(func() (bool, error) { - if _, err := client.InspectImage(imageName); err == nil { - return false, fmt.Errorf("image exists but should have been removed. Does another %v container exist?", imageName) + if _, err := client.InspectImage(cfg.Image); err == nil { + return false, fmt.Errorf("image exists but should have been removed. Does another %v container exist?", cfg.Image) } return true, nil @@ -2269,12 +1648,7 @@ func TestDockerDriver_OOMKilled(t *testing.T) { } testutil.DockerCompatible(t) - taskCfg := TaskConfig{ - Image: busyboxImageID, - LoadImage: "busybox.tar", - Command: "/bin/sh", - Args: []string{"-c", `/bin/sleep 2 && x=a && while true; do x="$x$x"; done`}, - } + taskCfg := newTaskConfig("", []string{"sh", "-c", `sleep 2 && x=a && while true; do x="$x$x"; done`}) task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "oom-killed", @@ -2398,7 +1772,7 @@ func TestDockerDriver_Entrypoint(t *testing.T) { } testutil.DockerCompatible(t) - entrypoint := []string{"/bin/sh", "-c"} + entrypoint := []string{"sh", "-c"} task, cfg, _ := dockerTask(t) cfg.Entrypoint = entrypoint cfg.Command = strings.Join(busyboxLongRunningCmd, " ") @@ -2560,28 +1934,6 @@ func TestDockerImageRef(t *testing.T) { } } -func TestDockerDriver_CPUCFSPeriod(t *testing.T) { - if !tu.IsTravis() { - t.Parallel() - } - testutil.DockerCompatible(t) - - task, cfg, _ := dockerTask(t) - cfg.CPUHardLimit = true - cfg.CPUCFSPeriod = 1000000 - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - client, _, handle, cleanup := dockerSetup(t, task) - defer cleanup() - - waitForExist(t, client, handle.containerID) - - container, err := client.InspectContainer(handle.containerID) - require.NoError(t, err) - - require.Equal(t, cfg.CPUCFSPeriod, container.HostConfig.CPUPeriod) -} - func waitForExist(t *testing.T, client *docker.Client, containerID string) { tu.WaitForResult(func() (bool, error) { container, err := client.InspectContainer(containerID) @@ -2596,32 +1948,3 @@ func waitForExist(t *testing.T, client *docker.Client, containerID string) { require.NoError(t, err) }) } - -func copyImage(t *testing.T, taskDir *allocdir.TaskDir, image string) { - dst := filepath.Join(taskDir.LocalDir, image) - copyFile(filepath.Join("./test-resources/docker", image), dst, t) -} - -// copyFile moves an existing file to the destination -func copyFile(src, dst string, t *testing.T) { - in, err := os.Open(src) - if err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - defer in.Close() - out, err := os.Create(dst) - if err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - defer func() { - if err := out.Close(); err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - }() - if _, err = io.Copy(out, in); err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - if err := out.Sync(); err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } -} diff --git a/drivers/docker/driver_unix_test.go b/drivers/docker/driver_unix_test.go new file mode 100644 index 000000000..2ff3024a9 --- /dev/null +++ b/drivers/docker/driver_unix_test.go @@ -0,0 +1,643 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package docker + +import ( + "fmt" + "sort" + "strconv" + "strings" + "testing" + "time" + + docker "github.com/fsouza/go-dockerclient" + "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/helper/uuid" + "github.com/hashicorp/nomad/plugins/drivers" + tu "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDockerDriver_User(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + testutil.DockerCompatible(t) + task, cfg, _ := dockerTask(t) + task.User = "alice" + cfg.Command = "/bin/sleep" + cfg.Args = []string{"10000"} + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + d := dockerDriverHarness(t, nil) + cleanup := d.MkAllocDir(task, true) + defer cleanup() + copyImage(t, task.TaskDir(), "busybox.tar") + + _, _, err := d.StartTask(task) + if err == nil { + d.DestroyTask(task.ID, true) + t.Fatalf("Should've failed") + } + + if !strings.Contains(err.Error(), "alice") { + t.Fatalf("Expected failure string not found, found %q instead", err.Error()) + } +} + +func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + testutil.DockerCompatible(t) + require := require.New(t) + + // Because go-dockerclient doesn't provide api for query network aliases, just check that + // a container can be created with a 'network_aliases' property + + // Create network, network-scoped alias is supported only for containers in user defined networks + client := newTestDockerClient(t) + networkOpts := docker.CreateNetworkOptions{Name: "foobar", Driver: "bridge"} + network, err := client.CreateNetwork(networkOpts) + require.NoError(err) + defer client.RemoveNetwork(network.ID) + + expected := []string{"foobar"} + taskCfg := newTaskConfig("", busyboxLongRunningCmd) + taskCfg.NetworkMode = network.Name + taskCfg.NetworkAliases = expected + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "busybox", + Resources: basicResources, + } + require.NoError(task.EncodeConcreteDriverConfig(&taskCfg)) + + d := dockerDriverHarness(t, nil) + cleanup := d.MkAllocDir(task, true) + defer cleanup() + copyImage(t, task.TaskDir(), "busybox.tar") + + _, _, err = d.StartTask(task) + require.NoError(err) + require.NoError(d.WaitUntilStarted(task.ID, 5*time.Second)) + + defer d.DestroyTask(task.ID, true) + + dockerDriver, ok := d.Impl().(*Driver) + require.True(ok) + + handle, ok := dockerDriver.tasks.Get(task.ID) + require.True(ok) + + _, err = client.InspectContainer(handle.containerID) + require.NoError(err) +} + +func TestDockerDriver_NetworkMode_Host(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + testutil.DockerCompatible(t) + expected := "host" + + taskCfg := newTaskConfig("", busyboxLongRunningCmd) + taskCfg.NetworkMode = expected + + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "busybox-demo", + Resources: basicResources, + } + require.NoError(t, task.EncodeConcreteDriverConfig(&taskCfg)) + + d := dockerDriverHarness(t, nil) + cleanup := d.MkAllocDir(task, true) + defer cleanup() + copyImage(t, task.TaskDir(), "busybox.tar") + + _, _, err := d.StartTask(task) + require.NoError(t, err) + + require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second)) + + defer d.DestroyTask(task.ID, true) + + dockerDriver, ok := d.Impl().(*Driver) + require.True(t, ok) + + handle, ok := dockerDriver.tasks.Get(task.ID) + require.True(t, ok) + + container, err := client.InspectContainer(handle.containerID) + if err != nil { + t.Fatalf("err: %v", err) + } + + actual := container.HostConfig.NetworkMode + require.Equal(t, expected, actual) +} + +func TestDockerDriver_CPUCFSPeriod(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + testutil.DockerCompatible(t) + + task, cfg, _ := dockerTask(t) + cfg.CPUHardLimit = true + cfg.CPUCFSPeriod = 1000000 + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + client, _, handle, cleanup := dockerSetup(t, task) + defer cleanup() + + waitForExist(t, client, handle.containerID) + + container, err := client.InspectContainer(handle.containerID) + require.NoError(t, err) + + require.Equal(t, cfg.CPUCFSPeriod, container.HostConfig.CPUPeriod) +} + +func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { + testutil.DockerCompatible(t) + task, cfg, _ := dockerTask(t) + expectedUlimits := map[string]string{ + "nproc": "4242", + "nofile": "2048:4096", + } + cfg.Sysctl = map[string]string{ + "net.core.somaxconn": "16384", + } + cfg.Ulimit = expectedUlimits + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + client, d, handle, cleanup := dockerSetup(t, task) + defer cleanup() + require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second)) + + container, err := client.InspectContainer(handle.containerID) + assert.Nil(t, err, "unexpected error: %v", err) + + want := "16384" + got := container.HostConfig.Sysctls["net.core.somaxconn"] + assert.Equal(t, want, got, "Wrong net.core.somaxconn config for docker job. Expect: %s, got: %s", want, got) + + expectedUlimitLen := 2 + actualUlimitLen := len(container.HostConfig.Ulimits) + assert.Equal(t, want, got, "Wrong number of ulimit configs for docker job. Expect: %d, got: %d", expectedUlimitLen, actualUlimitLen) + + for _, got := range container.HostConfig.Ulimits { + if expectedStr, ok := expectedUlimits[got.Name]; !ok { + t.Errorf("%s config unexpected for docker job.", got.Name) + } else { + if !strings.Contains(expectedStr, ":") { + expectedStr = expectedStr + ":" + expectedStr + } + + splitted := strings.SplitN(expectedStr, ":", 2) + soft, _ := strconv.Atoi(splitted[0]) + hard, _ := strconv.Atoi(splitted[1]) + assert.Equal(t, int64(soft), got.Soft, "Wrong soft %s ulimit for docker job. Expect: %d, got: %d", got.Name, soft, got.Soft) + assert.Equal(t, int64(hard), got.Hard, "Wrong hard %s ulimit for docker job. Expect: %d, got: %d", got.Name, hard, got.Hard) + + } + } +} + +func TestDockerDriver_Sysctl_Ulimit_Errors(t *testing.T) { + testutil.DockerCompatible(t) + brokenConfigs := []map[string]string{ + { + "nofile": "", + }, + { + "nofile": "abc:1234", + }, + { + "nofile": "1234:abc", + }, + } + + testCases := []struct { + ulimitConfig map[string]string + err error + }{ + {brokenConfigs[0], fmt.Errorf("Malformed ulimit specification nofile: \"\", cannot be empty")}, + {brokenConfigs[1], fmt.Errorf("Malformed soft ulimit nofile: abc:1234")}, + {brokenConfigs[2], fmt.Errorf("Malformed hard ulimit nofile: 1234:abc")}, + } + + for _, tc := range testCases { + task, cfg, _ := dockerTask(t) + cfg.Ulimit = tc.ulimitConfig + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + d := dockerDriverHarness(t, nil) + cleanup := d.MkAllocDir(task, true) + defer cleanup() + copyImage(t, task.TaskDir(), "busybox.tar") + + _, _, err := d.StartTask(task) + require.NotNil(t, err, "Expected non nil error") + require.Contains(t, err.Error(), tc.err.Error()) + } +} + +// This test does not run on Windows due to stricter path validation in the +// negative case for non existent mount paths. We should write a similar test +// for windows. +func TestDockerDriver_BindMountsHonorVolumesEnabledFlag(t *testing.T) { + t.Parallel() + + testutil.DockerCompatible(t) + + allocDir := "/tmp/nomad/alloc-dir" + + cases := []struct { + name string + requiresVolumes bool + + volumeDriver string + volumes []string + + expectedVolumes []string + }{ + { + name: "basic plugin", + requiresVolumes: true, + volumeDriver: "nfs", + volumes: []string{"test-path:/tmp/taskpath"}, + expectedVolumes: []string{"test-path:/tmp/taskpath"}, + }, + { + name: "absolute default driver", + requiresVolumes: true, + volumeDriver: "", + volumes: []string{"/abs/test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/abs/test-path:/tmp/taskpath"}, + }, + { + name: "absolute local driver", + requiresVolumes: true, + volumeDriver: "local", + volumes: []string{"/abs/test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/abs/test-path:/tmp/taskpath"}, + }, + { + name: "relative default driver", + requiresVolumes: false, + volumeDriver: "", + volumes: []string{"test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"}, + }, + { + name: "relative local driver", + requiresVolumes: false, + volumeDriver: "local", + volumes: []string{"test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/tmp/nomad/alloc-dir/demo/test-path:/tmp/taskpath"}, + }, + { + name: "relative outside task-dir default driver", + requiresVolumes: false, + volumeDriver: "", + volumes: []string{"../test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"}, + }, + { + name: "relative outside task-dir local driver", + requiresVolumes: false, + volumeDriver: "local", + volumes: []string{"../test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/tmp/nomad/alloc-dir/test-path:/tmp/taskpath"}, + }, + { + name: "relative outside alloc-dir default driver", + requiresVolumes: true, + volumeDriver: "", + volumes: []string{"../../test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"}, + }, + { + name: "relative outside task-dir local driver", + requiresVolumes: true, + volumeDriver: "local", + volumes: []string{"../../test-path:/tmp/taskpath"}, + expectedVolumes: []string{"/tmp/nomad/test-path:/tmp/taskpath"}, + }, + } + + t.Run("with volumes enabled", func(t *testing.T) { + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + driver.config.Volumes.Enabled = true + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + cfg.VolumeDriver = c.volumeDriver + cfg.Volumes = c.volumes + + task.AllocDir = allocDir + task.Name = "demo" + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + require.NoError(t, err) + + for _, v := range c.expectedVolumes { + require.Contains(t, cc.HostConfig.Binds, v) + } + }) + } + }) + + t.Run("with volumes disabled", func(t *testing.T) { + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + driver.config.Volumes.Enabled = false + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + cfg.VolumeDriver = c.volumeDriver + cfg.Volumes = c.volumes + + task.AllocDir = allocDir + task.Name = "demo" + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + if c.requiresVolumes { + require.Error(t, err, "volumes are not enabled") + } else { + require.NoError(t, err) + + for _, v := range c.expectedVolumes { + require.Contains(t, cc.HostConfig.Binds, v) + } + } + }) + } + }) +} + +// This test does not run on windows due to differences in the definition of +// an absolute path, changing path expansion behaviour. A similar test should +// be written for windows. +func TestDockerDriver_MountsSerialization(t *testing.T) { + t.Parallel() + testutil.DockerCompatible(t) + + allocDir := "/tmp/nomad/alloc-dir" + + cases := []struct { + name string + requiresVolumes bool + passedMounts []DockerMount + expectedMounts []docker.HostMount + }{ + { + name: "basic volume", + passedMounts: []DockerMount{ + { + Target: "/nomad", + ReadOnly: true, + Source: "test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "volume", + Target: "/nomad", + Source: "test", + ReadOnly: true, + VolumeOptions: &docker.VolumeOptions{}, + }, + }, + }, + { + name: "basic bind", + passedMounts: []DockerMount{ + { + Type: "bind", + Target: "/nomad", + Source: "test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/nomad/alloc-dir/demo/test", + BindOptions: &docker.BindOptions{}, + }, + }, + }, + { + name: "basic absolute bind", + requiresVolumes: true, + passedMounts: []DockerMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/test", + BindOptions: &docker.BindOptions{}, + }, + }, + }, + { + name: "bind relative outside", + requiresVolumes: true, + passedMounts: []DockerMount{ + { + Type: "bind", + Target: "/nomad", + Source: "../../test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/nomad/test", + BindOptions: &docker.BindOptions{}, + }, + }, + }, + { + name: "basic tmpfs", + requiresVolumes: false, + passedMounts: []DockerMount{ + { + Type: "tmpfs", + Target: "/nomad", + TmpfsOptions: DockerTmpfsOptions{ + SizeBytes: 321, + Mode: 0666, + }, + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "tmpfs", + Target: "/nomad", + TempfsOptions: &docker.TempfsOptions{ + SizeBytes: 321, + Mode: 0666, + }, + }, + }, + }, + } + + t.Run("with volumes enabled", func(t *testing.T) { + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + driver.config.Volumes.Enabled = true + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + cfg.Mounts = c.passedMounts + + task.AllocDir = allocDir + task.Name = "demo" + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + require.NoError(t, err) + require.EqualValues(t, c.expectedMounts, cc.HostConfig.Mounts) + }) + } + }) + + t.Run("with volumes disabled", func(t *testing.T) { + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + driver.config.Volumes.Enabled = false + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + cfg.Mounts = c.passedMounts + + task.AllocDir = allocDir + task.Name = "demo" + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + if c.requiresVolumes { + require.Error(t, err, "volumes are not enabled") + } else { + require.NoError(t, err) + require.EqualValues(t, c.expectedMounts, cc.HostConfig.Mounts) + } + }) + } + }) +} + +// TestDockerDriver_CreateContainerConfig_MountsCombined asserts that +// devices and mounts set by device managers/plugins are honored +// and present in docker.CreateContainerOptions, and that it is appended +// to any devices/mounts a user sets in the task config. +func TestDockerDriver_CreateContainerConfig_MountsCombined(t *testing.T) { + t.Parallel() + testutil.DockerCompatible(t) + + task, cfg, _ := dockerTask(t) + + task.Devices = []*drivers.DeviceConfig{ + { + HostPath: "/dev/fuse", + TaskPath: "/container/dev/task-fuse", + Permissions: "rw", + }, + } + task.Mounts = []*drivers.MountConfig{ + { + HostPath: "/tmp/task-mount", + TaskPath: "/container/tmp/task-mount", + Readonly: true, + }, + } + + cfg.Devices = []DockerDevice{ + { + HostPath: "/dev/stdout", + ContainerPath: "/container/dev/cfg-stdout", + CgroupPermissions: "rwm", + }, + } + cfg.Mounts = []DockerMount{ + { + Type: "bind", + Source: "/tmp/cfg-mount", + Target: "/container/tmp/cfg-mount", + ReadOnly: false, + }, + } + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + + c, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + require.NoError(t, err) + + expectedMounts := []docker.HostMount{ + { + Type: "bind", + Source: "/tmp/cfg-mount", + Target: "/container/tmp/cfg-mount", + ReadOnly: false, + BindOptions: &docker.BindOptions{}, + }, + { + Type: "bind", + Source: "/tmp/task-mount", + Target: "/container/tmp/task-mount", + ReadOnly: true, + }, + } + foundMounts := c.HostConfig.Mounts + sort.Slice(foundMounts, func(i, j int) bool { + return foundMounts[i].Target < foundMounts[j].Target + }) + require.EqualValues(t, expectedMounts, foundMounts) + + expectedDevices := []docker.Device{ + { + PathOnHost: "/dev/stdout", + PathInContainer: "/container/dev/cfg-stdout", + CgroupPermissions: "rwm", + }, + { + PathOnHost: "/dev/fuse", + PathInContainer: "/container/dev/task-fuse", + CgroupPermissions: "rw", + }, + } + + foundDevices := c.HostConfig.Devices + sort.Slice(foundDevices, func(i, j int) bool { + return foundDevices[i].PathInContainer < foundDevices[j].PathInContainer + }) + require.EqualValues(t, expectedDevices, foundDevices) +} diff --git a/drivers/docker/testing_unix.go b/drivers/docker/testing_unix.go new file mode 100644 index 000000000..c9189f134 --- /dev/null +++ b/drivers/docker/testing_unix.go @@ -0,0 +1,61 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package docker + +import ( + "fmt" + "io" + "os" + "path/filepath" + + "github.com/hashicorp/nomad/client/allocdir" + testing "github.com/mitchellh/go-testing-interface" +) + +func newTaskConfig(variant string, command []string) TaskConfig { + // busyboxImageID is the ID stored in busybox.tar + busyboxImageID := "busybox:1.29.3" + + image := busyboxImageID + loadImage := "busybox.tar" + if variant != "" { + image = fmt.Sprintf("%s-%s", busyboxImageID, variant) + loadImage = fmt.Sprintf("busybox_%s.tar", variant) + } + + return TaskConfig{ + Image: image, + LoadImage: loadImage, + Command: command[0], + Args: command[1:], + } +} + +func copyImage(t testing.T, taskDir *allocdir.TaskDir, image string) { + dst := filepath.Join(taskDir.LocalDir, image) + copyFile(filepath.Join("./test-resources/docker", image), dst, t) +} + +// copyFile moves an existing file to the destination +func copyFile(src, dst string, t testing.T) { + in, err := os.Open(src) + if err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + defer func() { + if err := out.Close(); err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + }() + if _, err = io.Copy(out, in); err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + if err := out.Sync(); err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } +} diff --git a/drivers/docker/testing_windows.go b/drivers/docker/testing_windows.go new file mode 100644 index 000000000..2c5c89efc --- /dev/null +++ b/drivers/docker/testing_windows.go @@ -0,0 +1,25 @@ +// +build windows + +package docker + +import ( + testing "github.com/mitchellh/go-testing-interface" + + "github.com/hashicorp/nomad/client/allocdir" +) + +func newTaskConfig(variant string, command []string) TaskConfig { + // busyboxImageID is an id of an image containting nanoserver windows and + // a busybox exe. + // See https://github.com/dantoml/windows/blob/81cff1ed77729d1fa36721abd6cb6efebff2f8ef/docker/busybox/Dockerfile + busyboxImageID := "dantoml/busybox-windows:08012019" + + return TaskConfig{ + Image: busyboxImageID, + Command: command[0], + Args: command[1:], + } +} + +func copyImage(t testing.T, taskDir *allocdir.TaskDir, image string) { +} diff --git a/drivers/docker/utils_test.go b/drivers/docker/utils_test.go index 8f196f321..2744d5865 100644 --- a/drivers/docker/utils_test.go +++ b/drivers/docker/utils_test.go @@ -1,66 +1,11 @@ package docker import ( - "path/filepath" "testing" "github.com/stretchr/testify/require" ) -func TestValidateCgroupPermission(t *testing.T) { - positiveCases := []string{ - "r", - "rw", - "rwm", - "mr", - "mrw", - "", - } - - for _, c := range positiveCases { - t.Run("positive case: "+c, func(t *testing.T) { - require.True(t, validateCgroupPermission(c)) - }) - } - - negativeCases := []string{ - "q", - "asdf", - "rq", - } - - for _, c := range negativeCases { - t.Run("negative case: "+c, func(t *testing.T) { - require.False(t, validateCgroupPermission(c)) - }) - } - -} - -func TestExpandPath(t *testing.T) { - cases := []struct { - base string - target string - expected string - }{ - {"/tmp/alloc/task", "/home/user", "/home/user"}, - {"/tmp/alloc/task", "/home/user/..", "/home"}, - - {"/tmp/alloc/task", ".", "/tmp/alloc/task"}, - {"/tmp/alloc/task", "..", "/tmp/alloc"}, - - {"/tmp/alloc/task", "d1/d2", "/tmp/alloc/task/d1/d2"}, - {"/tmp/alloc/task", "../d1/d2", "/tmp/alloc/d1/d2"}, - {"/tmp/alloc/task", "../../d1/d2", "/tmp/d1/d2"}, - } - - for _, c := range cases { - t.Run(c.expected, func(t *testing.T) { - require.Equal(t, c.expected, filepath.ToSlash(expandPath(c.base, c.target))) - }) - } -} - func TestIsParentPath(t *testing.T) { require.True(t, isParentPath("/a/b/c", "/a/b/c")) require.True(t, isParentPath("/a/b/c", "/a/b/c/d")) diff --git a/drivers/docker/utils_unix_test.go b/drivers/docker/utils_unix_test.go new file mode 100644 index 000000000..2260ef89e --- /dev/null +++ b/drivers/docker/utils_unix_test.go @@ -0,0 +1,63 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package docker + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidateCgroupPermission(t *testing.T) { + positiveCases := []string{ + "r", + "rw", + "rwm", + "mr", + "mrw", + "", + } + + for _, c := range positiveCases { + t.Run("positive case: "+c, func(t *testing.T) { + require.True(t, validateCgroupPermission(c)) + }) + } + + negativeCases := []string{ + "q", + "asdf", + "rq", + } + + for _, c := range negativeCases { + t.Run("negative case: "+c, func(t *testing.T) { + require.False(t, validateCgroupPermission(c)) + }) + } +} + +func TestExpandPath(t *testing.T) { + cases := []struct { + base string + target string + expected string + }{ + {"/tmp/alloc/task", ".", "/tmp/alloc/task"}, + {"/tmp/alloc/task", "..", "/tmp/alloc"}, + + {"/tmp/alloc/task", "d1/d2", "/tmp/alloc/task/d1/d2"}, + {"/tmp/alloc/task", "../d1/d2", "/tmp/alloc/d1/d2"}, + {"/tmp/alloc/task", "../../d1/d2", "/tmp/d1/d2"}, + + {"/tmp/alloc/task", "/home/user", "/home/user"}, + {"/tmp/alloc/task", "/home/user/..", "/home"}, + } + + for _, c := range cases { + t.Run(c.expected, func(t *testing.T) { + require.Equal(t, c.expected, filepath.ToSlash(expandPath(c.base, c.target))) + }) + } +} diff --git a/drivers/docker/utils_windows_test.go b/drivers/docker/utils_windows_test.go new file mode 100644 index 000000000..ba0d1bb8c --- /dev/null +++ b/drivers/docker/utils_windows_test.go @@ -0,0 +1,34 @@ +// +build windows + +package docker + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestExpandPath(t *testing.T) { + cases := []struct { + base string + target string + expected string + }{ + {"/tmp/alloc/task", ".", "/tmp/alloc/task"}, + {"/tmp/alloc/task", "..", "/tmp/alloc"}, + + {"/tmp/alloc/task", "d1/d2", "/tmp/alloc/task/d1/d2"}, + {"/tmp/alloc/task", "../d1/d2", "/tmp/alloc/d1/d2"}, + {"/tmp/alloc/task", "../../d1/d2", "/tmp/d1/d2"}, + + {"/tmp/alloc/task", "c:/home/user", "c:/home/user"}, + {"/tmp/alloc/task", "c:/home/user/..", "c:/home"}, + } + + for _, c := range cases { + t.Run(c.expected, func(t *testing.T) { + require.Equal(t, c.expected, filepath.ToSlash(expandPath(c.base, c.target))) + }) + } +} From ffcd76f03fb99fe4e3ee5f52475c5d83413ddf4c Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Fri, 11 Jan 2019 17:28:52 +0100 Subject: [PATCH 22/37] rawexec: SIGINT is not available on Windows --- drivers/rawexec/driver_test.go | 104 +++++++--------------------- drivers/rawexec/driver_unix_test.go | 69 ++++++++++++++++++ 2 files changed, 94 insertions(+), 79 deletions(-) diff --git a/drivers/rawexec/driver_test.go b/drivers/rawexec/driver_test.go index 186475747..d07583131 100644 --- a/drivers/rawexec/driver_test.go +++ b/drivers/rawexec/driver_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "strconv" "sync" "syscall" @@ -25,7 +26,6 @@ import ( pstructs "github.com/hashicorp/nomad/plugins/shared/structs" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" - "golang.org/x/sys/unix" ) func TestMain(m *testing.M) { @@ -164,73 +164,6 @@ func TestRawExecDriver_StartWait(t *testing.T) { require.NoError(harness.DestroyTask(task.ID, true)) } -func TestRawExecDriver_StartWaitStop(t *testing.T) { - t.Parallel() - require := require.New(t) - - d := NewRawExecDriver(testlog.HCLogger(t)) - harness := dtestutil.NewDriverHarness(t, d) - defer harness.Kill() - - // Disable cgroups so test works without root - config := &Config{NoCgroups: true} - var data []byte - require.NoError(basePlug.MsgPackEncode(&data, config)) - bconfig := &basePlug.Config{PluginConfig: data} - require.NoError(harness.SetConfig(bconfig)) - - task := &drivers.TaskConfig{ - ID: uuid.Generate(), - Name: "test", - } - - taskConfig := map[string]interface{}{} - taskConfig["command"] = testtask.Path() - taskConfig["args"] = []string{"sleep", "100s"} - - encodeDriverHelper(require, task, taskConfig) - - cleanup := harness.MkAllocDir(task, false) - defer cleanup() - - handle, _, err := harness.StartTask(task) - require.NoError(err) - - ch, err := harness.WaitTask(context.Background(), handle.Config.ID) - require.NoError(err) - - require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) - - go func() { - harness.StopTask(task.ID, 2*time.Second, "SIGINT") - }() - - select { - case result := <-ch: - require.Equal(int(unix.SIGINT), result.Signal) - case <-time.After(10 * time.Second): - require.Fail("timeout waiting for task to shutdown") - } - - // Ensure that the task is marked as dead, but account - // for WaitTask() closing channel before internal state is updated - testutil.WaitForResult(func() (bool, error) { - status, err := harness.InspectTask(task.ID) - if err != nil { - return false, fmt.Errorf("inspecting task failed: %v", err) - } - if status.State != drivers.TaskStateExited { - return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) - } - - return true, nil - }, func(err error) { - require.NoError(err) - }) - - require.NoError(harness.DestroyTask(task.ID, true)) -} - func TestRawExecDriver_StartWaitRecoverWaitStop(t *testing.T) { t.Parallel() require := require.New(t) @@ -312,7 +245,6 @@ func TestRawExecDriver_StartWaitRecoverWaitStop(t *testing.T) { wg.Wait() require.NoError(d.DestroyTask(task.ID, false)) require.True(waitDone) - } func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { @@ -483,17 +415,31 @@ func TestRawExecDriver_Exec(t *testing.T) { _, _, err := harness.StartTask(task) require.NoError(err) - // Exec a command that should work - res, err := harness.ExecTask(task.ID, []string{"/usr/bin/stat", "/tmp"}, 1*time.Second) - require.NoError(err) - require.True(res.ExitResult.Successful()) - require.True(len(res.Stdout) > 100) + if runtime.GOOS == "windows" { + // Exec a command that should work + res, err := harness.ExecTask(task.ID, []string{"echo", "hello"}, 1*time.Second) + require.NoError(err) + require.True(res.ExitResult.Successful()) + require.True(len(res.Stdout) > 100) - // Exec a command that should fail - res, err = harness.ExecTask(task.ID, []string{"/usr/bin/stat", "notarealfile123abc"}, 1*time.Second) - require.NoError(err) - require.False(res.ExitResult.Successful()) - require.Contains(string(res.Stdout), "No such file or directory") + // Exec a command that should fail + res, err = harness.ExecTask(task.ID, []string{"stat", "notarealfile123abc"}, 1*time.Second) + require.NoError(err) + require.False(res.ExitResult.Successful()) + require.Contains(string(res.Stdout), "not recognized") + } else { + // Exec a command that should work + res, err := harness.ExecTask(task.ID, []string{"/usr/bin/stat", "/tmp"}, 1*time.Second) + require.NoError(err) + require.True(res.ExitResult.Successful()) + require.True(len(res.Stdout) > 100) + + // Exec a command that should fail + res, err = harness.ExecTask(task.ID, []string{"/usr/bin/stat", "notarealfile123abc"}, 1*time.Second) + require.NoError(err) + require.False(res.ExitResult.Successful()) + require.Contains(string(res.Stdout), "No such file or directory") + } require.NoError(harness.DestroyTask(task.ID, true)) } diff --git a/drivers/rawexec/driver_unix_test.go b/drivers/rawexec/driver_unix_test.go index 3682a6753..568f16ebe 100644 --- a/drivers/rawexec/driver_unix_test.go +++ b/drivers/rawexec/driver_unix_test.go @@ -16,10 +16,12 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/testtask" "github.com/hashicorp/nomad/helper/uuid" + basePlug "github.com/hashicorp/nomad/plugins/base" "github.com/hashicorp/nomad/plugins/drivers" dtestutil "github.com/hashicorp/nomad/plugins/drivers/testutils" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" ) func TestRawExecDriver_User(t *testing.T) { @@ -128,3 +130,70 @@ done return true, nil }, func(err error) { require.NoError(err) }) } + +func TestRawExecDriver_StartWaitStop(t *testing.T) { + t.Parallel() + require := require.New(t) + + d := NewRawExecDriver(testlog.HCLogger(t)) + harness := dtestutil.NewDriverHarness(t, d) + defer harness.Kill() + + // Disable cgroups so test works without root + config := &Config{NoCgroups: true} + var data []byte + require.NoError(basePlug.MsgPackEncode(&data, config)) + bconfig := &basePlug.Config{PluginConfig: data} + require.NoError(harness.SetConfig(bconfig)) + + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "test", + } + + taskConfig := map[string]interface{}{} + taskConfig["command"] = testtask.Path() + taskConfig["args"] = []string{"sleep", "100s"} + + encodeDriverHelper(require, task, taskConfig) + + cleanup := harness.MkAllocDir(task, false) + defer cleanup() + + handle, _, err := harness.StartTask(task) + require.NoError(err) + + ch, err := harness.WaitTask(context.Background(), handle.Config.ID) + require.NoError(err) + + require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) + + go func() { + harness.StopTask(task.ID, 2*time.Second, "SIGINT") + }() + + select { + case result := <-ch: + require.Equal(int(unix.SIGINT), result.Signal) + case <-time.After(10 * time.Second): + require.Fail("timeout waiting for task to shutdown") + } + + // Ensure that the task is marked as dead, but account + // for WaitTask() closing channel before internal state is updated + testutil.WaitForResult(func() (bool, error) { + status, err := harness.InspectTask(task.ID) + if err != nil { + return false, fmt.Errorf("inspecting task failed: %v", err) + } + if status.State != drivers.TaskStateExited { + return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) + } + + return true, nil + }, func(err error) { + require.NoError(err) + }) + + require.NoError(harness.DestroyTask(task.ID, true)) +} From 0ecef707153254fa1f85d4f30f4d7864191012f9 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Fri, 11 Jan 2019 16:49:53 +0000 Subject: [PATCH 23/37] rawexec: Fix Exec test on windows --- drivers/rawexec/driver_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/rawexec/driver_test.go b/drivers/rawexec/driver_test.go index d07583131..31c25e085 100644 --- a/drivers/rawexec/driver_test.go +++ b/drivers/rawexec/driver_test.go @@ -417,13 +417,13 @@ func TestRawExecDriver_Exec(t *testing.T) { if runtime.GOOS == "windows" { // Exec a command that should work - res, err := harness.ExecTask(task.ID, []string{"echo", "hello"}, 1*time.Second) + res, err := harness.ExecTask(task.ID, []string{"cmd.exe", "/c", "echo", "hello"}, 1*time.Second) require.NoError(err) require.True(res.ExitResult.Successful()) - require.True(len(res.Stdout) > 100) + require.Equal(string(res.Stdout), "hello\r\n") // Exec a command that should fail - res, err = harness.ExecTask(task.ID, []string{"stat", "notarealfile123abc"}, 1*time.Second) + res, err = harness.ExecTask(task.ID, []string{"cmd.exe", "/c", "stat", "notarealfile123abc"}, 1*time.Second) require.NoError(err) require.False(res.ExitResult.Successful()) require.Contains(string(res.Stdout), "not recognized") From 8731fe3730be998577fa085d245ad8784157c9a5 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Fri, 11 Jan 2019 17:55:41 +0100 Subject: [PATCH 24/37] drivers/exec: SIGINT unavailable on windows --- drivers/exec/driver_test.go | 60 ------------------------ drivers/exec/driver_unix_test.go | 79 ++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 60 deletions(-) create mode 100644 drivers/exec/driver_unix_test.go diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 16ea4e87d..467a3e79a 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -123,66 +123,6 @@ func TestExecDriver_StartWait(t *testing.T) { require.NoError(harness.DestroyTask(task.ID, true)) } -func TestExecDriver_StartWaitStop(t *testing.T) { - t.Parallel() - require := require.New(t) - ctestutils.ExecCompatible(t) - - d := NewExecDriver(testlog.HCLogger(t)) - harness := dtestutil.NewDriverHarness(t, d) - task := &drivers.TaskConfig{ - ID: uuid.Generate(), - Name: "test", - Resources: testResources, - } - - taskConfig := map[string]interface{}{ - "command": "/bin/sleep", - "args": []string{"600"}, - } - encodeDriverHelper(require, task, taskConfig) - - cleanup := harness.MkAllocDir(task, false) - defer cleanup() - - handle, _, err := harness.StartTask(task) - require.NoError(err) - - ch, err := harness.WaitTask(context.Background(), handle.Config.ID) - require.NoError(err) - - require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) - - go func() { - harness.StopTask(task.ID, 2*time.Second, "SIGINT") - }() - - select { - case result := <-ch: - require.Equal(int(unix.SIGINT), result.Signal) - case <-time.After(10 * time.Second): - require.Fail("timeout waiting for task to shutdown") - } - - // Ensure that the task is marked as dead, but account - // for WaitTask() closing channel before internal state is updated - testutil.WaitForResult(func() (bool, error) { - status, err := harness.InspectTask(task.ID) - if err != nil { - return false, fmt.Errorf("inspecting task failed: %v", err) - } - if status.State != drivers.TaskStateExited { - return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) - } - - return true, nil - }, func(err error) { - require.NoError(err) - }) - - require.NoError(harness.DestroyTask(task.ID, true)) -} - func TestExecDriver_StartWaitStopKill(t *testing.T) { t.Parallel() require := require.New(t) diff --git a/drivers/exec/driver_unix_test.go b/drivers/exec/driver_unix_test.go new file mode 100644 index 000000000..6de52e6cb --- /dev/null +++ b/drivers/exec/driver_unix_test.go @@ -0,0 +1,79 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package exec + +import ( + "context" + "fmt" + "testing" + "time" + + ctestutils "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/helper/uuid" + "github.com/hashicorp/nomad/plugins/drivers" + dtestutil "github.com/hashicorp/nomad/plugins/drivers/testutils" + "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +func TestExecDriver_StartWaitStop(t *testing.T) { + t.Parallel() + require := require.New(t) + ctestutils.ExecCompatible(t) + + d := NewExecDriver(testlog.HCLogger(t)) + harness := dtestutil.NewDriverHarness(t, d) + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "test", + Resources: testResources, + } + + taskConfig := map[string]interface{}{ + "command": "/bin/sleep", + "args": []string{"600"}, + } + encodeDriverHelper(require, task, taskConfig) + + cleanup := harness.MkAllocDir(task, false) + defer cleanup() + + handle, _, err := harness.StartTask(task) + require.NoError(err) + + ch, err := harness.WaitTask(context.Background(), handle.Config.ID) + require.NoError(err) + + require.NoError(harness.WaitUntilStarted(task.ID, 1*time.Second)) + + go func() { + harness.StopTask(task.ID, 2*time.Second, "SIGINT") + }() + + select { + case result := <-ch: + require.Equal(int(unix.SIGINT), result.Signal) + case <-time.After(10 * time.Second): + require.Fail("timeout waiting for task to shutdown") + } + + // Ensure that the task is marked as dead, but account + // for WaitTask() closing channel before internal state is updated + testutil.WaitForResult(func() (bool, error) { + status, err := harness.InspectTask(task.ID) + if err != nil { + return false, fmt.Errorf("inspecting task failed: %v", err) + } + if status.State != drivers.TaskStateExited { + return false, fmt.Errorf("task hasn't exited yet; status: %v", status.State) + } + + return true, nil + }, func(err error) { + require.NoError(err) + }) + + require.NoError(harness.DestroyTask(task.ID, true)) +} From 3f3eb68a27a7fe5523a75cfa3c0f38649b7762a6 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 13:34:24 +0100 Subject: [PATCH 25/37] chore: Fix docker test linting Due to https://github.com/tsenart/deadcode/issues/3 we can't specify these consts on their own. This moves them into the _platform_test.go files to avoid creating a package that only exposes a couple of values. --- drivers/docker/driver_unix_test.go | 52 ++++++++++++++++ ...ting_windows.go => driver_windows_test.go} | 4 +- drivers/docker/testing_unix.go | 61 ------------------- 3 files changed, 54 insertions(+), 63 deletions(-) rename drivers/docker/{testing_windows.go => driver_windows_test.go} (80%) delete mode 100644 drivers/docker/testing_unix.go diff --git a/drivers/docker/driver_unix_test.go b/drivers/docker/driver_unix_test.go index 2ff3024a9..70ceebb47 100644 --- a/drivers/docker/driver_unix_test.go +++ b/drivers/docker/driver_unix_test.go @@ -4,6 +4,9 @@ package docker import ( "fmt" + "io" + "os" + "path/filepath" "sort" "strconv" "strings" @@ -11,6 +14,7 @@ import ( "time" docker "github.com/fsouza/go-dockerclient" + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/plugins/drivers" @@ -641,3 +645,51 @@ func TestDockerDriver_CreateContainerConfig_MountsCombined(t *testing.T) { }) require.EqualValues(t, expectedDevices, foundDevices) } + +func newTaskConfig(variant string, command []string) TaskConfig { + // busyboxImageID is the ID stored in busybox.tar + busyboxImageID := "busybox:1.29.3" + + image := busyboxImageID + loadImage := "busybox.tar" + if variant != "" { + image = fmt.Sprintf("%s-%s", busyboxImageID, variant) + loadImage = fmt.Sprintf("busybox_%s.tar", variant) + } + + return TaskConfig{ + Image: image, + LoadImage: loadImage, + Command: command[0], + Args: command[1:], + } +} + +func copyImage(t *testing.T, taskDir *allocdir.TaskDir, image string) { + dst := filepath.Join(taskDir.LocalDir, image) + copyFile(filepath.Join("./test-resources/docker", image), dst, t) +} + +// copyFile moves an existing file to the destination +func copyFile(src, dst string, t *testing.T) { + in, err := os.Open(src) + if err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + defer in.Close() + out, err := os.Create(dst) + if err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + defer func() { + if err := out.Close(); err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + }() + if _, err = io.Copy(out, in); err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } + if err := out.Sync(); err != nil { + t.Fatalf("copying %v -> %v failed: %v", src, dst, err) + } +} diff --git a/drivers/docker/testing_windows.go b/drivers/docker/driver_windows_test.go similarity index 80% rename from drivers/docker/testing_windows.go rename to drivers/docker/driver_windows_test.go index 2c5c89efc..b163353d9 100644 --- a/drivers/docker/testing_windows.go +++ b/drivers/docker/driver_windows_test.go @@ -3,7 +3,7 @@ package docker import ( - testing "github.com/mitchellh/go-testing-interface" + "testing" "github.com/hashicorp/nomad/client/allocdir" ) @@ -21,5 +21,5 @@ func newTaskConfig(variant string, command []string) TaskConfig { } } -func copyImage(t testing.T, taskDir *allocdir.TaskDir, image string) { +func copyImage(t *testing.T, taskDir *allocdir.TaskDir, image string) { } diff --git a/drivers/docker/testing_unix.go b/drivers/docker/testing_unix.go deleted file mode 100644 index c9189f134..000000000 --- a/drivers/docker/testing_unix.go +++ /dev/null @@ -1,61 +0,0 @@ -// +build darwin dragonfly freebsd linux netbsd openbsd solaris - -package docker - -import ( - "fmt" - "io" - "os" - "path/filepath" - - "github.com/hashicorp/nomad/client/allocdir" - testing "github.com/mitchellh/go-testing-interface" -) - -func newTaskConfig(variant string, command []string) TaskConfig { - // busyboxImageID is the ID stored in busybox.tar - busyboxImageID := "busybox:1.29.3" - - image := busyboxImageID - loadImage := "busybox.tar" - if variant != "" { - image = fmt.Sprintf("%s-%s", busyboxImageID, variant) - loadImage = fmt.Sprintf("busybox_%s.tar", variant) - } - - return TaskConfig{ - Image: image, - LoadImage: loadImage, - Command: command[0], - Args: command[1:], - } -} - -func copyImage(t testing.T, taskDir *allocdir.TaskDir, image string) { - dst := filepath.Join(taskDir.LocalDir, image) - copyFile(filepath.Join("./test-resources/docker", image), dst, t) -} - -// copyFile moves an existing file to the destination -func copyFile(src, dst string, t testing.T) { - in, err := os.Open(src) - if err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - defer in.Close() - out, err := os.Create(dst) - if err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - defer func() { - if err := out.Close(); err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - }() - if _, err = io.Copy(out, in); err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } - if err := out.Sync(); err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } -} From 0bc1dbef569529a43afa6d491c4cdd6f6b46aa33 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 14:16:23 +0100 Subject: [PATCH 26/37] chore: Fix docklog linting --- drivers/docker/docklog/docker_logger_test.go | 13 +++++++++++++ drivers/docker/docklog/testing_unix.go | 9 --------- drivers/docker/docklog/testing_windows.go | 9 --------- 3 files changed, 13 insertions(+), 18 deletions(-) delete mode 100644 drivers/docker/docklog/testing_unix.go delete mode 100644 drivers/docker/docklog/testing_windows.go diff --git a/drivers/docker/docklog/docker_logger_test.go b/drivers/docker/docklog/docker_logger_test.go index 16373c5f9..24e90cb69 100644 --- a/drivers/docker/docklog/docker_logger_test.go +++ b/drivers/docker/docklog/docker_logger_test.go @@ -3,6 +3,7 @@ package docklog import ( "bytes" "fmt" + "runtime" "testing" docker "github.com/fsouza/go-dockerclient" @@ -13,12 +14,24 @@ import ( "golang.org/x/net/context" ) +func testContainerDetails() (string, string, string) { + if runtime.GOOS == "windows" { + return "dantoml/busybox-windows:08012019", + "dantoml/busybox-windows", + "08012019" + } + + return "busybox:1", "busybox", "1" +} + func TestDockerLogger(t *testing.T) { ctu.DockerCompatible(t) t.Parallel() require := require.New(t) + containerImage, containerImageName, containerImageTag := testContainerDetails() + client, err := docker.NewClientFromEnv() if err != nil { t.Skip("docker unavailable:", err) diff --git a/drivers/docker/docklog/testing_unix.go b/drivers/docker/docklog/testing_unix.go deleted file mode 100644 index 42c319d83..000000000 --- a/drivers/docker/docklog/testing_unix.go +++ /dev/null @@ -1,9 +0,0 @@ -// +build darwin dragonfly freebsd linux netbsd openbsd solaris - -package docklog - -var ( - containerImage = "busybox:1" - containerImageName = "busybox" - containerImageTag = "1" -) diff --git a/drivers/docker/docklog/testing_windows.go b/drivers/docker/docklog/testing_windows.go deleted file mode 100644 index 4ec6b73c7..000000000 --- a/drivers/docker/docklog/testing_windows.go +++ /dev/null @@ -1,9 +0,0 @@ -// +build windows - -package docklog - -var ( - containerImage = "dantoml/busybox-windows:08012019" - containerImageName = "dantoml/busybox-windows" - containerImageTag = "08012019" -) From 070558e18d46e9432a5c6754e3b900e8fdefa5c2 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 14:17:41 +0100 Subject: [PATCH 27/37] fixup: Typo in docker test --- drivers/docker/driver_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/docker/driver_windows_test.go b/drivers/docker/driver_windows_test.go index b163353d9..cc2efed8c 100644 --- a/drivers/docker/driver_windows_test.go +++ b/drivers/docker/driver_windows_test.go @@ -9,7 +9,7 @@ import ( ) func newTaskConfig(variant string, command []string) TaskConfig { - // busyboxImageID is an id of an image containting nanoserver windows and + // busyboxImageID is an id of an image containing nanoserver windows and // a busybox exe. // See https://github.com/dantoml/windows/blob/81cff1ed77729d1fa36721abd6cb6efebff2f8ef/docker/busybox/Dockerfile busyboxImageID := "dantoml/busybox-windows:08012019" From 2b3f5f2cb7eaecd8582f82a49bd6033ff77db649 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 14:18:35 +0100 Subject: [PATCH 28/37] chore: goimports exec driver --- drivers/exec/driver_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 467a3e79a..9e35ec966 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -25,7 +25,6 @@ import ( "github.com/hashicorp/nomad/plugins/shared/hclutils" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" - "golang.org/x/sys/unix" ) func TestMain(m *testing.M) { From 8966c20155a849519c59c6d1088426810b581ef1 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 14:58:33 +0100 Subject: [PATCH 29/37] docker: Only run Cleanup test on unix os' --- drivers/docker/driver_test.go | 39 ----------------------------- drivers/docker/driver_unix_test.go | 40 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index e695f9f3a..235caf238 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -1552,45 +1552,6 @@ func TestDockerDriver_Mounts(t *testing.T) { } } -// TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images. -func TestDockerDriver_Cleanup(t *testing.T) { - testutil.DockerCompatible(t) - - // using a small image and an specific point release to avoid accidental conflicts with other tasks - cfg := newTaskConfig("", []string{"sleep", "100"}) - cfg.LoadImage = "" - task := &drivers.TaskConfig{ - ID: uuid.Generate(), - Name: "cleanup_test", - Resources: basicResources, - } - - require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) - - client, driver, handle, cleanup := dockerSetup(t, task) - defer cleanup() - - require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second)) - // Cleanup - require.NoError(t, driver.DestroyTask(task.ID, true)) - - // Ensure image was removed - tu.WaitForResult(func() (bool, error) { - if _, err := client.InspectImage(cfg.Image); err == nil { - return false, fmt.Errorf("image exists but should have been removed. Does another %v container exist?", cfg.Image) - } - - return true, nil - }, func(err error) { - require.NoError(t, err) - }) - - // The image doesn't exist which shouldn't be an error when calling - // Cleanup, so call it again to make sure. - require.NoError(t, driver.Impl().(*Driver).cleanupImage(handle)) - -} - func TestDockerDriver_AuthConfiguration(t *testing.T) { if !tu.IsTravis() { t.Parallel() diff --git a/drivers/docker/driver_unix_test.go b/drivers/docker/driver_unix_test.go index 70ceebb47..f3403d70d 100644 --- a/drivers/docker/driver_unix_test.go +++ b/drivers/docker/driver_unix_test.go @@ -646,6 +646,46 @@ func TestDockerDriver_CreateContainerConfig_MountsCombined(t *testing.T) { require.EqualValues(t, expectedDevices, foundDevices) } +// TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images. +// Doesn't run on windows because it requires an image variant +func TestDockerDriver_Cleanup(t *testing.T) { + testutil.DockerCompatible(t) + + // using a small image and an specific point release to avoid accidental conflicts with other tasks + cfg := newTaskConfig("", []string{"sleep", "100"}) + cfg.Image = "busybox:1.29.2" + cfg.LoadImage = "" + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "cleanup_test", + Resources: basicResources, + } + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + client, driver, handle, cleanup := dockerSetup(t, task) + defer cleanup() + + require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second)) + // Cleanup + require.NoError(t, driver.DestroyTask(task.ID, true)) + + // Ensure image was removed + tu.WaitForResult(func() (bool, error) { + if _, err := client.InspectImage(cfg.Image); err == nil { + return false, fmt.Errorf("image exists but should have been removed. Does another %v container exist?", cfg.Image) + } + + return true, nil + }, func(err error) { + require.NoError(t, err) + }) + + // The image doesn't exist which shouldn't be an error when calling + // Cleanup, so call it again to make sure. + require.NoError(t, driver.Impl().(*Driver).cleanupImage(handle)) +} + func newTaskConfig(variant string, command []string) TaskConfig { // busyboxImageID is the ID stored in busybox.tar busyboxImageID := "busybox:1.29.3" From db76db4b18b139eaff3606d3990a78197701e376 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 15:04:16 +0100 Subject: [PATCH 30/37] allocwatcher: Stat_t is unavailable on win --- client/allocwatcher/alloc_watcher_test.go | 145 ---------------- .../allocwatcher/alloc_watcher_unix_test.go | 161 ++++++++++++++++++ 2 files changed, 161 insertions(+), 145 deletions(-) create mode 100644 client/allocwatcher/alloc_watcher_unix_test.go diff --git a/client/allocwatcher/alloc_watcher_test.go b/client/allocwatcher/alloc_watcher_test.go index 2f34894a2..ea92525a4 100644 --- a/client/allocwatcher/alloc_watcher_test.go +++ b/client/allocwatcher/alloc_watcher_test.go @@ -5,19 +5,16 @@ import ( "bytes" "context" "fmt" - "io" "io/ioutil" "os" "path/filepath" "strings" - "syscall" "testing" "time" hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad/client/allocdir" cstructs "github.com/hashicorp/nomad/client/structs" - ctestutil "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -201,148 +198,6 @@ func TestPrevAlloc_LocalPrevAlloc_Terminated(t *testing.T) { require.NoError(t, waiter.Wait(ctx)) } -// TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir -// works. -func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) { - ctestutil.RequireRoot(t) - t.Parallel() - dir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("err: %v", err) - } - defer os.RemoveAll(dir) - - // Create foo/ - fooDir := filepath.Join(dir, "foo") - if err := os.Mkdir(fooDir, 0777); err != nil { - t.Fatalf("err: %v", err) - } - - // Change ownership of foo/ to test #3702 (any non-root user is fine) - const uid, gid = 1, 1 - if err := os.Chown(fooDir, uid, gid); err != nil { - t.Fatalf("err : %v", err) - } - - dirInfo, err := os.Stat(fooDir) - if err != nil { - t.Fatalf("err: %v", err) - } - - // Create foo/bar - f, err := os.Create(filepath.Join(fooDir, "bar")) - if err != nil { - t.Fatalf("err: %v", err) - } - if _, err := f.WriteString("123"); err != nil { - t.Fatalf("err: %v", err) - } - if err := f.Chmod(0644); err != nil { - t.Fatalf("err: %v", err) - } - fInfo, err := f.Stat() - if err != nil { - t.Fatalf("err: %v", err) - } - f.Close() - - // Create foo/baz -> bar symlink - if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil { - t.Fatalf("err: %v", err) - } - linkInfo, err := os.Lstat(filepath.Join(dir, "foo", "baz")) - if err != nil { - t.Fatalf("err: %v", err) - } - - buf := new(bytes.Buffer) - tw := tar.NewWriter(buf) - - walkFn := func(path string, fileInfo os.FileInfo, err error) error { - // Include the path of the file name relative to the alloc dir - // so that we can put the files in the right directories - link := "" - if fileInfo.Mode()&os.ModeSymlink != 0 { - target, err := os.Readlink(path) - if err != nil { - return fmt.Errorf("error reading symlink: %v", err) - } - link = target - } - hdr, err := tar.FileInfoHeader(fileInfo, link) - if err != nil { - return fmt.Errorf("error creating file header: %v", err) - } - hdr.Name = fileInfo.Name() - tw.WriteHeader(hdr) - - // If it's a directory or symlink we just write the header into the tar - if fileInfo.IsDir() || (fileInfo.Mode()&os.ModeSymlink != 0) { - return nil - } - - // Write the file into the archive - file, err := os.Open(path) - if err != nil { - return err - } - defer file.Close() - - if _, err := io.Copy(tw, file); err != nil { - return err - } - - return nil - } - - if err := filepath.Walk(dir, walkFn); err != nil { - t.Fatalf("err: %v", err) - } - tw.Close() - - dir1, err := ioutil.TempDir("", "nomadtest-") - if err != nil { - t.Fatalf("err: %v", err) - } - defer os.RemoveAll(dir1) - - rc := ioutil.NopCloser(buf) - prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)} - if err := prevAlloc.streamAllocDir(context.Background(), rc, dir1); err != nil { - t.Fatalf("err: %v", err) - } - - // Ensure foo is present - fi, err := os.Stat(filepath.Join(dir1, "foo")) - if err != nil { - t.Fatalf("err: %v", err) - } - if fi.Mode() != dirInfo.Mode() { - t.Fatalf("mode: %v", fi.Mode()) - } - stat := fi.Sys().(*syscall.Stat_t) - if stat.Uid != uid || stat.Gid != gid { - t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d", - uid, gid, stat.Uid, stat.Gid) - } - - fi1, err := os.Stat(filepath.Join(dir1, "bar")) - if err != nil { - t.Fatalf("err: %v", err) - } - if fi1.Mode() != fInfo.Mode() { - t.Fatalf("mode: %v", fi1.Mode()) - } - - fi2, err := os.Lstat(filepath.Join(dir1, "baz")) - if err != nil { - t.Fatalf("err: %v", err) - } - if fi2.Mode() != linkInfo.Mode() { - t.Fatalf("mode: %v", fi2.Mode()) - } -} - // TestPrevAlloc_StreamAllocDir_Error asserts that errors encountered while // streaming a tar cause the migration to be cancelled and no files are written // (migrations are atomic). diff --git a/client/allocwatcher/alloc_watcher_unix_test.go b/client/allocwatcher/alloc_watcher_unix_test.go new file mode 100644 index 000000000..21f95cdf9 --- /dev/null +++ b/client/allocwatcher/alloc_watcher_unix_test.go @@ -0,0 +1,161 @@ +// +build !windows + +package allocwatcher + +import ( + "archive/tar" + "bytes" + "context" + "fmt" + "io" + "io/ioutil" + "os" + "path/filepath" + "syscall" + "testing" + + ctestutil "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/helper/testlog" +) + +// TestPrevAlloc_StreamAllocDir_Ok asserts that streaming a tar to an alloc dir +// works. +func TestPrevAlloc_StreamAllocDir_Ok(t *testing.T) { + ctestutil.RequireRoot(t) + t.Parallel() + dir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("err: %v", err) + } + defer os.RemoveAll(dir) + + // Create foo/ + fooDir := filepath.Join(dir, "foo") + if err := os.Mkdir(fooDir, 0777); err != nil { + t.Fatalf("err: %v", err) + } + + // Change ownership of foo/ to test #3702 (any non-root user is fine) + const uid, gid = 1, 1 + if err := os.Chown(fooDir, uid, gid); err != nil { + t.Fatalf("err : %v", err) + } + + dirInfo, err := os.Stat(fooDir) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Create foo/bar + f, err := os.Create(filepath.Join(fooDir, "bar")) + if err != nil { + t.Fatalf("err: %v", err) + } + if _, err := f.WriteString("123"); err != nil { + t.Fatalf("err: %v", err) + } + if err := f.Chmod(0644); err != nil { + t.Fatalf("err: %v", err) + } + fInfo, err := f.Stat() + if err != nil { + t.Fatalf("err: %v", err) + } + f.Close() + + // Create foo/baz -> bar symlink + if err := os.Symlink("bar", filepath.Join(dir, "foo", "baz")); err != nil { + t.Fatalf("err: %v", err) + } + linkInfo, err := os.Lstat(filepath.Join(dir, "foo", "baz")) + if err != nil { + t.Fatalf("err: %v", err) + } + + buf := new(bytes.Buffer) + tw := tar.NewWriter(buf) + + walkFn := func(path string, fileInfo os.FileInfo, err error) error { + // Include the path of the file name relative to the alloc dir + // so that we can put the files in the right directories + link := "" + if fileInfo.Mode()&os.ModeSymlink != 0 { + target, err := os.Readlink(path) + if err != nil { + return fmt.Errorf("error reading symlink: %v", err) + } + link = target + } + hdr, err := tar.FileInfoHeader(fileInfo, link) + if err != nil { + return fmt.Errorf("error creating file header: %v", err) + } + hdr.Name = fileInfo.Name() + tw.WriteHeader(hdr) + + // If it's a directory or symlink we just write the header into the tar + if fileInfo.IsDir() || (fileInfo.Mode()&os.ModeSymlink != 0) { + return nil + } + + // Write the file into the archive + file, err := os.Open(path) + if err != nil { + return err + } + defer file.Close() + + if _, err := io.Copy(tw, file); err != nil { + return err + } + + return nil + } + + if err := filepath.Walk(dir, walkFn); err != nil { + t.Fatalf("err: %v", err) + } + tw.Close() + + dir1, err := ioutil.TempDir("", "nomadtest-") + if err != nil { + t.Fatalf("err: %v", err) + } + defer os.RemoveAll(dir1) + + rc := ioutil.NopCloser(buf) + prevAlloc := &remotePrevAlloc{logger: testlog.HCLogger(t)} + if err := prevAlloc.streamAllocDir(context.Background(), rc, dir1); err != nil { + t.Fatalf("err: %v", err) + } + + // Ensure foo is present + fi, err := os.Stat(filepath.Join(dir1, "foo")) + if err != nil { + t.Fatalf("err: %v", err) + } + if fi.Mode() != dirInfo.Mode() { + t.Fatalf("mode: %v", fi.Mode()) + } + stat := fi.Sys().(*syscall.Stat_t) + if stat.Uid != uid || stat.Gid != gid { + t.Fatalf("foo/ has incorrect ownership: expected %d:%d found %d:%d", + uid, gid, stat.Uid, stat.Gid) + } + + fi1, err := os.Stat(filepath.Join(dir1, "bar")) + if err != nil { + t.Fatalf("err: %v", err) + } + if fi1.Mode() != fInfo.Mode() { + t.Fatalf("mode: %v", fi1.Mode()) + } + + fi2, err := os.Lstat(filepath.Join(dir1, "baz")) + if err != nil { + t.Fatalf("err: %v", err) + } + if fi2.Mode() != linkInfo.Mode() { + t.Fatalf("mode: %v", fi2.Mode()) + } +} From 06ea0b5377f2da28f6358c3aa0ff462959660b17 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Mon, 14 Jan 2019 17:27:07 +0100 Subject: [PATCH 31/37] appveyor: disable tests on windows again --- appveyor.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 737aab913..7494be2a5 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -33,10 +33,10 @@ build_script: set PATH=%GOPATH%/bin;%PATH% mkdir -p $GOPATH\bin go build -o $GOPATH\bin\nomad -test_script: - - cmd: gotestsum -f short-verbose --junitfile results.xml -on_finish: - - ps: | - Push-AppveyorArtifact (Resolve-Path .\results.xml) - $wc = New-Object 'System.Net.WebClient' - $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\results.xml)) +# test_script: +# - cmd: gotestsum -f short-verbose --junitfile results.xml +# on_finish: +# - ps: | +# Push-AppveyorArtifact (Resolve-Path .\results.xml) +# $wc = New-Object 'System.Net.WebClient' +# $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\results.xml)) From fe8c60e1570e359761d34699cf85b2e7a9c060f0 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 14 Jan 2019 20:03:42 +0100 Subject: [PATCH 32/37] chore: Stylistic cleanup Co-Authored-By: dantoml --- plugins/shared/loader/init.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index de94dfc47..127fea4c5 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -256,7 +256,7 @@ func (l *PluginLoader) scan() ([]os.FileInfo, error) { // actually validate the executability of a file, so here we skip executability checks // for windows systems. if runtime.GOOS != "windows" { - if s.Mode().Perm()&0111 == 0 { + if runtime.GOOS != "windows" && s.Mode().Perm()&0111 == 0 { l.logger.Debug("skipping un-executable file in plugin folder", "file", f) continue } From 9e4e2d812d4d0982b87c66afd7cbc364d4d733b9 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 15 Jan 2019 22:01:59 +0100 Subject: [PATCH 33/37] Update helper/testtask/testtask_windows.go Co-Authored-By: dantoml --- helper/testtask/testtask_windows.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helper/testtask/testtask_windows.go b/helper/testtask/testtask_windows.go index 64717665f..a532b418e 100644 --- a/helper/testtask/testtask_windows.go +++ b/helper/testtask/testtask_windows.go @@ -8,6 +8,7 @@ import ( ) func executeProcessGroup(gid string) { - fmt.Fprintf(os.Stderr, "process groups are not supported on windows\n") + fmt.Fprintf(os.Stderr, "TODO: implement process groups are on windows\n") + fmt.Fprintf(os.Stderr, "TODO: see https://github.com/hashicorp/nomad/blob/109c5ef650206fc62334d202002cda92ceb67399/drivers/shared/executor/executor_windows.go#L9-L17\n") os.Exit(1) } From f40d243cbd5230850466bed47a573bda3064b8a5 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 15 Jan 2019 22:02:41 +0100 Subject: [PATCH 34/37] Update testutil/vault.go Co-Authored-By: dantoml --- testutil/vault.go | 1 + 1 file changed, 1 insertion(+) diff --git a/testutil/vault.go b/testutil/vault.go index 6719b6610..ea377abad 100644 --- a/testutil/vault.go +++ b/testutil/vault.go @@ -171,6 +171,7 @@ func (tv *TestVault) Start() error { tv.waitCh = make(chan error, 1) go func() { + // Must call Start and Wait in the same goroutine on Windows #5174 if err := tv.cmd.Start(); err != nil { tv.waitCh <- err return From 3287c3f019a616af5fa02f8dc4ca4e0dd6a2c451 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 16 Jan 2019 15:06:39 +0100 Subject: [PATCH 35/37] chore: General Cleanup --- drivers/docker/docklog/docker_logger_test.go | 2 +- drivers/docker/driver_windows_test.go | 1 + plugins/shared/loader/init.go | 8 +++----- plugins/shared/loader/loader_test.go | 2 +- testutil/vault.go | 3 ++- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/docker/docklog/docker_logger_test.go b/drivers/docker/docklog/docker_logger_test.go index 24e90cb69..22c3dea8c 100644 --- a/drivers/docker/docklog/docker_logger_test.go +++ b/drivers/docker/docklog/docker_logger_test.go @@ -14,7 +14,7 @@ import ( "golang.org/x/net/context" ) -func testContainerDetails() (string, string, string) { +func testContainerDetails() (image string, imageName string, imageTag string) { if runtime.GOOS == "windows" { return "dantoml/busybox-windows:08012019", "dantoml/busybox-windows", diff --git a/drivers/docker/driver_windows_test.go b/drivers/docker/driver_windows_test.go index cc2efed8c..ffec23a5f 100644 --- a/drivers/docker/driver_windows_test.go +++ b/drivers/docker/driver_windows_test.go @@ -21,5 +21,6 @@ func newTaskConfig(variant string, command []string) TaskConfig { } } +// No-op on windows because we don't load images. func copyImage(t *testing.T, taskDir *allocdir.TaskDir, image string) { } diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 127fea4c5..2795656a4 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -255,11 +255,9 @@ func (l *PluginLoader) scan() ([]os.FileInfo, error) { // extension and the file begins with MZ, however, there is no easy way for us to // actually validate the executability of a file, so here we skip executability checks // for windows systems. - if runtime.GOOS != "windows" { - if runtime.GOOS != "windows" && s.Mode().Perm()&0111 == 0 { - l.logger.Debug("skipping un-executable file in plugin folder", "file", f) - continue - } + if runtime.GOOS != "windows" && s.Mode().Perm()&0111 == 0 { + l.logger.Debug("skipping un-executable file in plugin folder", "file", f) + continue } plugins = append(plugins, s) } diff --git a/plugins/shared/loader/loader_test.go b/plugins/shared/loader/loader_test.go index 2fd9004d1..0b759de51 100644 --- a/plugins/shared/loader/loader_test.go +++ b/plugins/shared/loader/loader_test.go @@ -61,7 +61,7 @@ func newHarness(t *testing.T, plugins []string) *harness { exeSuffix = ".exe" } for _, p := range plugins { - dest := strings.Join([]string{filepath.Join(h.tmpDir, p), exeSuffix}, "") + dest := filepath.Join(h.tmpDir, p) + exeSuffix if err := copyFile(selfExe, dest); err != nil { t.Fatalf("failed to copy file: %v", err) } diff --git a/testutil/vault.go b/testutil/vault.go index ea377abad..7083f01d6 100644 --- a/testutil/vault.go +++ b/testutil/vault.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/nomad/nomad/structs/config" vapi "github.com/hashicorp/vault/api" testing "github.com/mitchellh/go-testing-interface" + "github.com/stretchr/testify/require" ) // TestVault is a test helper. It uses a fork/exec model to create a test Vault @@ -205,7 +206,7 @@ func (tv *TestVault) Stop() { case <-tv.waitCh: return case <-time.After(1 * time.Second): - tv.t.Error("Timed out waiting for vault to terminate") + require.Fail(tv.t, "Timed out waiting for vault to terminate") } } } From 8ffd94ca557a3461de0c1c6dfe75ddb30342cf7b Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Wed, 16 Jan 2019 15:22:03 +0100 Subject: [PATCH 36/37] plugins: Require an exe extension on windows --- plugins/shared/loader/filter_unix.go | 10 ++++++++++ plugins/shared/loader/filter_windows.go | 15 +++++++++++++++ plugins/shared/loader/init.go | 7 +------ 3 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 plugins/shared/loader/filter_unix.go create mode 100644 plugins/shared/loader/filter_windows.go diff --git a/plugins/shared/loader/filter_unix.go b/plugins/shared/loader/filter_unix.go new file mode 100644 index 000000000..fefea7cb2 --- /dev/null +++ b/plugins/shared/loader/filter_unix.go @@ -0,0 +1,10 @@ +// +build !windows + +package loader + +import "os" + +// executable Checks to see if the file is executable by anyone. +func executable(path string, f os.FileInfo) bool { + return f.Mode().Perm()&0111 != 0 +} diff --git a/plugins/shared/loader/filter_windows.go b/plugins/shared/loader/filter_windows.go new file mode 100644 index 000000000..a3423c548 --- /dev/null +++ b/plugins/shared/loader/filter_windows.go @@ -0,0 +1,15 @@ +// +build windows + +package loader + +import ( + "os" + "path/filepath" +) + +// On windows, an executable can be any file with any extension. To avoid +// introspecting the file, here we skip executability checks on windows systems +// and simply check for the convention of an `exe` extension. +func executable(path string, s os.FileInfo) bool { + return filepath.Ext(path) == "exe" +} diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 2795656a4..f61f47344 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -5,7 +5,6 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "sort" multierror "github.com/hashicorp/go-multierror" @@ -251,11 +250,7 @@ func (l *PluginLoader) scan() ([]os.FileInfo, error) { continue } - // Check if it is executable by anyone. On windows, an executable is any file with any - // extension and the file begins with MZ, however, there is no easy way for us to - // actually validate the executability of a file, so here we skip executability checks - // for windows systems. - if runtime.GOOS != "windows" && s.Mode().Perm()&0111 == 0 { + if !executable(f, s) { l.logger.Debug("skipping un-executable file in plugin folder", "file", f) continue } From 5637d816420bdedd4007d40e558e797ad08467f7 Mon Sep 17 00:00:00 2001 From: Danielle Tomlinson Date: Thu, 17 Jan 2019 18:44:27 +0100 Subject: [PATCH 37/37] docker: Fix missing import --- drivers/docker/driver_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index 235caf238..47c0cfc70 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -27,6 +27,7 @@ import ( dtestutil "github.com/hashicorp/nomad/plugins/drivers/testutils" "github.com/hashicorp/nomad/plugins/shared/loader" tu "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" )