Merge pull request #4584 from hashicorp/refactor_cluster_config (#16517)

NET-2841: PART 1 - refactor NewPeeringCluster to support custom config

test/integration/consul-container/libs/topology/peering_topology.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"github.com/testcontainers/testcontainers-go"
 
 	"github.com/hashicorp/consul/api"
 
@@ -43,16 +44,26 @@ func BasicPeeringTwoClustersSetup(
 	consulVersion string,
 	peeringThroughMeshgateway bool,
 ) (*BuiltCluster, *BuiltCluster) {
-	// acceptingCluster, acceptingCtx, acceptingClient := NewPeeringCluster(t, "dc1", 3, consulVersion, true)
+	acceptingCluster, acceptingCtx, acceptingClient := NewCluster(t, &ClusterConfig{
-	acceptingCluster, acceptingCtx, acceptingClient := NewPeeringCluster(t, 3, 1, &libcluster.BuildOptions{
+		NumServers: 3,
+		NumClients: 1,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:           "dc1",
 			ConsulVersion:        consulVersion,
 			InjectAutoEncryption: true,
+		},
+		ApplyDefaultProxySettings: true,
 	})
-	dialingCluster, dialingCtx, dialingClient := NewPeeringCluster(t, 1, 1, &libcluster.BuildOptions{
+
+	dialingCluster, dialingCtx, dialingClient := NewCluster(t, &ClusterConfig{
+		NumServers: 1,
+		NumClients: 1,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:           "dc2",
 			ConsulVersion:        consulVersion,
 			InjectAutoEncryption: true,
+		},
+		ApplyDefaultProxySettings: true,
 	})
 
 	// Create the mesh gateway for dataplane traffic and peering control plane traffic (if enabled)
@@ -151,92 +162,68 @@ func BasicPeeringTwoClustersSetup(
 		}
 }
 
-// NewDialingCluster creates a cluster for peering with a single dev agent
+type ClusterConfig struct {
-// TODO: note: formerly called CreatingPeeringClusterAndSetup
+	NumServers                int
-//
+	NumClients                int
-// Deprecated: use NewPeeringCluster mostly
+	ApplyDefaultProxySettings bool
-func NewDialingCluster(
+	BuildOpts                 *libcluster.BuildOptions
-	t *testing.T,
+	Cmd                       string
-	version string,
+	LogConsumer               *TestLogConsumer
-	dialingPeerName string,
+	Ports                     []int
-) (*libcluster.Cluster, *api.Client, libservice.Service) {
-	t.Helper()
-	t.Logf("creating the dialing cluster")
-
-	opts := libcluster.BuildOptions{
-		Datacenter:             "dc2",
-		InjectAutoEncryption:   true,
-		InjectGossipEncryption: true,
-		AllowHTTPAnyway:        true,
-		ConsulVersion:          version,
-	}
-	ctx := libcluster.NewBuildContext(t, opts)
-
-	conf := libcluster.NewConfigBuilder(ctx).
-		Peering(true).
-		ToAgentConfig(t)
-	t.Logf("dc2 server config: \n%s", conf.JSON)
-
-	cluster, err := libcluster.NewN(t, *conf, 1)
-	require.NoError(t, err)
-
-	node := cluster.Agents[0]
-	client := node.GetClient()
-	libcluster.WaitForLeader(t, cluster, client)
-	libcluster.WaitForMembers(t, client, 1)
-
-	// Default Proxy Settings
-	ok, err := utils.ApplyDefaultProxySettings(client)
-	require.NoError(t, err)
-	require.True(t, ok)
-
-	// Create the mesh gateway for dataplane traffic
-	_, err = libservice.NewGatewayService(context.Background(), "mesh", "mesh", node)
-	require.NoError(t, err)
-
-	// Create a service and proxy instance
-	clientProxyService, err := libservice.CreateAndRegisterStaticClientSidecar(node, dialingPeerName, true)
-	require.NoError(t, err)
-
-	libassert.CatalogServiceExists(t, client, "static-client-sidecar-proxy", nil)
-
-	return cluster, client, clientProxyService
 }
 
-// NewPeeringCluster creates a cluster with peering enabled. It also creates
+// NewCluster creates a cluster with peering enabled. It also creates
 // and registers a mesh-gateway at the client agent. The API client returned is
 // pointed at the client agent.
 // - proxy-defaults.protocol = tcp
-func NewPeeringCluster(
+func NewCluster(
 	t *testing.T,
-	numServers int,
+	config *ClusterConfig,
-	numClients int,
-	buildOpts *libcluster.BuildOptions,
 ) (*libcluster.Cluster, *libcluster.BuildContext, *api.Client) {
-	require.NotEmpty(t, buildOpts.Datacenter)
+	var (
-	require.True(t, numServers > 0)
+		cluster *libcluster.Cluster
+		err     error
+	)
+	require.NotEmpty(t, config.BuildOpts.Datacenter)
+	require.True(t, config.NumServers > 0)
 
 	opts := libcluster.BuildOptions{
-		Datacenter:             buildOpts.Datacenter,
+		Datacenter:             config.BuildOpts.Datacenter,
-		InjectAutoEncryption:   buildOpts.InjectAutoEncryption,
+		InjectAutoEncryption:   config.BuildOpts.InjectAutoEncryption,
 		InjectGossipEncryption: true,
 		AllowHTTPAnyway:        true,
-		ConsulVersion:          buildOpts.ConsulVersion,
+		ConsulVersion:          config.BuildOpts.ConsulVersion,
-		ACLEnabled:             buildOpts.ACLEnabled,
+		ACLEnabled:             config.BuildOpts.ACLEnabled,
 	}
 	ctx := libcluster.NewBuildContext(t, opts)
 
 	serverConf := libcluster.NewConfigBuilder(ctx).
-		Bootstrap(numServers).
+		Bootstrap(config.NumServers).
 		Peering(true).
 		ToAgentConfig(t)
 	t.Logf("%s server config: \n%s", opts.Datacenter, serverConf.JSON)
 
-	cluster, err := libcluster.NewN(t, *serverConf, numServers)
+	// optional
+	if config.LogConsumer != nil {
+		serverConf.LogConsumer = config.LogConsumer
+	}
+
+	t.Logf("Cluster config:\n%s", serverConf.JSON)
+
+	// optional custom cmd
+	if config.Cmd != "" {
+		serverConf.Cmd = append(serverConf.Cmd, config.Cmd)
+	}
+
+	if config.Ports != nil {
+		cluster, err = libcluster.New(t, []libcluster.Config{*serverConf}, config.Ports...)
+	} else {
+		cluster, err = libcluster.NewN(t, *serverConf, config.NumServers)
+	}
 	require.NoError(t, err)
 
 	var retryJoin []string
-	for i := 0; i < numServers; i++ {
+	for i := 0; i < config.NumServers; i++ {
 		retryJoin = append(retryJoin, fmt.Sprintf("agent-%d", i))
 	}
 
@@ -248,18 +235,33 @@ func NewPeeringCluster(
 	clientConf := configbuiilder.ToAgentConfig(t)
 	t.Logf("%s client config: \n%s", opts.Datacenter, clientConf.JSON)
 
-	require.NoError(t, cluster.AddN(*clientConf, numClients, true))
+	require.NoError(t, cluster.AddN(*clientConf, config.NumClients, true))
 
 	// Use the client agent as the HTTP endpoint since we will not rotate it in many tests.
-	clientNode := cluster.Agents[numServers]
+	var client *api.Client
-	client := clientNode.GetClient()
+	if config.NumClients > 0 {
+		clientNode := cluster.Agents[config.NumServers]
+		client = clientNode.GetClient()
+	} else {
+		client = cluster.Agents[0].GetClient()
+	}
 	libcluster.WaitForLeader(t, cluster, client)
-	libcluster.WaitForMembers(t, client, numServers+numClients)
+	libcluster.WaitForMembers(t, client, config.NumServers+config.NumClients)
 
 	// Default Proxy Settings
+	if config.ApplyDefaultProxySettings {
 		ok, err := utils.ApplyDefaultProxySettings(client)
 		require.NoError(t, err)
 		require.True(t, ok)
+	}
 
 	return cluster, ctx, client
 }
+
+type TestLogConsumer struct {
+	Msgs []string
+}
+
+func (g *TestLogConsumer) Accept(l testcontainers.Log) {
+	g.Msgs = append(g.Msgs, string(l.Content))
+}

test/integration/consul-container/test/basic/connect_service_test.go

@@ -24,14 +24,18 @@ import (
 func TestBasicConnectService(t *testing.T) {
 	t.Parallel()
 
-	buildOptions := &libcluster.BuildOptions{
+	cluster, _, _ := topology.NewCluster(t, &topology.ClusterConfig{
+		NumServers:                1,
+		NumClients:                1,
+		ApplyDefaultProxySettings: true,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:             "dc1",
 			InjectAutoEncryption:   true,
 			InjectGossipEncryption: true,
 			// TODO(rb): fix the test to not need the service/envoy stack to use :8500
 			AllowHTTPAnyway: true,
-	}
+		},
-	cluster, _, _ := topology.NewPeeringCluster(t, 1, 1, buildOptions)
+	})
 
 	clientService := createServices(t, cluster)
 	_, port := clientService.GetAddr()

test/integration/consul-container/test/common.go

@@ -1,70 +0,0 @@
-package test
-
-import (
-	"testing"
-
-	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
-	"github.com/hashicorp/consul/test/integration/consul-container/libs/utils"
-	"github.com/stretchr/testify/require"
-	"github.com/testcontainers/testcontainers-go"
-)
-
-type TestLogConsumer struct {
-	Msgs []string
-}
-
-func (g *TestLogConsumer) Accept(l testcontainers.Log) {
-	g.Msgs = append(g.Msgs, string(l.Content))
-}
-
-// Creates a cluster with options for basic customization. All args except t
-// are optional and will use sensible defaults when not provided.
-func CreateCluster(
-	t *testing.T,
-	cmd string,
-	logConsumer *TestLogConsumer,
-	buildOptions *libcluster.BuildOptions,
-	applyDefaultProxySettings bool,
-	ports ...int,
-) *libcluster.Cluster {
-
-	// optional
-	if buildOptions == nil {
-		buildOptions = &libcluster.BuildOptions{
-			InjectAutoEncryption:   true,
-			InjectGossipEncryption: true,
-		}
-	}
-	ctx := libcluster.NewBuildContext(t, *buildOptions)
-
-	conf := libcluster.NewConfigBuilder(ctx).ToAgentConfig(t)
-
-	// optional
-	if logConsumer != nil {
-		conf.LogConsumer = logConsumer
-	}
-
-	t.Logf("Cluster config:\n%s", conf.JSON)
-
-	// optional custom cmd
-	if cmd != "" {
-		conf.Cmd = append(conf.Cmd, cmd)
-	}
-
-	cluster, err := libcluster.New(t, []libcluster.Config{*conf}, ports...)
-	require.NoError(t, err)
-
-	node := cluster.Agents[0]
-	client := node.GetClient()
-
-	libcluster.WaitForLeader(t, cluster, client)
-	libcluster.WaitForMembers(t, client, 1)
-
-	if applyDefaultProxySettings {
-		ok, err := utils.ApplyDefaultProxySettings(client)
-		require.NoError(t, err)
-		require.True(t, ok)
-	}
-
-	return cluster
-}

test/integration/consul-container/test/gateways/gateway_endpoint_test.go

@@ -14,8 +14,8 @@ import (
 	libassert "github.com/hashicorp/consul/test/integration/consul-container/libs/assert"
 	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
 	libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
+	libtopology "github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
 	"github.com/hashicorp/consul/test/integration/consul-container/libs/utils"
-	"github.com/hashicorp/consul/test/integration/consul-container/test"
 	"github.com/hashicorp/go-cleanhttp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -31,13 +31,20 @@ func TestAPIGatewayCreate(t *testing.T) {
 	t.Parallel()
 	listenerPortOne := 6000
 
-	buildOpts := &libcluster.BuildOptions{
+	clusterConfig := &libtopology.ClusterConfig{
+		NumServers: 1,
+		NumClients: 1,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:             "dc1",
 			InjectAutoEncryption:   true,
 			InjectGossipEncryption: true,
 			AllowHTTPAnyway:        true,
+		},
+		Ports:                     []int{listenerPortOne},
+		ApplyDefaultProxySettings: true,
 	}
-	cluster := test.CreateCluster(t, "", nil, buildOpts, true, listenerPortOne)
+
+	cluster, _, _ := libtopology.NewCluster(t, clusterConfig)
 	client := cluster.APIClient(0)
 
 	// add api gateway config

test/integration/consul-container/test/gateways/http_route_test.go

@@ -5,16 +5,15 @@ import (
 	"crypto/rand"
 	"encoding/hex"
 	"fmt"
-	"testing"
-	"time"
-
 	"github.com/hashicorp/consul/api"
 	libassert "github.com/hashicorp/consul/test/integration/consul-container/libs/assert"
 	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
 	libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
-	"github.com/hashicorp/consul/test/integration/consul-container/test"
+	libtopology "github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"testing"
+	"time"
 )
 
 func getNamespace() string {
@@ -43,14 +42,21 @@ func TestHTTPRouteFlattening(t *testing.T) {
 
 	//infrastructure set up
 	listenerPort := 6000
-	//create cluster
+
-	buildOpts := &libcluster.BuildOptions{
+	clusterConfig := &libtopology.ClusterConfig{
+		NumServers: 1,
+		NumClients: 1,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:             "dc1",
 			InjectAutoEncryption:   true,
 			InjectGossipEncryption: true,
 			AllowHTTPAnyway:        true,
+		},
+		Ports:                     []int{listenerPort},
+		ApplyDefaultProxySettings: true,
 	}
-	cluster := test.CreateCluster(t, "", nil, buildOpts, true, listenerPort)
+
+	cluster, _, _ := libtopology.NewCluster(t, clusterConfig)
 	client := cluster.Agents[0].GetClient()
 
 	service1ResponseCode := 200

test/integration/consul-container/test/observability/access_logs_test.go

@@ -45,9 +45,14 @@ func TestAccessLogs(t *testing.T) {
 		t.Skip()
 	}
 
-	cluster, _, _ := topology.NewPeeringCluster(t, 1, 1, &libcluster.BuildOptions{
+	cluster, _, _ := topology.NewCluster(t, &topology.ClusterConfig{
+		NumServers:                1,
+		NumClients:                1,
+		ApplyDefaultProxySettings: true,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:           "dc1",
 			InjectAutoEncryption: true,
+		},
 	})
 
 	// Turn on access logs. Do this before starting the sidecars so that they inherit the configuration

test/integration/consul-container/test/ratelimit/ratelimit_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
-	"github.com/hashicorp/consul/test/integration/consul-container/test"
+	libtopology "github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
 )
 
 const (
@@ -128,8 +128,20 @@ func TestServerRequestRateLimit(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			logConsumer := &test.TestLogConsumer{}
+			clusterConfig := &libtopology.ClusterConfig{
-			cluster := test.CreateCluster(t, tc.cmd, logConsumer, nil, false)
+				NumServers:  1,
+				NumClients:  0,
+				Cmd:         tc.cmd,
+				LogConsumer: &libtopology.TestLogConsumer{},
+				BuildOpts: &libcluster.BuildOptions{
+					Datacenter:             "dc1",
+					InjectAutoEncryption:   true,
+					InjectGossipEncryption: true,
+				},
+				ApplyDefaultProxySettings: false,
+			}
+
+			cluster, _, _ := libtopology.NewCluster(t, clusterConfig)
 			defer terminate(t, cluster)
 
 			client, err := cluster.GetClient(nil, true)
@@ -137,7 +149,7 @@ func TestServerRequestRateLimit(t *testing.T) {
 
 			// perform actions and validate returned errors to client
 			for _, op := range tc.operations {
-				err = op.action.function(client)
+				err := op.action.function(client)
 				if len(op.expectedErrorMsg) > 0 {
 					require.Error(t, err)
 					require.Equal(t, op.expectedErrorMsg, err.Error())
@@ -165,7 +177,7 @@ func TestServerRequestRateLimit(t *testing.T) {
 					// validate logs
 					// putting this last as there are cases where logs
 					// were not present in consumer when assertion was made.
-					checkLogsForMessage(r, logConsumer.Msgs,
+					checkLogsForMessage(r, clusterConfig.LogConsumer.Msgs,
 						fmt.Sprintf("[DEBUG] agent.server.rpc-rate-limit: RPC exceeded allowed rate limit: rpc=%s", op.action.rateLimitOperation),
 						op.action.rateLimitOperation, "exceeded", op.expectExceededLog)
 

test/integration/consul-container/test/troubleshoot/troubleshoot_test.go

@@ -18,9 +18,14 @@ import (
 
 func TestTroubleshootProxy(t *testing.T) {
 	t.Parallel()
-	cluster, _, _ := topology.NewPeeringCluster(t, 1, 1, &libcluster.BuildOptions{
+	cluster, _, _ := topology.NewCluster(t, &topology.ClusterConfig{
+		NumServers: 1,
+		NumClients: 1,
+		BuildOpts: &libcluster.BuildOptions{
 			Datacenter:           "dc1",
 			InjectAutoEncryption: true,
+		},
+		ApplyDefaultProxySettings: true,
 	})
 
 	serverService, clientService := topology.CreateServices(t, cluster)

test/integration/consul-container/test/upgrade/acl_node_test.go

@@ -36,11 +36,16 @@ func TestACL_Upgrade_Node_Token(t *testing.T) {
 
 	run := func(t *testing.T, tc testcase) {
 		// NOTE: Disable auto.encrypt due to its conflict with ACL token during bootstrap
-		cluster, _, _ := libtopology.NewPeeringCluster(t, 1, 1, &libcluster.BuildOptions{
+		cluster, _, _ := libtopology.NewCluster(t, &libtopology.ClusterConfig{
+			NumServers: 1,
+			NumClients: 1,
+			BuildOpts: &libcluster.BuildOptions{
 				Datacenter:           "dc1",
 				ConsulVersion:        tc.oldversion,
 				InjectAutoEncryption: false,
 				ACLEnabled:           true,
+			},
+			ApplyDefaultProxySettings: true,
 		})
 
 		agentToken, err := cluster.CreateAgentToken("dc1",

test/integration/consul-container/test/upgrade/healthcheck_test.go

@@ -1,6 +1,7 @@
 package upgrade
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -9,6 +10,7 @@ import (
 	"github.com/hashicorp/consul/api"
 	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
 	libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
+	libtopology "github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
 	"github.com/hashicorp/consul/test/integration/consul-container/libs/utils"
 )
 
@@ -21,9 +23,22 @@ func TestTargetServersWithLatestGAClients(t *testing.T) {
 		numClients = 1
 	)
 
-	cluster := serversCluster(t, numServers, utils.TargetImageName, utils.TargetVersion)
+	clusterConfig := &libtopology.ClusterConfig{
+		NumServers: numServers,
+		NumClients: numClients,
+		BuildOpts: &libcluster.BuildOptions{
+			Datacenter:    "dc1",
+			ConsulVersion: utils.TargetVersion,
+		},
+		ApplyDefaultProxySettings: true,
+	}
 
-	libservice.ClientsCreate(t, numClients, utils.LatestImageName, utils.LatestVersion, cluster)
+	cluster, _, _ := libtopology.NewCluster(t, clusterConfig)
+
+	// change the version of Client agent to latest version
+	config := cluster.Agents[3].GetConfig()
+	config.Version = utils.LatestVersion
+	cluster.Agents[3].Upgrade(context.Background(), config)
 
 	client := cluster.APIClient(0)
 

test/integration/consul-container/test/upgrade/ingress_gateway_test.go

@@ -49,10 +49,15 @@ func TestIngressGateway_UpgradeToTarget_fromLatest(t *testing.T) {
 	run := func(t *testing.T, oldVersion, targetVersion string) {
 		// setup
 		// TODO? we don't need a peering cluster, so maybe this is overkill
-		cluster, _, client := topology.NewPeeringCluster(t, 1, 2, &libcluster.BuildOptions{
+		cluster, _, client := topology.NewCluster(t, &topology.ClusterConfig{
+			NumServers: 1,
+			NumClients: 2,
+			BuildOpts: &libcluster.BuildOptions{
 				Datacenter:    "dc1",
 				ConsulVersion: oldVersion,
 				// TODO? InjectAutoEncryption: true,
+			},
+			ApplyDefaultProxySettings: true,
 		})
 
 		// upsert config entry making http default protocol for global

test/integration/consul-container/test/upgrade/l7_traffic_management/resolver_default_subset_test.go

@@ -335,7 +335,12 @@ func TestTrafficManagement_ServiceResolver(t *testing.T) {
 		if oldVersionTmp.LessThan(libutils.Version_1_14) {
 			buildOpts.InjectAutoEncryption = false
 		}
-		cluster, _, _ := topology.NewPeeringCluster(t, 1, 1, buildOpts)
+		cluster, _, _ := topology.NewCluster(t, &topology.ClusterConfig{
+			NumServers:                1,
+			NumClients:                1,
+			BuildOpts:                 buildOpts,
+			ApplyDefaultProxySettings: true,
+		})
 		node := cluster.Agents[0]
 		client := node.GetClient()