status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #8509 from hashicorp/dnephin/use-t.cleanup-in-testagent 

testing: Use t.cleanup in TestAgent , and fix two flaky tests
This commit is contained in:
Daniel Nephin 2020-08-14 16:33:16 -04:00 committed by GitHub
parent 2bf85fae62 070e843113
commit bb6737dc60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 82 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
agent/acl_test.go
View File

@ -3,6 +3,7 @@ package agent
import (
"fmt"
"io"
"os"
"testing"
"time"
@ -25,23 +26,6 @@ type authzResolver func(string) (structs.ACLIdentity, acl.Authorizer, error)
type identResolver func(string) (structs.ACLIdentity, error)
type TestACLAgent struct {
// Name is an optional name of the agent.
Name string
HCL string
// Config is the agent configuration. If Config is nil then
// TestConfig() is used. If Config.DataDir is set then it is
// the callers responsibility to clean up the data directory.
// Otherwise, a temporary data directory is created and removed
// when Shutdown() is called.
Config *config.RuntimeConfig
// DataDir is the data directory which is used when Config.DataDir
// is not set. It is created automatically and removed when
// Shutdown() is called.
DataDir string
resolveAuthzFn authzResolver
resolveIdentFn identResolver
@ -52,11 +36,15 @@ type TestACLAgent struct {
// Basically it needs a local state for some of the vet* functions, a logger and a delegate.
// The key is that we are the delegate so we can control the ResolveToken responses
func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzResolver, resolveIdent identResolver) *TestACLAgent {
a := &TestACLAgent{Name: name, HCL: hcl, resolveAuthzFn: resolveAuthz, resolveIdentFn: resolveIdent}
dataDir := `data_dir = "acl-agent"`
a := &TestACLAgent{resolveAuthzFn: resolveAuthz, resolveIdentFn: resolveIdent}
dataDir := testutil.TempDir(t, "acl-agent")
t.Cleanup(func() {
os.RemoveAll(dataDir)
})
logger := hclog.NewInterceptLogger(&hclog.LoggerOptions{
Name: a.Name,
Name: name,
Level: hclog.Debug,
Output: testutil.NewLogBuffer(t),
})
@ -66,22 +54,22 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe
WithBuilderOpts(config.BuilderOpts{
HCL: []string{
TestConfigHCL(NodeID()),
a.HCL,
dataDir,
hcl,
fmt.Sprintf(`data_dir = "%s"`, dataDir),
},
}),
}
agent, err := New(opts...)
require.NoError(t, err)
a.Config = agent.GetConfig()
cfg := agent.GetConfig()
a.Agent = agent
agent.logger = logger
agent.MemSink = metrics.NewInmemSink(1*time.Second, time.Minute)
a.Agent.delegate = a
a.Agent.State = local.NewState(LocalConfig(a.Config), a.Agent.logger, a.Agent.tokens)
a.Agent.State = local.NewState(LocalConfig(cfg), logger, a.Agent.tokens)
a.Agent.State.TriggerSyncChanges = func() {}
return a
}

9
agent/agent_endpoint_test.go
View File

@ -4542,12 +4542,15 @@ func TestAgent_Monitor(t *testing.T) {
req = req.WithContext(cancelCtx)
resp := httptest.NewRecorder()
errCh := make(chan error)
chErr := make(chan error)
chStarted := make(chan struct{})
go func() {
close(chStarted)
_, err := a.srv.AgentMonitor(resp, req)
errCh <- err
chErr <- err
}()
<-chStarted
require.NoError(t, a.Shutdown())
// Wait until we have received some type of logging output
@ -4556,7 +4559,7 @@ func TestAgent_Monitor(t *testing.T) {
}, 3*time.Second, 100*time.Millisecond)
cancelFunc()
err := <-errCh
err := <-chErr
require.NoError(t, err)
got := resp.Body.String()

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

6
agent/dns_test.go
View File

@ -5829,16 +5829,12 @@ func TestDNS_NonExistentDC_RPC(t *testing.T) {
server = false
`)
defer c.Shutdown()
testrpc.WaitForLeader(t, s.RPC, "dc1")
// Join LAN cluster
addr := fmt.Sprintf("127.0.0.1:%d", s.Config.SerfPortLAN)
_, err := c.JoinLAN([]string{addr})
require.NoError(t, err)
retry.Run(t, func(r *retry.R) {
require.Len(r, s.LANMembers(), 2)
require.Len(r, c.LANMembers(), 2)
})
testrpc.WaitForTestAgent(t, c.RPC, "dc1")
m := new(dns.Msg)
m.SetQuestion("consul.service.dc2.consul.", dns.TypeANY)

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

93
agent/testagent.go
View File

@ -8,13 +8,10 @@ import (
"crypto/x509"
"fmt"
"io"
"io/ioutil"
"math/rand"
"net/http/httptest"
"os"
"path/filepath"
"strconv"
"strings"
"testing"
"text/template"
"time"
@ -40,9 +37,6 @@ func init() {
rand.Seed(time.Now().UnixNano()) // seed random number generator
}
// TempDir defines the base dir for temporary directories.
var TempDir = os.TempDir()
// TestAgent encapsulates an Agent with a default configuration and
// startup procedure suitable for testing. It panics if there are errors
// during creation or startup instead of returning errors. It manages a
@ -60,22 +54,16 @@ type TestAgent struct {
// when Shutdown() is called.
Config *config.RuntimeConfig
// returnPortsFn will put the ports claimed for the test back into the
// general freeport pool
returnPortsFn func()
// 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.
// 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.
Key string
// UseTLS, if true, will disable the HTTP port and enable the HTTPS
// one.
UseTLS bool
@ -155,35 +143,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 {
@ -198,7 +165,7 @@ func (a *TestAgent) Start(t *testing.T) (err error) {
})
portsConfig, returnPortsFn := randomPortsSource(a.UseTLS)
a.returnPortsFn = returnPortsFn
t.Cleanup(returnPortsFn)
nodeID := NodeID()
@ -221,36 +188,8 @@ func (a *TestAgent) Start(t *testing.T) (err error) {
),
}
defer func() {
if err != nil && a.returnPortsFn != nil {
a.returnPortsFn()
a.returnPortsFn = nil
}
}()
// 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)
}
@ -261,7 +200,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()
@ -274,7 +212,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)
@ -357,14 +294,6 @@ func (a *TestAgent) Shutdown() error {
return nil
}
// Return ports last of all
defer func() {
if a.returnPortsFn != nil {
a.returnPortsFn()
a.returnPortsFn = nil
}
}()
// shutdown agent before endpoints
defer a.Agent.ShutdownEndpoints()
if err := a.Agent.ShutdownAgent(); err != nil {

17
sdk/testutil/io.go
View File

@ -29,21 +29,28 @@ func init() {
}
}
var noCleanup = strings.ToLower(os.Getenv("TEST_NOCLEANUP")) == "true"
// TempDir creates a temporary directory within tmpdir
// with the name 'testname-name'. If the directory cannot
// be created t.Fatal is called.
func TempDir(t *testing.T, name string) string {
if t != nil && t.Name() != "" {
name = t.Name() + "-" + name
if t == nil {
panic("argument t must be non-nil")
}
name = t.Name() + "-" + name
name = strings.Replace(name, "/", "_", -1)
d, err := ioutil.TempDir(tmpdir, name)
if err != nil {
if t == nil {
panic(err)
}
t.Fatalf("err: %s", err)
}
t.Cleanup(func() {
if noCleanup {
t.Logf("skipping cleanup because TEST_NOCLEANUP was enabled")
return
}
os.RemoveAll(d)
})
return d
}