luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

testing: Remove TestAgent.Key and change TestAgent.DataDir 

TestAgent.Key was only used by 3 tests. Extracting it from the common helper that is used in hundreds of
tests helps keep the shared part small and more focused.

This required a second change (which I was planning on making anyway), which was to change the behaviour of
DataDir. Now in all cases the TestAgent will use the DataDir, and clean it up once the test is complete.
This commit is contained in:
Daniel Nephin 2020-08-13 17:17:21 -04:00
parent 3abe4e43d3
commit b6d91d59f3
4 changed files with 50 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
agent/agent_test.go
View File

@ -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")
}

30
agent/keyring_test.go
View File

@ -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

17
agent/service_manager_test.go
View File

@ -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()
{

71
agent/testagent.go
View File

@ -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)