From 21d78be96136a7f2dbb7dadcee3293459f7c4781 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Mon, 15 Oct 2018 20:07:16 -0700 Subject: [PATCH] tests: explicitly cleanup after clients --- client/acl_test.go | 16 ++--- client/alloc_endpoint_test.go | 21 +++--- client/allocrunner/alloc_runner_test.go | 40 +++++------ client/client_stats_endpoint_test.go | 7 +- client/client_test.go | 81 ++++++++++++----------- client/config/testing.go | 36 +++++++++- client/fingerprint_manager_test.go | 76 ++++++++------------- client/fs_endpoint_test.go | 88 ++++++++++++------------- client/rpc_test.go | 8 +-- client/testing.go | 20 ++++-- 10 files changed, 212 insertions(+), 181 deletions(-) diff --git a/client/acl_test.go b/client/acl_test.go index bb21fcb8b..12493e301 100644 --- a/client/acl_test.go +++ b/client/acl_test.go @@ -17,11 +17,11 @@ func TestClient_ACL_resolveTokenValue(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 c.ACLEnabled = true }) - defer c1.Shutdown() + defer cleanup() // Create a policy / token policy := mock.ACLPolicy() @@ -66,11 +66,11 @@ func TestClient_ACL_resolvePolicies(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 c.ACLEnabled = true }) - defer c1.Shutdown() + defer cleanup() // Create a policy / token policy := mock.ACLPolicy() @@ -106,10 +106,10 @@ func TestClient_ACL_ResolveToken_Disabled(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() // Should always get nil when disabled aclObj, err := c1.ResolveToken("blah") @@ -122,11 +122,11 @@ func TestClient_ACL_ResolveToken(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 c.ACLEnabled = true }) - defer c1.Shutdown() + defer cleanup() // Create a policy / token policy := mock.ACLPolicy() diff --git a/client/alloc_endpoint_test.go b/client/alloc_endpoint_test.go index b616a7c9a..f7ec3dc6c 100644 --- a/client/alloc_endpoint_test.go +++ b/client/alloc_endpoint_test.go @@ -16,7 +16,8 @@ import ( func TestAllocations_GarbageCollectAll(t *testing.T) { t.Parallel() require := require.New(t) - client := TestClient(t, nil) + client, cleanup := TestClient(t, nil) + defer cleanup() req := &nstructs.NodeSpecificRequest{} var resp nstructs.GenericResponse @@ -29,11 +30,11 @@ func TestAllocations_GarbageCollectAll_ACL(t *testing.T) { server, addr, root := testACLServer(t, nil) defer server.Shutdown() - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} c.ACLEnabled = true }) - defer client.Shutdown() + defer cleanup() // Try request without a token and expect failure { @@ -79,9 +80,10 @@ func TestAllocations_GarbageCollect(t *testing.T) { t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.GCDiskUsageThreshold = 100.0 }) + defer cleanup() a := mock.Alloc() a.Job.TaskGroups[0].Tasks[0].Driver = "mock_driver" @@ -122,11 +124,11 @@ func TestAllocations_GarbageCollect_ACL(t *testing.T) { server, addr, root := testACLServer(t, nil) defer server.Shutdown() - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} c.ACLEnabled = true }) - defer client.Shutdown() + defer cleanup() // Try request without a token and expect failure { @@ -178,7 +180,8 @@ func TestAllocations_Stats(t *testing.T) { t.Skip("missing exec driver plugin implementation") t.Parallel() require := require.New(t) - client := TestClient(t, nil) + client, cleanup := TestClient(t, nil) + defer cleanup() a := mock.Alloc() require.Nil(client.addAlloc(a, "")) @@ -213,11 +216,11 @@ func TestAllocations_Stats_ACL(t *testing.T) { server, addr, root := testACLServer(t, nil) defer server.Shutdown() - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} c.ACLEnabled = true }) - defer client.Shutdown() + defer cleanup() // Try request without a token and expect failure { diff --git a/client/allocrunner/alloc_runner_test.go b/client/allocrunner/alloc_runner_test.go index 3c50983a2..92b8ddf27 100644 --- a/client/allocrunner/alloc_runner_test.go +++ b/client/allocrunner/alloc_runner_test.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/plugins/shared/catalog" - "github.com/hashicorp/nomad/plugins/shared/loader" "github.com/hashicorp/nomad/plugins/shared/singleton" "github.com/hashicorp/nomad/testutil" "github.com/stretchr/testify/require" @@ -56,21 +55,24 @@ func (m *MockStateUpdater) Reset() { } // testAllocRunnerConfig returns a new allocrunner.Config with mocks and noop -// versions of dependencies. -func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) *Config { +// versions of dependencies along with a cleanup func. +func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) (*Config, func()) { logger := testlog.HCLogger(t) - return &Config{ + pluginLoader := catalog.TestPluginLoader(t) + clientConf, cleanup := config.TestClientConfig(t) + conf := &Config{ // Copy the alloc in case the caller edits and reuses it Alloc: alloc.Copy(), Logger: logger, - ClientConfig: config.TestClientConfig(), + ClientConfig: clientConf, StateDB: state.NoopDB{}, Consul: consulapi.NewMockConsulServiceClient(t, logger), Vault: vaultclient.NewMockVaultClient(), StateUpdater: &MockStateUpdater{}, PrevAllocWatcher: allocwatcher.NoopPrevAlloc{}, - PluginSingletonLoader: &loader.MockCatalog{}, + PluginSingletonLoader: singleton.NewSingletonLoader(logger, pluginLoader), } + return conf, cleanup } // TestAllocRunner_AllocState_Initialized asserts that getting TaskStates via @@ -78,10 +80,11 @@ func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) *Config { func TestAllocRunner_AllocState_Initialized(t *testing.T) { t.Parallel() - conf := testAllocRunnerConfig(t, mock.Alloc()) + alloc := mock.Alloc() + alloc.Job.TaskGroups[0].Tasks[0].Driver = "mock_driver" + conf, cleanup := testAllocRunnerConfig(t, alloc) + defer cleanup() - pluginLoader := catalog.TestPluginLoader(t) - conf.PluginSingletonLoader = singleton.NewSingletonLoader(conf.Logger, pluginLoader) ar, err := NewAllocRunner(conf) require.NoError(t, err) @@ -105,7 +108,7 @@ func TestAllocRunner_TaskLeader_KillTG(t *testing.T) { task.Driver = "mock_driver" task.KillTimeout = 10 * time.Millisecond task.Config = map[string]interface{}{ - "run_for": "10s", + "run_for": 10 * time.Second, } task2 := alloc.Job.TaskGroups[0].Tasks[0].Copy() @@ -113,12 +116,13 @@ func TestAllocRunner_TaskLeader_KillTG(t *testing.T) { task2.Driver = "mock_driver" task2.Leader = true task2.Config = map[string]interface{}{ - "run_for": "1s", + "run_for": 1 * time.Second, } alloc.Job.TaskGroups[0].Tasks = append(alloc.Job.TaskGroups[0].Tasks, task2) alloc.TaskResources[task2.Name] = task2.Resources - conf := testAllocRunnerConfig(t, alloc) + conf, cleanup := testAllocRunnerConfig(t, alloc) + defer cleanup() ar, err := NewAllocRunner(conf) require.NoError(t, err) defer ar.Destroy() @@ -186,31 +190,29 @@ func TestAllocRunner_TaskLeader_StopTG(t *testing.T) { task := alloc.Job.TaskGroups[0].Tasks[0] task.Name = "follower1" task.Driver = "mock_driver" - task.KillTimeout = 10 * time.Millisecond task.Config = map[string]interface{}{ - "run_for": "10s", + "run_for": 10 * time.Second, } task2 := alloc.Job.TaskGroups[0].Tasks[0].Copy() task2.Name = "leader" task2.Driver = "mock_driver" task2.Leader = true - task2.KillTimeout = 10 * time.Millisecond task2.Config = map[string]interface{}{ - "run_for": "10s", + "run_for": 10 * time.Second, } task3 := alloc.Job.TaskGroups[0].Tasks[0].Copy() task3.Name = "follower2" task3.Driver = "mock_driver" - task3.KillTimeout = 10 * time.Millisecond task3.Config = map[string]interface{}{ - "run_for": "10s", + "run_for": 10 * time.Second, } alloc.Job.TaskGroups[0].Tasks = append(alloc.Job.TaskGroups[0].Tasks, task2, task3) alloc.TaskResources[task2.Name] = task2.Resources - conf := testAllocRunnerConfig(t, alloc) + conf, cleanup := testAllocRunnerConfig(t, alloc) + defer cleanup() ar, err := NewAllocRunner(conf) require.NoError(t, err) defer ar.Destroy() diff --git a/client/client_stats_endpoint_test.go b/client/client_stats_endpoint_test.go index c16f91f3d..2349d26ad 100644 --- a/client/client_stats_endpoint_test.go +++ b/client/client_stats_endpoint_test.go @@ -14,7 +14,8 @@ import ( func TestClientStats_Stats(t *testing.T) { t.Parallel() require := require.New(t) - client := TestClient(t, nil) + client, cleanup := TestClient(t, nil) + defer cleanup() req := &nstructs.NodeSpecificRequest{} var resp structs.ClientStatsResponse @@ -30,11 +31,11 @@ func TestClientStats_Stats_ACL(t *testing.T) { server, addr, root := testACLServer(t, nil) defer server.Shutdown() - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} c.ACLEnabled = true }) - defer client.Shutdown() + defer cleanup() // Try request without a token and expect failure { diff --git a/client/client_test.go b/client/client_test.go index 5b293a261..58020ea6f 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -37,7 +37,8 @@ func testServer(t *testing.T, cb func(*nomad.Config)) (*nomad.Server, string) { func TestClient_StartStop(t *testing.T) { t.Parallel() - client := TestClient(t, nil) + client, cleanup := TestClient(t, nil) + defer cleanup() if err := client.Shutdown(); err != nil { t.Fatalf("err: %v", err) } @@ -49,10 +50,11 @@ func TestClient_BaseLabels(t *testing.T) { t.Parallel() assert := assert.New(t) - client := TestClient(t, nil) + client, cleanup := TestClient(t, nil) if err := client.Shutdown(); err != nil { t.Fatalf("err: %v", err) } + defer cleanup() // directly invoke this function, as otherwise this will fail on a CI build // due to a race condition @@ -74,10 +76,10 @@ func TestClient_RPC(t *testing.T) { s1, addr := testServer(t, nil) defer s1.Shutdown() - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} }) - defer c1.Shutdown() + defer cleanup() // RPC should succeed testutil.WaitForResult(func() (bool, error) { @@ -94,10 +96,10 @@ func TestClient_RPC_FireRetryWatchers(t *testing.T) { s1, addr := testServer(t, nil) defer s1.Shutdown() - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} }) - defer c1.Shutdown() + defer cleanup() watcher := c1.rpcRetryWatcher() @@ -122,10 +124,10 @@ func TestClient_RPC_Passthrough(t *testing.T) { s1, _ := testServer(t, nil) defer s1.Shutdown() - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() // RPC should succeed testutil.WaitForResult(func() (bool, error) { @@ -140,8 +142,8 @@ func TestClient_RPC_Passthrough(t *testing.T) { func TestClient_Fingerprint(t *testing.T) { t.Parallel() - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Ensure we are fingerprinting testutil.WaitForResult(func() (bool, error) { @@ -162,13 +164,13 @@ func TestClient_Fingerprint_Periodic(t *testing.T) { t.Skip("missing mock driver plugin implementation") t.Parallel() - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ driver.ShutdownPeriodicAfter: "true", driver.ShutdownPeriodicDuration: "1", } }) - defer c1.Shutdown() + defer cleanup() node := c1.config.Node { @@ -251,10 +253,10 @@ func TestClient_MixedTLS(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} }) - defer c1.Shutdown() + defer cleanup() req := structs.NodeSpecificRequest{ NodeID: c1.Node().ID, @@ -301,7 +303,7 @@ func TestClient_BadTLS(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} c.TLSConfig = &nconfig.TLSConfig{ EnableHTTP: true, @@ -312,7 +314,7 @@ func TestClient_BadTLS(t *testing.T) { KeyFile: badkey, } }) - defer c1.Shutdown() + defer cleanup() req := structs.NodeSpecificRequest{ NodeID: c1.Node().ID, @@ -339,10 +341,10 @@ func TestClient_Register(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() req := structs.NodeSpecificRequest{ NodeID: c1.Node().ID, @@ -373,10 +375,10 @@ func TestClient_Heartbeat(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() req := structs.NodeSpecificRequest{ NodeID: c1.Node().ID, @@ -406,10 +408,10 @@ func TestClient_UpdateAllocStatus(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() // Wait until the node is ready waitTilNodeReady(c1, t) @@ -457,10 +459,10 @@ func TestClient_WatchAllocs(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() // Wait until the node is ready waitTilNodeReady(c1, t) @@ -552,11 +554,11 @@ func TestClient_SaveRestoreState(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.DevMode = false c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() // Wait until the node is ready waitTilNodeReady(c1, t) @@ -670,10 +672,10 @@ func TestClient_BlockedAllocations(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.RPCHandler = s1 }) - defer c1.Shutdown() + defer cleanup() // Wait for the node to be ready state := s1.State() @@ -781,10 +783,10 @@ func TestClient_ValidateMigrateToken_ValidToken(t *testing.T) { t.Parallel() assert := assert.New(t) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.ACLEnabled = true }) - defer c.Shutdown() + defer cleanup() alloc := mock.Alloc() validToken, err := structs.GenerateMigrateToken(alloc.ID, c.secretNodeID()) @@ -797,10 +799,10 @@ func TestClient_ValidateMigrateToken_InvalidToken(t *testing.T) { t.Parallel() assert := assert.New(t) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.ACLEnabled = true }) - defer c.Shutdown() + defer cleanup() assert.Equal(c.ValidateMigrateToken("", ""), false) @@ -813,8 +815,8 @@ func TestClient_ValidateMigrateToken_ACLDisabled(t *testing.T) { t.Parallel() assert := assert.New(t) - c := TestClient(t, func(c *config.Config) {}) - defer c.Shutdown() + c, cleanup := TestClient(t, func(c *config.Config) {}) + defer cleanup() assert.Equal(c.ValidateMigrateToken("", ""), true) } @@ -835,10 +837,10 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) { fookey = "../helper/tlsutil/testdata/nomad-foo-key.pem" ) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} }) - defer c1.Shutdown() + defer cleanup() // Registering a node over plaintext should succeed { @@ -911,7 +913,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { fookey = "../helper/tlsutil/testdata/nomad-foo-key.pem" ) - c1 := TestClient(t, func(c *config.Config) { + c1, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{addr} c.TLSConfig = &nconfig.TLSConfig{ EnableHTTP: true, @@ -922,7 +924,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { KeyFile: fookey, } }) - defer c1.Shutdown() + defer cleanup() // assert that when one node is running in encrypted mode, a RPC request to a // node running in plaintext mode should fail @@ -974,7 +976,8 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { // nomad server list. func TestClient_ServerList(t *testing.T) { t.Parallel() - client := TestClient(t, func(c *config.Config) {}) + client, cleanup := TestClient(t, func(c *config.Config) {}) + defer cleanup() if s := client.GetServers(); len(s) != 0 { t.Fatalf("expected server lit to be empty but found: %+q", s) diff --git a/client/config/testing.go b/client/config/testing.go index c98cad8c9..8281938ff 100644 --- a/client/config/testing.go +++ b/client/config/testing.go @@ -1,13 +1,43 @@ package config import ( + "io/ioutil" + "os" + "path/filepath" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" + "github.com/mitchellh/go-testing-interface" ) -// TestClientConfig returns a default client configuration for test clients. -func TestClientConfig() *Config { +// TestClientConfig returns a default client configuration for test clients and +// a cleanup func to remove the state and alloc dirs when finished. +func TestClientConfig(t testing.T) (*Config, func()) { conf := DefaultConfig() + + // Create a tempdir to hold state and alloc subdirs + parent, err := ioutil.TempDir("", "nomadtest") + if err != nil { + t.Fatalf("error creating client dir: %v", err) + } + cleanup := func() { + os.RemoveAll(parent) + } + + allocDir := filepath.Join(parent, "allocs") + if err := os.Mkdir(allocDir, 0777); err != nil { + cleanup() + t.Fatalf("error creating alloc dir: %v", err) + } + conf.AllocDir = allocDir + + stateDir := filepath.Join(parent, "client") + if err := os.Mkdir(stateDir, 0777); err != nil { + cleanup() + t.Fatalf("error creating alloc dir: %v", err) + } + conf.StateDir = stateDir + conf.VaultConfig.Enabled = helper.BoolToPtr(false) conf.DevMode = true conf.Node = &structs.Node{ @@ -19,5 +49,5 @@ func TestClientConfig() *Config { // Loosen GC threshold conf.GCDiskUsageThreshold = 98.0 conf.GCInodeUsageThreshold = 98.0 - return conf + return conf, cleanup } diff --git a/client/fingerprint_manager_test.go b/client/fingerprint_manager_test.go index 7b0804399..50fc114ea 100644 --- a/client/fingerprint_manager_test.go +++ b/client/fingerprint_manager_test.go @@ -20,10 +20,10 @@ func TestFingerprintManager_Run_MockDriver(t *testing.T) { t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - testClient := TestClient(t, nil) + testClient, cleanup := TestClient(t, nil) testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -48,10 +48,8 @@ func TestFingerprintManager_Run_MockDriver(t *testing.T) { func TestFingerprintManager_Run_ResourcesFingerprint(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, nil) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + testClient, cleanup := TestClient(t, nil) + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -76,14 +74,12 @@ func TestFingerprintManager_Run_ResourcesFingerprint(t *testing.T) { func TestFingerprintManager_Fingerprint_Run(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "true", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -109,15 +105,13 @@ func TestFingerprintManager_Fingerprint_Periodic(t *testing.T) { t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -172,16 +166,14 @@ func TestFingerprintManager_HealthCheck_Driver(t *testing.T) { t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "1", "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -275,15 +267,13 @@ func TestFingerprintManager_HealthCheck_Periodic(t *testing.T) { t.Skip("missing mock driver plugin implementation") t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -373,15 +363,13 @@ func TestFimgerprintManager_Run_InWhitelist(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "test.shutdown_periodic_after": "true", "test.shutdown_periodic_duration": "2", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -404,15 +392,13 @@ func TestFimgerprintManager_Run_InWhitelist(t *testing.T) { func TestFingerprintManager_Run_InBlacklist(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "fingerprint.whitelist": " arch,memory,foo,bar ", "fingerprint.blacklist": " cpu ", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -437,15 +423,13 @@ func TestFingerprintManager_Run_Combination(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "fingerprint.whitelist": " arch,cpu,memory,foo,bar ", "fingerprint.blacklist": " memory,nomad ", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -471,15 +455,13 @@ func TestFingerprintManager_Run_Combination(t *testing.T) { func TestFingerprintManager_Run_WhitelistDrivers(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " raw_exec , foo ", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -503,15 +485,13 @@ func TestFingerprintManager_Run_AllDriversBlacklisted(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " foo,bar,baz ", } }) - - testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -538,7 +518,7 @@ func TestFingerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing. t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " raw_exec,exec,foo,bar,baz ", @@ -547,7 +527,7 @@ func TestFingerprintManager_Run_DriversWhiteListBlacklistCombination(t *testing. }) testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, @@ -572,14 +552,14 @@ func TestFingerprintManager_Run_DriverFailure(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "1", } }) testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() singLoader := testClient.config.PluginSingletonLoader @@ -626,7 +606,7 @@ func TestFingerprintManager_Run_DriversInBlacklist(t *testing.T) { t.Parallel() require := require.New(t) - testClient := TestClient(t, func(c *config.Config) { + testClient, cleanup := TestClient(t, func(c *config.Config) { c.Options = map[string]string{ "driver.raw_exec.enable": "1", "driver.whitelist": " raw_exec,foo,bar,baz ", @@ -635,7 +615,7 @@ func TestFingerprintManager_Run_DriversInBlacklist(t *testing.T) { }) testClient.logger = testlog.HCLogger(t) - defer testClient.Shutdown() + defer cleanup() fm := NewFingerprintManager( testClient.config.PluginSingletonLoader, diff --git a/client/fs_endpoint_test.go b/client/fs_endpoint_test.go index 4986338e4..f3cf6d646 100644 --- a/client/fs_endpoint_test.go +++ b/client/fs_endpoint_test.go @@ -57,8 +57,8 @@ func TestFS_Stat_NoAlloc(t *testing.T) { require := require.New(t) // Start a client - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Make the request with bad allocation id req := &cstructs.FsStatRequest{ @@ -79,8 +79,8 @@ func TestFS_Stat(t *testing.T) { require := require.New(t) // Start a client - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Create and add an alloc a := mock.Alloc() @@ -134,11 +134,11 @@ func TestFS_Stat_ACL(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.ACLEnabled = true c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer client.Shutdown() + defer cleanup() // Create a bad token policyBad := mock.NamespacePolicy("other", "", []string{acl.NamespaceCapabilityDeny}) @@ -196,8 +196,8 @@ func TestFS_List_NoAlloc(t *testing.T) { require := require.New(t) // Start a client - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Make the request with bad allocation id req := &cstructs.FsListRequest{ @@ -218,8 +218,8 @@ func TestFS_List(t *testing.T) { require := require.New(t) // Start a client - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Create and add an alloc a := mock.Alloc() @@ -273,11 +273,11 @@ func TestFS_List_ACL(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.ACLEnabled = true c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer client.Shutdown() + defer cleanup() // Create a bad token policyBad := mock.NamespacePolicy("other", "", []string{acl.NamespaceCapabilityDeny}) @@ -335,8 +335,8 @@ func TestFS_Stream_NoAlloc(t *testing.T) { require := require.New(t) // Start a client - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Make the request with bad allocation id req := &cstructs.FsStreamRequest{ @@ -414,11 +414,11 @@ func TestFS_Stream_ACL(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.ACLEnabled = true c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer client.Shutdown() + defer cleanup() // Create a bad token policyBad := mock.NamespacePolicy("other", "", []string{acl.NamespaceCapabilityReadFS}) @@ -534,10 +534,10 @@ func TestFS_Stream(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() expected := "Hello from the other side" job := mock.BatchJob() @@ -645,10 +645,10 @@ func TestFS_Stream_Follow(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() expectedBase := "Hello from the other side" repeat := 10 @@ -743,10 +743,10 @@ func TestFS_Stream_Limit(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() var limit int64 = 5 full := "Hello from the other side" @@ -833,8 +833,8 @@ func TestFS_Logs_NoAlloc(t *testing.T) { require := require.New(t) // Start a client - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Make the request with bad allocation id req := &cstructs.FsLogsRequest{ @@ -916,10 +916,10 @@ func TestFS_Logs_TaskPending(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() job := mock.BatchJob() job.TaskGroups[0].Count = 1 @@ -1024,11 +1024,11 @@ func TestFS_Logs_ACL(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - client := TestClient(t, func(c *config.Config) { + client, cleanup := TestClient(t, func(c *config.Config) { c.ACLEnabled = true c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer client.Shutdown() + defer cleanup() // Create a bad token policyBad := mock.NamespacePolicy("other", "", []string{acl.NamespaceCapabilityReadFS}) @@ -1145,10 +1145,10 @@ func TestFS_Logs(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() expected := "Hello from the other side\n" job := mock.BatchJob() @@ -1247,10 +1247,10 @@ func TestFS_Logs_Follow(t *testing.T) { defer s.Shutdown() testutil.WaitForLeader(t, s.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() expectedBase := "Hello from the other side\n" repeat := 10 @@ -1545,8 +1545,8 @@ func TestFS_findClosest(t *testing.T) { func TestFS_streamFile_NoFile(t *testing.T) { t.Parallel() require := require.New(t) - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() ad := tempAllocDir(t) defer os.RemoveAll(ad.AllocDir) @@ -1565,8 +1565,8 @@ func TestFS_streamFile_NoFile(t *testing.T) { func TestFS_streamFile_Modify(t *testing.T) { t.Parallel() - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Get a temp alloc dir ad := tempAllocDir(t) @@ -1634,8 +1634,8 @@ func TestFS_streamFile_Modify(t *testing.T) { func TestFS_streamFile_Truncate(t *testing.T) { t.Parallel() - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Get a temp alloc dir ad := tempAllocDir(t) @@ -1739,8 +1739,8 @@ func TestFS_streamFile_Truncate(t *testing.T) { func TestFS_streamImpl_Delete(t *testing.T) { t.Parallel() - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Get a temp alloc dir ad := tempAllocDir(t) @@ -1807,8 +1807,8 @@ func TestFS_streamImpl_Delete(t *testing.T) { func TestFS_logsImpl_NoFollow(t *testing.T) { t.Parallel() - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Get a temp alloc dir and create the log dir ad := tempAllocDir(t) @@ -1874,8 +1874,8 @@ func TestFS_logsImpl_NoFollow(t *testing.T) { func TestFS_logsImpl_Follow(t *testing.T) { t.Parallel() - c := TestClient(t, nil) - defer c.Shutdown() + c, cleanup := TestClient(t, nil) + defer cleanup() // Get a temp alloc dir and create the log dir ad := tempAllocDir(t) diff --git a/client/rpc_test.go b/client/rpc_test.go index c25033923..404e47c12 100644 --- a/client/rpc_test.go +++ b/client/rpc_test.go @@ -19,10 +19,10 @@ func TestRpc_streamingRpcConn_badEndpoint(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Servers = []string{s1.GetConfig().RPCAddr.String()} }) - defer c.Shutdown() + defer cleanup() // Wait for the client to connect testutil.WaitForResult(func() (bool, error) { @@ -75,7 +75,7 @@ func TestRpc_streamingRpcConn_badEndpoint_TLS(t *testing.T) { defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) - c := TestClient(t, func(c *config.Config) { + c, cleanup := TestClient(t, func(c *config.Config) { c.Region = "regionFoo" c.Servers = []string{s1.GetConfig().RPCAddr.String()} c.TLSConfig = &sconfig.TLSConfig{ @@ -87,7 +87,7 @@ func TestRpc_streamingRpcConn_badEndpoint_TLS(t *testing.T) { KeyFile: fookey, } }) - defer c.Shutdown() + defer cleanup() // Wait for the client to connect testutil.WaitForResult(func() (bool, error) { diff --git a/client/testing.go b/client/testing.go index 941094990..bd67fd5a4 100644 --- a/client/testing.go +++ b/client/testing.go @@ -11,9 +11,14 @@ import ( "github.com/mitchellh/go-testing-interface" ) -// TestClient creates an in-memory client for testing purposes. -func TestClient(t testing.T, cb func(c *config.Config)) *Client { - conf := config.TestClientConfig() +// TestClient creates an in-memory client for testing purposes and returns a +// cleanup func to shutdown the client and remove the alloc and state dirs. +// +// There is no need to override the AllocDir or StateDir as they are randomized +// and removed in the returned cleanup function. If they are overridden in the +// callback then the caller still must run the returned cleanup func. +func TestClient(t testing.T, cb func(c *config.Config)) (*Client, func()) { + conf, cleanup := config.TestClientConfig(t) // Tighten the fingerprinter timeouts (must be done in client package // to avoid circular dependencies) @@ -38,7 +43,14 @@ func TestClient(t testing.T, cb func(c *config.Config)) *Client { mockService := consulApi.NewMockConsulServiceClient(t, logger) client, err := NewClient(conf, catalog, mockService) if err != nil { + cleanup() t.Fatalf("err: %v", err) } - return client + return client, func() { + // Shutdown client + client.Shutdown() + + // Call TestClientConfig cleanup + cleanup() + } }