From c3eaa0f4c86108737c5eddeca9d3d664d684755f Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 8 Jan 2019 16:21:49 -0500 Subject: [PATCH 1/4] tests: enable and fix tests requiring mock driver --- client/alloc_endpoint_test.go | 1 - client/alloc_watcher_e2e_test.go | 15 +++-- client/client_test.go | 2 - client/fs_endpoint_test.go | 100 ++++++++++--------------------- testutil/wait.go | 11 +++- 5 files changed, 49 insertions(+), 80 deletions(-) diff --git a/client/alloc_endpoint_test.go b/client/alloc_endpoint_test.go index f7ec3dc6c..66502a4c5 100644 --- a/client/alloc_endpoint_test.go +++ b/client/alloc_endpoint_test.go @@ -77,7 +77,6 @@ func TestAllocations_GarbageCollectAll_ACL(t *testing.T) { } func TestAllocations_GarbageCollect(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) client, cleanup := TestClient(t, func(c *config.Config) { diff --git a/client/alloc_watcher_e2e_test.go b/client/alloc_watcher_e2e_test.go index efc1aee71..5c8df9bf0 100644 --- a/client/alloc_watcher_e2e_test.go +++ b/client/alloc_watcher_e2e_test.go @@ -19,7 +19,6 @@ import ( // TestPrevAlloc_StreamAllocDir_TLS asserts ephemeral disk migrations still // work when TLS is enabled. func TestPrevAlloc_StreamAllocDir_TLS(t *testing.T) { - t.Skip("missing mock driver plugin implementation") const ( caFn = "../helper/tlsutil/testdata/global-ca.pem" serverCertFn = "../helper/tlsutil/testdata/global-server.pem" @@ -65,8 +64,13 @@ func TestPrevAlloc_StreamAllocDir_TLS(t *testing.T) { defer client2.Shutdown() job := mock.Job() - job.Constraints[0].LTarget = "${node.unique.name}" - job.Constraints[0].RTarget = "client1" + job.Constraints = []*structs.Constraint{ + { + LTarget: "${node.unique.name}", + RTarget: "client1", + Operand: "=", + }, + } job.TaskGroups[0].Count = 1 job.TaskGroups[0].EphemeralDisk.Sticky = true job.TaskGroups[0].EphemeralDisk.Migrate = true @@ -100,7 +104,10 @@ func TestPrevAlloc_StreamAllocDir_TLS(t *testing.T) { // Migrate alloc to other node job.Constraints[0].RTarget = "client2" - testutil.WaitForRunning(t, server.RPC, job.Copy()) + + // Only register job - don't wait for running - since previous completed allocs + // will interfere + testutil.RegisterJob(t, server.RPC, job.Copy()) // Wait for new alloc to be running var newAlloc *structs.AllocListStub diff --git a/client/client_test.go b/client/client_test.go index 47142fafd..e15fcc7f2 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -655,7 +655,6 @@ func TestClient_Init(t *testing.T) { } func TestClient_BlockedAllocations(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() s1, _ := testServer(t, nil) defer s1.Shutdown() @@ -691,7 +690,6 @@ func TestClient_BlockedAllocations(t *testing.T) { "run_for": "100s", "exit_code": 0, "exit_signal": 0, - "exit_err": "", } state.UpsertJobSummary(99, mock.JobSummary(alloc.JobID)) diff --git a/client/fs_endpoint_test.go b/client/fs_endpoint_test.go index 365b59df2..b2ca33f23 100644 --- a/client/fs_endpoint_test.go +++ b/client/fs_endpoint_test.go @@ -74,48 +74,31 @@ func TestFS_Stat_NoAlloc(t *testing.T) { } func TestFS_Stat(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - // Start a client - c, cleanup := TestClient(t, nil) + // Start a server and client + s := nomad.TestServer(t, nil) + defer s.Shutdown() + testutil.WaitForLeader(t, s.RPC) + + c, cleanup := TestClient(t, func(c *config.Config) { + c.Servers = []string{s.GetConfig().RPCAddr.String()} + }) defer cleanup() // Create and add an alloc - a := mock.Alloc() - task := a.Job.TaskGroups[0].Tasks[0] - task.Driver = "mock_driver" - task.Config = map[string]interface{}{ + job := mock.BatchJob() + job.TaskGroups[0].Count = 1 + job.TaskGroups[0].Tasks[0].Config = map[string]interface{}{ "run_for": "500ms", } - c.addAlloc(a, "") - - // Wait for the client to start it - testutil.WaitForResult(func() (bool, error) { - ar, ok := c.allocs[a.ID] - if !ok { - return false, fmt.Errorf("alloc doesn't exist") - } - - alloc := ar.Alloc() - running := false - for _, s := range alloc.TaskStates { - if s.State == structs.TaskStateRunning { - running = true - } else { - running = false - } - } - - return running, fmt.Errorf("tasks not running") - }, func(err error) { - t.Fatal(err) - }) + // Wait for alloc to be running + alloc := testutil.WaitForRunning(t, s.RPC, job)[0] // Make the request with bad allocation id req := &cstructs.FsStatRequest{ - AllocID: a.ID, + AllocID: alloc.ID, Path: "/", QueryOptions: structs.QueryOptions{Region: "global"}, } @@ -215,48 +198,31 @@ func TestFS_List_NoAlloc(t *testing.T) { } func TestFS_List(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - // Start a client - c, cleanup := TestClient(t, nil) + // Start a server and client + s := nomad.TestServer(t, nil) + defer s.Shutdown() + testutil.WaitForLeader(t, s.RPC) + + c, cleanup := TestClient(t, func(c *config.Config) { + c.Servers = []string{s.GetConfig().RPCAddr.String()} + }) defer cleanup() // Create and add an alloc - a := mock.Alloc() - task := a.Job.TaskGroups[0].Tasks[0] - task.Driver = "mock_driver" - task.Config = map[string]interface{}{ + job := mock.BatchJob() + job.TaskGroups[0].Count = 1 + job.TaskGroups[0].Tasks[0].Config = map[string]interface{}{ "run_for": "500ms", } - c.addAlloc(a, "") - - // Wait for the client to start it - testutil.WaitForResult(func() (bool, error) { - ar, ok := c.allocs[a.ID] - if !ok { - return false, fmt.Errorf("alloc doesn't exist") - } - - alloc := ar.Alloc() - running := false - for _, s := range alloc.TaskStates { - if s.State == structs.TaskStateRunning { - running = true - } else { - running = false - } - } - - return running, fmt.Errorf("tasks not running") - }, func(err error) { - t.Fatal(err) - }) + // Wait for alloc to be running + alloc := testutil.WaitForRunning(t, s.RPC, job)[0] // Make the request req := &cstructs.FsListRequest{ - AllocID: a.ID, + AllocID: alloc.ID, Path: "/", QueryOptions: structs.QueryOptions{Region: "global"}, } @@ -529,7 +495,6 @@ func TestFS_Stream_ACL(t *testing.T) { } func TestFS_Stream(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) @@ -557,7 +522,7 @@ func TestFS_Stream(t *testing.T) { // Make the request req := &cstructs.FsStreamRequest{ AllocID: alloc.ID, - Path: "alloc/logs/worker.stdout.0", + Path: "alloc/logs/web.stdout.0", PlainText: true, QueryOptions: structs.QueryOptions{Region: "global"}, } @@ -640,7 +605,6 @@ func (r *ReadWriteCloseChecker) Close() error { } func TestFS_Stream_Follow(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) @@ -672,7 +636,7 @@ func TestFS_Stream_Follow(t *testing.T) { // Make the request req := &cstructs.FsStreamRequest{ AllocID: alloc.ID, - Path: "alloc/logs/worker.stdout.0", + Path: "alloc/logs/web.stdout.0", PlainText: true, Follow: true, QueryOptions: structs.QueryOptions{Region: "global"}, @@ -738,7 +702,6 @@ OUTER: } func TestFS_Stream_Limit(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) @@ -911,7 +874,6 @@ OUTER: // TestFS_Logs_TaskPending asserts that trying to stream logs for tasks which // have not started returns a 404 error. func TestFS_Logs_TaskPending(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) @@ -1140,7 +1102,6 @@ func TestFS_Logs_ACL(t *testing.T) { } func TestFS_Logs(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) @@ -1242,7 +1203,6 @@ OUTER: } func TestFS_Logs_Follow(t *testing.T) { - t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) diff --git a/testutil/wait.go b/testutil/wait.go index 3e0ea166f..c85333552 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/go-testing-interface" + "github.com/stretchr/testify/require" ) const ( @@ -87,8 +88,7 @@ func WaitForLeader(t testing.T, rpc rpcFn) { }) } -// WaitForRunning runs a job and blocks until all allocs are out of pending. -func WaitForRunning(t testing.T, rpc rpcFn, job *structs.Job) []*structs.AllocListStub { +func RegisterJob(t testing.T, rpc rpcFn, job *structs.Job) { WaitForResult(func() (bool, error) { args := &structs.JobRegisterRequest{} args.Job = job @@ -101,6 +101,11 @@ func WaitForRunning(t testing.T, rpc rpcFn, job *structs.Job) []*structs.AllocLi }) t.Logf("Job %q registered", job.ID) +} + +// WaitForRunning runs a job and blocks until all allocs are out of pending. +func WaitForRunning(t testing.T, rpc rpcFn, job *structs.Job) []*structs.AllocListStub { + RegisterJob(t, rpc, job) var resp structs.JobAllocationsResponse @@ -126,7 +131,7 @@ func WaitForRunning(t testing.T, rpc rpcFn, job *structs.Job) []*structs.AllocLi return true, nil }, func(err error) { - t.Fatalf("job not running: %v", err) + require.NoError(t, err) }) return resp.Allocations From 9740443703320979d28343acce16fd4f7a0f6a98 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 8 Jan 2019 16:17:47 -0500 Subject: [PATCH 2/4] tests: ignore _JAVA_OPTIONS line ignore _JAVA_OPTIONS line in `java -version`, as it's relevant. --- drivers/java/utils.go | 4 ++++ drivers/java/utils_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/drivers/java/utils.go b/drivers/java/utils.go index 3bf9c16f8..35bfa73c3 100644 --- a/drivers/java/utils.go +++ b/drivers/java/utils.go @@ -29,6 +29,10 @@ func parseJavaVersionOutput(infoString string) (version, runtime, vm string, err infoString = strings.TrimSpace(infoString) lines := strings.Split(infoString, "\n") + if strings.Contains(lines[0], "Picked up _JAVA_OPTIONS") { + lines = lines[1:] + } + if len(lines) != 3 { return "", "", "", fmt.Errorf("unexpected java version info output, expected 3 lines but got: %v", infoString) } diff --git a/drivers/java/utils_test.go b/drivers/java/utils_test.go index 4c4165ecc..f29d07474 100644 --- a/drivers/java/utils_test.go +++ b/drivers/java/utils_test.go @@ -37,6 +37,16 @@ func TestDriver_parseJavaVersionOutput(t *testing.T) { "OpenJDK Runtime Environment 18.9 (build 11.0.1+13)", "OpenJDK 64-Bit Server VM 18.9 (build 11.0.1+13, mixed mode)", }, + { + "OpenJDK", + `Picked up _JAVA_OPTIONS: -Xmx2048m -Xms512m + openjdk version "11.0.1" 2018-10-16 + OpenJDK Runtime Environment 18.9 (build 11.0.1+13) + OpenJDK 64-Bit Server VM 18.9 (build 11.0.1+13, mixed mode)`, + "11.0.1", + "OpenJDK Runtime Environment 18.9 (build 11.0.1+13)", + "OpenJDK 64-Bit Server VM 18.9 (build 11.0.1+13, mixed mode)", + }, { "IcedTea", `java version "1.6.0_36" From 614f63f40da1f9ae3f95193cb616e08be4d971ee Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Wed, 9 Jan 2019 21:29:53 -0500 Subject: [PATCH 3/4] drivers/java: use libcontainer executor on java linux --- drivers/java/driver.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/java/driver.go b/drivers/java/driver.go index 23c09d627..9cd42d3eb 100644 --- a/drivers/java/driver.go +++ b/drivers/java/driver.go @@ -318,8 +318,9 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive pluginLogFile := filepath.Join(cfg.TaskDir().Dir, "executor.out") executorConfig := &executor.ExecutorConfig{ - LogFile: pluginLogFile, - LogLevel: "debug", + LogFile: pluginLogFile, + LogLevel: "debug", + FSIsolation: capabilities.FSIsolation == drivers.FSIsolationChroot, } exec, pluginClient, err := executor.CreateExecutor(os.Stderr, hclog.Debug, d.nomadConfig, executorConfig) From b7d421a1492db43d7794d2d3f18db10cf0c9415a Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Thu, 10 Jan 2019 15:36:57 -0500 Subject: [PATCH 4/4] tests: WaitForRunning checks for pending only WaitForRunning risks a race condition where the allocation succeeds and completes before WaitForRunning is called (or while it is running). Here, I made the behavior match the function documentation. I considered making it stricter, but callers need to account for allocation terminating immediately after WaitForRunning terminates anyway. --- testutil/wait.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/wait.go b/testutil/wait.go index c85333552..7c947a3c6 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -123,7 +123,7 @@ func WaitForRunning(t testing.T, rpc rpcFn, job *structs.Job) []*structs.AllocLi } for _, alloc := range resp.Allocations { - if alloc.ClientStatus != structs.AllocClientStatusRunning { + if alloc.ClientStatus == structs.AllocClientStatusPending { return false, fmt.Errorf("alloc not running: id=%v tg=%v status=%v", alloc.ID, alloc.TaskGroup, alloc.ClientStatus) }