diff --git a/agent/agent_test.go b/agent/agent_test.go index 4b109b967..4746fd3ff 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1536,15 +1536,12 @@ func TestAgent_RestoreServiceWithAliasCheck(t *testing.T) { a.logger.Info("testharness: " + fmt.Sprintf(format, args...)) } - dataDir := testutil.TempDir(t, "agent") // we manage the data dir cfg := ` server = false bootstrap = false enable_central_service_config = false - data_dir = "` + dataDir + `" ` - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) - defer os.RemoveAll(dataDir) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() testCtx, testCancel := context.WithCancel(context.Background()) @@ -1627,7 +1624,7 @@ node_name = "` + a.Config.NodeName + `" t.Helper() // Reload and retain former NodeID and data directory. - a2 := StartTestAgent(t, TestAgent{HCL: futureHCL, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: futureHCL, DataDir: a.DataDir}) defer a2.Shutdown() a = nil @@ -2012,15 +2009,11 @@ func TestAgent_PersistService(t *testing.T) { func testAgent_PersistService(t *testing.T, extraHCL string) { t.Helper() - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - cfg := ` server = false bootstrap = false - data_dir = "` + dataDir + `" ` + extraHCL - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() svc := &structs.NodeService{ @@ -2086,7 +2079,7 @@ func testAgent_PersistService(t *testing.T, extraHCL string) { a.Shutdown() // Should load it back during later start - a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() restored := a2.State.ServiceState(structs.NewServiceID(svc.ID, nil)) @@ -2224,15 +2217,11 @@ func TestAgent_PurgeServiceOnDuplicate(t *testing.T) { func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { t.Helper() - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - cfg := ` - data_dir = "` + dataDir + `" server = false bootstrap = false ` + extraHCL - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() svc1 := &structs.NodeService{ @@ -2255,7 +2244,7 @@ func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { tags = ["bar"] port = 9000 } - `, DataDir: dataDir}) + `, DataDir: a.DataDir}) defer a2.Shutdown() sid := svc1.CompoundServiceID() @@ -2269,15 +2258,12 @@ func testAgent_PurgeServiceOnDuplicate(t *testing.T, extraHCL string) { func TestAgent_PersistCheck(t *testing.T) { t.Parallel() - dataDir := testutil.TempDir(t, "agent") // we manage the data dir cfg := ` - data_dir = "` + dataDir + `" server = false bootstrap = false enable_script_checks = true ` - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) - defer os.RemoveAll(dataDir) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() check := &structs.HealthCheck{ @@ -2333,7 +2319,7 @@ func TestAgent_PersistCheck(t *testing.T) { a.Shutdown() // Should load it back during later start - a2 := StartTestAgent(t, TestAgent{Name: "Agent2", HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{Name: "Agent2", HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() result := requireCheckExists(t, a2, check.CheckID) @@ -2384,18 +2370,14 @@ func TestAgent_PurgeCheck(t *testing.T) { func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { t.Parallel() nodeID := NodeID() - dataDir := testutil.TempDir(t, "agent") a := StartTestAgent(t, TestAgent{ - DataDir: dataDir, HCL: ` node_id = "` + nodeID + `" node_name = "Node ` + nodeID + `" - data_dir = "` + dataDir + `" server = false bootstrap = false enable_script_checks = true `}) - defer os.RemoveAll(dataDir) defer a.Shutdown() check1 := &structs.HealthCheck{ @@ -2415,11 +2397,10 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { // Start again with the check registered in config a2 := StartTestAgent(t, TestAgent{ Name: "Agent2", - DataDir: dataDir, + DataDir: a.DataDir, HCL: ` node_id = "` + nodeID + `" node_name = "Node ` + nodeID + `" - data_dir = "` + dataDir + `" server = false bootstrap = false enable_script_checks = true @@ -2434,7 +2415,7 @@ func TestAgent_PurgeCheckOnDuplicate(t *testing.T) { defer a2.Shutdown() cid := check1.CompoundCheckID() - file := filepath.Join(dataDir, checksDir, cid.StringHash()) + file := filepath.Join(a.DataDir, checksDir, cid.StringHash()) if _, err := os.Stat(file); err == nil { t.Fatalf("should have removed persisted check") } diff --git a/agent/keyring_test.go b/agent/keyring_test.go index 2b160d4f7..16acc0161 100644 --- a/agent/keyring_test.go +++ b/agent/keyring_test.go @@ -54,7 +54,10 @@ func TestAgent_LoadKeyrings(t *testing.T) { // Server should auto-load LAN and WAN keyring files t.Run("server with keys", func(t *testing.T) { - a2 := StartTestAgent(t, TestAgent{Key: key}) + dataDir := testutil.TempDir(t, "keyfile") + writeKeyRings(t, key, dataDir) + + a2 := StartTestAgent(t, TestAgent{DataDir: dataDir}) defer a2.Shutdown() c2 := a2.consulConfig() @@ -80,10 +83,16 @@ func TestAgent_LoadKeyrings(t *testing.T) { // Client should auto-load only the LAN keyring file t.Run("client with keys", func(t *testing.T) { - a3 := StartTestAgent(t, TestAgent{HCL: ` + dataDir := testutil.TempDir(t, "keyfile") + writeKeyRings(t, key, dataDir) + + a3 := StartTestAgent(t, TestAgent{ + HCL: ` server = false bootstrap = false - `, Key: key}) + `, + DataDir: dataDir, + }) defer a3.Shutdown() c3 := a3.consulConfig() @@ -105,6 +114,16 @@ func TestAgent_LoadKeyrings(t *testing.T) { }) } +func writeKeyRings(t *testing.T, key string, dataDir string) { + t.Helper() + writeKey := func(key, filename string) { + path := filepath.Join(dataDir, filename) + require.NoError(t, initKeyring(path, key), "Error creating keyring %s", path) + } + writeKey(key, SerfLANKeyring) + writeKey(key, SerfWANKeyring) +} + func TestAgent_InmemKeyrings(t *testing.T) { t.Parallel() key := "tbLJg26ZJyJ9pK3qhc9jig==" @@ -272,11 +291,14 @@ func TestAgentKeyring_ACL(t *testing.T) { key1 := "tbLJg26ZJyJ9pK3qhc9jig==" key2 := "4leC33rgtXKIVUr9Nr0snQ==" + dataDir := testutil.TempDir(t, "keyfile") + writeKeyRings(t, key1, dataDir) + a := StartTestAgent(t, TestAgent{HCL: TestACLConfig() + ` acl_datacenter = "dc1" acl_master_token = "root" acl_default_policy = "deny" - `, Key: key1}) + `, DataDir: dataDir}) defer a.Shutdown() // List keys without access fails diff --git a/agent/service_manager_test.go b/agent/service_manager_test.go index e71e21f22..8cb3ce2a2 100644 --- a/agent/service_manager_test.go +++ b/agent/service_manager_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" "github.com/stretchr/testify/require" @@ -293,16 +292,12 @@ func TestServiceManager_PersistService_API(t *testing.T) { ) // Now launch a single client agent - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - cfg := ` enable_central_service_config = true server = false bootstrap = false - data_dir = "` + dataDir + `" ` - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() // Join first @@ -465,7 +460,7 @@ func TestServiceManager_PersistService_API(t *testing.T) { serverAgent.Shutdown() // Should load it back during later start. - a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() { @@ -510,9 +505,6 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { ) // Now launch a single client agent - dataDir := testutil.TempDir(t, "agent") // we manage the data dir - defer os.RemoveAll(dataDir) - serviceSnippet := ` service = { kind = "connect-proxy" @@ -535,12 +527,11 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { cfg := ` enable_central_service_config = true - data_dir = "` + dataDir + `" server = false bootstrap = false ` + serviceSnippet - a := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a := StartTestAgent(t, TestAgent{HCL: cfg}) defer a.Shutdown() // Join first @@ -639,7 +630,7 @@ func TestServiceManager_PersistService_ConfigFiles(t *testing.T) { serverAgent.Shutdown() // Should load it back during later start. - a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: dataDir}) + a2 := StartTestAgent(t, TestAgent{HCL: cfg, DataDir: a.DataDir}) defer a2.Shutdown() { diff --git a/agent/testagent.go b/agent/testagent.go index 74570a368..426de1a91 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -8,13 +8,11 @@ import ( "crypto/x509" "fmt" "io" - "io/ioutil" "math/rand" "net/http/httptest" "os" "path/filepath" "strconv" - "strings" "testing" "text/template" "time" @@ -58,23 +56,18 @@ type TestAgent struct { // the callers responsibility to clean up the data directory. // Otherwise, a temporary data directory is created and removed // when Shutdown() is called. - // TODO: Config *config.RuntimeConfig // LogOutput is the sink for the logs. If nil, logs are written // to os.Stderr. LogOutput io.Writer - // DataDir is the data directory which is used when Config.DataDir - // is not set. It is created automatically and removed when - // Shutdown() is called. - // TODO: + // DataDir may be set to a directory which exists. If is it not set, + // TestAgent.Start will create one and set DataDir to the directory path. + // In all cases the agent will be configured to use this path as the data directory, + // and the directory will be removed once the test ends. DataDir string - // Key is the optional encryption key for the LAN and WAN keyring. - // TODO: - Key string - // UseTLS, if true, will disable the HTTP port and enable the HTTPS // one. UseTLS bool @@ -154,35 +147,14 @@ func (a *TestAgent) Start(t *testing.T) (err error) { name = "TestAgent" } - var cleanupTmpDir = func() { - // Clean out the data dir if we are responsible for it before we - // try again, since the old ports may have gotten written to - // the data dir, such as in the Raft configuration. - if a.DataDir != "" { - if err := os.RemoveAll(a.DataDir); err != nil { - fmt.Printf("%s Error resetting data dir: %s", name, err) - } - } - } - - var hclDataDir string if a.DataDir == "" { - dirname := "agent" - if name != "" { - dirname = name + "-agent" - } - dirname = strings.Replace(dirname, "/", "_", -1) - d, err := ioutil.TempDir(TempDir, dirname) - if err != nil { - return fmt.Errorf("Error creating data dir %s: %s", filepath.Join(TempDir, dirname), err) - } - // Convert windows style path to posix style path - // to avoid illegal char escape error when hcl - // parsing. - d = filepath.ToSlash(d) - hclDataDir = `data_dir = "` + d + `"` - a.DataDir = d + dirname := name + "-agent" + a.DataDir = testutil.TempDir(t, dirname) } + // Convert windows style path to posix style path to avoid illegal char escape + // error when hcl parsing. + d := filepath.ToSlash(a.DataDir) + hclDataDir := fmt.Sprintf(`data_dir = "%s"`, d) logOutput := a.LogOutput if logOutput == nil { @@ -220,29 +192,8 @@ func (a *TestAgent) Start(t *testing.T) (err error) { ), } - // write the keyring - if a.Key != "" { - writeKey := func(key, filename string) error { - path := filepath.Join(a.DataDir, filename) - if err := initKeyring(path, key); err != nil { - cleanupTmpDir() - return fmt.Errorf("Error creating keyring %s: %s", path, err) - } - return nil - } - if err = writeKey(a.Key, SerfLANKeyring); err != nil { - cleanupTmpDir() - return err - } - if err = writeKey(a.Key, SerfWANKeyring); err != nil { - cleanupTmpDir() - return err - } - } - agent, err := New(opts...) if err != nil { - cleanupTmpDir() return fmt.Errorf("Error creating agent: %s", err) } @@ -253,7 +204,6 @@ func (a *TestAgent) Start(t *testing.T) (err error) { id := string(a.Config.NodeID) if err := agent.Start(context.Background()); err != nil { - cleanupTmpDir() agent.ShutdownAgent() agent.ShutdownEndpoints() @@ -266,7 +216,6 @@ func (a *TestAgent) Start(t *testing.T) (err error) { a.Agent.StartSync() if err := a.waitForUp(); err != nil { - cleanupTmpDir() a.Shutdown() t.Logf("Error while waiting for test agent to start: %v", err) return errwrap.Wrapf(name+": {{err}}", err)