metropolis: fix tests using etcd
Ever since we bumped etcd, we have started calling
integration.BeforeTest. The correct call is BeforeTestExternal,
otherwise some internal-etcd-integration-test logic is invoked, which
seems to break test error calls (!) in some cases (probably goroutine
leak detection, which is enabled by default when using BeforeTest -
which we don't care about, as we don't [yet] expect our tests to be fully
clean).
We also have to modify the harnesses used by curator tests to
synchronously terminate the cluster on each test end. Otherwise, etcd
will fail due to conflicts on domain sockets on which test members
listen. Unfortunately, there doesn't seem to be an easy way to run each
cluster/test in a totally separate, non-conflicting socket setup.
Change-Id: I2fb1332edb35349b66af131684feb378ae3a13ee
Reviewed-on: https://review.monogon.dev/c/monogon/+/688
Reviewed-by: Mateusz Zalega <mateusz@monogon.tech>
diff --git a/metropolis/node/core/curator/BUILD.bazel b/metropolis/node/core/curator/BUILD.bazel
index c13282e..16bfbfe 100644
--- a/metropolis/node/core/curator/BUILD.bazel
+++ b/metropolis/node/core/curator/BUILD.bazel
@@ -64,7 +64,6 @@
"//metropolis/pkg/supervisor",
"//metropolis/proto/api",
"//metropolis/proto/common",
- "@io_etcd_go_etcd_client_pkg_v3//testutil",
"@io_etcd_go_etcd_client_v3//:client",
"@io_etcd_go_etcd_tests_v3//integration",
"@org_golang_google_grpc//:go_default_library",
diff --git a/metropolis/node/core/curator/curator_test.go b/metropolis/node/core/curator/curator_test.go
index 3ea66a0..b1a9671 100644
--- a/metropolis/node/core/curator/curator_test.go
+++ b/metropolis/node/core/curator/curator_test.go
@@ -2,13 +2,11 @@
import (
"context"
- "flag"
"fmt"
"os"
"testing"
"time"
- "go.etcd.io/etcd/client/pkg/v3/testutil"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/tests/v3/integration"
@@ -25,30 +23,6 @@
endpoints []string
)
-// TestMain brings up a 3 node etcd cluster for tests to use.
-func TestMain(m *testing.M) {
- cfg := integration.ClusterConfig{
- Size: 3,
- GRPCKeepAliveMinTime: time.Millisecond,
- }
- t, cancel := testutil.NewTestingTBProthesis("curator")
- defer cancel()
-
- flag.Parse()
-
- integration.BeforeTest(t)
-
- cluster = integration.NewClusterV3(t, &cfg)
- endpoints = make([]string, 3)
- for i := range endpoints {
- endpoints[i] = cluster.Client(i).Endpoints()[0]
- }
-
- v := m.Run()
- cluster.Terminate(t)
- os.Exit(v)
-}
-
// dut is the design under test harness - in this case, a curator instance.
type dut struct {
// endpoint of the etcd server that this instance is connected to. Each instance
@@ -201,7 +175,22 @@
// re-election.
func TestLeaderElectionStatus(t *testing.T) {
ctx, ctxC := context.WithCancel(context.Background())
- defer ctxC()
+ cfg := integration.ClusterConfig{
+ Size: 3,
+ GRPCKeepAliveMinTime: time.Millisecond,
+ }
+ integration.BeforeTestExternal(t)
+ cluster = integration.NewClusterV3(t, &cfg)
+ t.Cleanup(func() {
+ ctxC()
+ cluster.Terminate(t)
+ cluster = nil
+ endpoints = nil
+ })
+ endpoints = make([]string, 3)
+ for i := range endpoints {
+ endpoints[i] = cluster.Client(i).Endpoints()[0]
+ }
// Map from endpoint name to etcd member list index. Alongside with the
// endpoints list, this is used to quickly look up endpoint<->member_num. Since
diff --git a/metropolis/node/core/curator/impl_leader_test.go b/metropolis/node/core/curator/impl_leader_test.go
index 2894b29..fffe69c 100644
--- a/metropolis/node/core/curator/impl_leader_test.go
+++ b/metropolis/node/core/curator/impl_leader_test.go
@@ -44,15 +44,17 @@
ctx, ctxC := context.WithCancel(context.Background())
// Start a single-node etcd cluster.
- integration.BeforeTest(t)
+ integration.BeforeTestExternal(t)
cluster := integration.NewClusterV3(t, &integration.ClusterConfig{
Size: 1,
})
- // Terminate the etcd cluster on context cancel.
- go func() {
- <-ctx.Done()
+ // Clean up the etcd cluster and cancel the context on test end. We don't just
+ // use a context because we need the cluster to terminate synchronously before
+ // another test is ran.
+ t.Cleanup(func() {
+ ctxC()
cluster.Terminate(t)
- }()
+ })
// Create etcd client to test cluster.
curEtcd, _ := client.NewLocal(cluster.Client(0)).Sub("curator")
@@ -195,7 +197,6 @@
otherNodeID: identity.NodeID(otherPub),
otherNodePriv: otherPriv,
ca: nodeCredentials.ClusterCA(),
- cancel: ctxC,
etcd: curEtcd,
}
}
@@ -227,8 +228,6 @@
// state, ie. the private key for otherNodeID.
otherNodePriv ed25519.PrivateKey
ca *x509.Certificate
- // cancel shuts down the fake leader and all client connections.
- cancel context.CancelFunc
// etcd contains a low-level connection to the curator K/V store, which can be
// used to perform low-level changes to the store in tests.
etcd client.Namespaced
@@ -238,7 +237,6 @@
// through updates, to its deletion.
func TestWatchNodeInCluster(t *testing.T) {
cl := fakeLeader(t)
- defer cl.cancel()
ctx, ctxC := context.WithCancel(context.Background())
defer ctxC()
@@ -351,7 +349,6 @@
// through updates, to not deletion.
func TestWatchNodesInCluster(t *testing.T) {
cl := fakeLeader(t)
- defer cl.cancel()
ctx, ctxC := context.WithCancel(context.Background())
defer ctxC()
@@ -503,7 +500,6 @@
// test as a set of credentials.
func TestRegistration(t *testing.T) {
cl := fakeLeader(t)
- defer cl.cancel()
mgmt := apb.NewManagementClient(cl.mgmtConn)
@@ -604,7 +600,6 @@
// document, assuming the node has already completed Register Flow.
func TestJoin(t *testing.T) {
cl := fakeLeader(t)
- defer cl.cancel()
ctx, ctxC := context.WithCancel(context.Background())
defer ctxC()
@@ -659,7 +654,6 @@
// events.
func TestClusterUpdateNodeStatus(t *testing.T) {
cl := fakeLeader(t)
- defer cl.cancel()
curator := ipb.NewCuratorClient(cl.localNodeConn)
@@ -728,7 +722,6 @@
// TestManagementClusterInfo exercises GetClusterInfo after setting a status.
func TestManagementClusterInfo(t *testing.T) {
cl := fakeLeader(t)
- defer cl.cancel()
mgmt := apb.NewManagementClient(cl.mgmtConn)
curator := ipb.NewCuratorClient(cl.localNodeConn)
diff --git a/metropolis/pkg/event/etcd/etcd_test.go b/metropolis/pkg/event/etcd/etcd_test.go
index d91a6b9..f95fa0b 100644
--- a/metropolis/pkg/event/etcd/etcd_test.go
+++ b/metropolis/pkg/event/etcd/etcd_test.go
@@ -36,7 +36,7 @@
tb, cancel := testutil.NewTestingTBProthesis("curator")
defer cancel()
flag.Parse()
- integration.BeforeTest(tb)
+ integration.BeforeTestExternal(tb)
cluster = integration.NewClusterV3(tb, &cfg)
endpoints = make([]string, 3)
for i := range endpoints {