tls enforcement flaky tests (#16543)

* tests: add WaitForLeaders helpers using must/wait timings

* tests: start servers for mtls tests together

Fixes #16253 (hopefully)

nomad/rpc_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-msgpack/codec"
 	"github.com/hashicorp/go-sockaddr"
 	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
@@ -1346,9 +1347,9 @@ type tlsTestHelper struct {
 	nodeID int
 
 	mtlsServer1            *Server
-	mtlsServer1Cleanup     func()
+	mtlsServerCleanup1     func()
 	mtlsServer2            *Server
-	mtlsServer2Cleanup     func()
+	mtlsServerCleanup2     func()
 	nonVerifyServer        *Server
 	nonVerifyServerCleanup func()
 
@@ -1367,57 +1368,41 @@ func newTLSTestHelper(t *testing.T) tlsTestHelper {
 
 	// Generate CA certificate and write it to disk.
 	h.caPEM, h.pk, err = tlsutil.GenerateCA(tlsutil.CAOpts{Days: 5, Domain: "nomad"})
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	err = os.WriteFile(filepath.Join(h.dir, "ca.pem"), []byte(h.caPEM), 0600)
-	require.NoError(t, err)
+	must.NoError(t, err)
 
 	// Generate servers and their certificate.
 	h.serverCert = h.newCert(t, "server.global.nomad")
 
-	h.mtlsServer1, h.mtlsServer1Cleanup = TestServer(t, func(c *Config) {
-		c.BootstrapExpect = 2
-		c.TLSConfig = &config.TLSConfig{
-			EnableRPC:            true,
-			VerifyServerHostname: true,
-			CAFile:               filepath.Join(h.dir, "ca.pem"),
-			CertFile:             h.serverCert + ".pem",
-			KeyFile:              h.serverCert + ".key",
-		}
-	})
-	h.mtlsServer2, h.mtlsServer2Cleanup = TestServer(t, func(c *Config) {
-		c.BootstrapExpect = 2
-		c.TLSConfig = &config.TLSConfig{
-			EnableRPC:            true,
-			VerifyServerHostname: true,
-			CAFile:               filepath.Join(h.dir, "ca.pem"),
-			CertFile:             h.serverCert + ".pem",
-			KeyFile:              h.serverCert + ".key",
-		}
-	})
-	TestJoin(t, h.mtlsServer1, h.mtlsServer2)
-	testutil.WaitForLeader(t, h.mtlsServer1.RPC)
-	testutil.WaitForLeader(t, h.mtlsServer2.RPC)
+	makeServer := func(bootstrapExpect int, verifyServerHostname bool) (*Server, func()) {
+		return TestServer(t, func(c *Config) {
+			c.Logger.SetLevel(hclog.Off)
+			c.BootstrapExpect = bootstrapExpect
+			c.TLSConfig = &config.TLSConfig{
+				EnableRPC:            true,
+				VerifyServerHostname: verifyServerHostname,
+				CAFile:               filepath.Join(h.dir, "ca.pem"),
+				CertFile:             h.serverCert + ".pem",
+				KeyFile:              h.serverCert + ".key",
+			}
+		})
+	}
 
-	h.nonVerifyServer, h.nonVerifyServerCleanup = TestServer(t, func(c *Config) {
-		c.TLSConfig = &config.TLSConfig{
-			EnableRPC:            true,
-			VerifyServerHostname: false,
-			CAFile:               filepath.Join(h.dir, "ca.pem"),
-			CertFile:             h.serverCert + ".pem",
-			KeyFile:              h.serverCert + ".key",
-		}
-	})
-	testutil.WaitForLeader(t, h.nonVerifyServer.RPC)
+	h.mtlsServer1, h.mtlsServerCleanup1 = makeServer(3, true)
+	h.mtlsServer2, h.mtlsServerCleanup2 = makeServer(3, true)
+	h.nonVerifyServer, h.nonVerifyServerCleanup = makeServer(3, false)
 
+	TestJoin(t, h.mtlsServer1, h.mtlsServer2, h.nonVerifyServer)
+	testutil.WaitForLeaders(t, h.mtlsServer1.RPC, h.mtlsServer2.RPC, h.nonVerifyServer.RPC)
 	return h
 }
 
 func (h tlsTestHelper) cleanup() {
-	h.mtlsServer1Cleanup()
-	h.mtlsServer2Cleanup()
+	h.mtlsServerCleanup1()
+	h.mtlsServerCleanup2()
 	h.nonVerifyServerCleanup()
-	os.RemoveAll(h.dir)
 }
 
 func (h tlsTestHelper) newCert(t *testing.T, name string) string {

nomad/testing.go

@@ -1,6 +1,7 @@
 package nomad
 
 import (
+	"errors"
 	"fmt"
 	"math/rand"
 	"net"
@@ -14,7 +15,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/version"
-	"github.com/stretchr/testify/require"
+	"github.com/shoenig/test/must"
 )
 
 var (
@@ -38,7 +39,7 @@ func TestACLServer(t *testing.T, cb func(*Config)) (*Server, *structs.ACLToken,
 
 func TestServer(t *testing.T, cb func(*Config)) (*Server, func()) {
 	s, c, err := TestServerErr(t, cb)
-	require.NoError(t, err, "failed to start test server")
+	must.NoError(t, err, must.Sprint("failed to start test server"))
 	return s, c
 }
 
@@ -122,7 +123,7 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
 					defer close(ch)
 
 					// Shutdown server
-					err := server.Shutdown()
+					err = server.Shutdown()
 					if err != nil {
 						ch <- fmt.Errorf("failed to shutdown server: %w", err)
 					}
@@ -137,9 +138,7 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
 					t.Fatal("timed out while shutting down server")
 				}
 			}, nil
-		} else if i == 0 {
-			return nil, nil, err
-		} else {
+		} else if i > 0 {
 			if server != nil {
 				_ = server.Shutdown()
 			}
@@ -148,7 +147,7 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
 		}
 	}
 
-	return nil, nil, nil
+	return nil, nil, errors.New("unable to acquire ports for test server")
 }
 
 func TestJoin(t *testing.T, servers ...*Server) {

testutil/wait.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/kr/pretty"
 	"github.com/shoenig/test/must"
+	"github.com/shoenig/test/wait"
 )
 
 type testFn func() (bool, error)
@@ -142,6 +143,25 @@ func WaitForLeader(t testing.TB, rpc rpcFn) {
 	})
 }
 
+// WaitForLeaders blocks until each serverRPC knows the leader.
+func WaitForLeaders(t testing.TB, serverRPCs ...rpcFn) {
+	t.Helper()
+
+	for i := 0; i < len(serverRPCs); i++ {
+		ok := func() (bool, error) {
+			args := &structs.GenericRequest{}
+			var leader string
+			err := serverRPCs[i]("Status.Leader", args, &leader)
+			return leader != "", err
+		}
+		must.Wait(t, wait.InitialSuccess(
+			wait.TestFunc(ok),
+			wait.Timeout(10*time.Second),
+			wait.Gap(1*time.Second),
+		))
+	}
+}
+
 // WaitForClient blocks until the client can be found
 func WaitForClient(t testing.TB, rpc rpcFn, nodeID string, region string) {
 	t.Helper()