Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch all client nodes in dc2 to dataplane [NET-4299] #18608

Merged
merged 17 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test-integ/peering_commontopo/ac5_2_pq_failover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func (s *ac5_2PQFailoverSuite) testPreparedQuerySingleFailover(t *testing.T, ct

require.Equal(r, 1, queryResult.Failovers, "expected 1 prepared query failover")
require.Equal(r, peerClu.Name, queryResult.Nodes[0].Node.Datacenter, fmt.Sprintf("the pq results should originate from peer clu %s", peerClu.Name))
require.Equal(r, pqFailoverTargets[0].Peer, queryResult.Nodes[0].Checks[0].PeerName,
nfi-hashicorp marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(r, pqFailoverTargets[0].Peer, queryResult.Nodes[0].Node.PeerName,
fmt.Sprintf("pq results should come from the first failover target peer %s", pqFailoverTargets[0].Peer))
})
})
Expand Down
6 changes: 5 additions & 1 deletion test-integ/peering_commontopo/ac7_1_rotate_gw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,12 @@ func (s *suiteRotateGW) setup(t *testing.T, ct *commonTopo) {

// add a second mesh gateway "new"
s.newMGWNodeName = fmt.Sprintf("new-%s-default-mgw", clu.Name)
nodeKind := topology.NodeKindClient
if clu.Datacenter == agentlessDC {
nodeKind = topology.NodeKindDataplane
}
clu.Nodes = append(clu.Nodes, newTopologyMeshGatewaySet(
topology.NodeKindClient,
nodeKind,
"default",
s.newMGWNodeName,
1,
Expand Down
11 changes: 3 additions & 8 deletions test-integ/peering_commontopo/asserter.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type asserter struct {
type sprawlLite interface {
HTTPClientForCluster(clusterName string) (*http.Client, error)
APIClientForNode(clusterName string, nid topology.NodeID, token string) (*api.Client, error)
APIClientForCluster(clusterName string, token string) (*api.Client, error)
Topology() *topology.Topology
}

Expand All @@ -58,18 +59,12 @@ func (a *asserter) mustGetHTTPClient(t *testing.T, cluster string) *http.Client
}

func (a *asserter) mustGetAPIClient(t *testing.T, cluster string) *api.Client {
cl, err := a.apiClientFor(cluster)
clu := a.sp.Topology().Clusters[cluster]
cl, err := a.sp.APIClientForCluster(clu.Name, "")
require.NoError(t, err)
return cl
}

func (a *asserter) apiClientFor(cluster string) (*api.Client, error) {
clu := a.sp.Topology().Clusters[cluster]
// TODO: this always goes to the first client, but we might want to balance this
cl, err := a.sp.APIClientForNode(cluster, clu.FirstClient().ID(), "")
return cl, err
}

// httpClientFor returns a pre-configured http.Client that proxies requests
// through the embedded squid instance in each LAN.
//
Expand Down
35 changes: 19 additions & 16 deletions test-integ/peering_commontopo/commontopo.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ type commonTopo struct {
services map[string]map[topology.ServiceID]struct{}
}

const agentlessDC = "dc2"

func NewCommonTopo(t *testing.T) *commonTopo {
t.Helper()

Expand Down Expand Up @@ -84,11 +86,9 @@ func NewCommonTopo(t *testing.T) *commonTopo {
peerings = append(peerings, addPeerings(dc1, dc3)...)
peerings = append(peerings, addPeerings(dc2, dc3)...)

addMeshGateways(dc1, topology.NodeKindClient)
addMeshGateways(dc2, topology.NodeKindClient)
addMeshGateways(dc3, topology.NodeKindClient)
// TODO: consul-topology doesn't support this yet
// addMeshGateways(dc2, topology.NodeKindDataplane)
addMeshGateways(dc1)
addMeshGateways(dc2)
addMeshGateways(dc3)

setupGlobals(dc1)
setupGlobals(dc2)
Expand Down Expand Up @@ -131,7 +131,7 @@ func (ct *commonTopo) postLaunchChecks(t *testing.T) {
)

// check that exports line up as expected
for _, clu := range ct.Sprawl.Config().Clusters {
for _, clu := range ct.Sprawl.Topology().Clusters {
// expected exports per peer
type key struct {
peer string
Expand Down Expand Up @@ -191,9 +191,6 @@ func LocalPeerName(clu *topology.Cluster, partition string) string {
type serviceExt struct {
*topology.Service

// default NodeKindClient
NodeKind topology.NodeKind

Exports []api.ServiceConsumer
Config *api.ServiceConfigEntry
Intentions *api.ServiceIntentionsConfigEntry
Expand Down Expand Up @@ -227,8 +224,13 @@ func (ct *commonTopo) AddServiceNode(clu *topology.Cluster, svc serviceExt) *top
return n
}

nodeKind := topology.NodeKindClient
if clu.Datacenter == "dc2" {
nodeKind = topology.NodeKindDataplane
}

node := &topology.Node{
Kind: topology.NodeKindClient,
Kind: nodeKind,
Name: serviceHostnameString(clu.Datacenter, svc.ID),
Partition: svc.ID.Partition,
Addresses: []*topology.Address{
Expand All @@ -239,9 +241,6 @@ func (ct *commonTopo) AddServiceNode(clu *topology.Cluster, svc serviceExt) *top
},
Cluster: clusterName,
}
if svc.NodeKind != "" {
node.Kind = svc.NodeKind
}
clu.Nodes = append(clu.Nodes, node)

// Export if necessary
Expand All @@ -265,7 +264,7 @@ func (ct *commonTopo) AddServiceNode(clu *topology.Cluster, svc serviceExt) *top
}

func (ct *commonTopo) APIClientForCluster(t *testing.T, clu *topology.Cluster) *api.Client {
cl, err := ct.Sprawl.APIClientForNode(clu.Name, clu.FirstClient().ID(), "")
cl, err := ct.Sprawl.APIClientForCluster(clu.Name, "")
require.NoError(t, err)
return cl
}
Expand Down Expand Up @@ -372,10 +371,14 @@ func setupGlobals(clu *topology.Cluster) {

// addMeshGateways adds a mesh gateway for every partition in the cluster.
// Assumes that the LAN network name is equal to datacenter name.
func addMeshGateways(c *topology.Cluster, kind topology.NodeKind) {
func addMeshGateways(c *topology.Cluster) {
nodeKind := topology.NodeKindClient
if c.Datacenter == agentlessDC {
nodeKind = topology.NodeKindDataplane
}
for _, p := range c.Partitions {
c.Nodes = topology.MergeSlices(c.Nodes, newTopologyMeshGatewaySet(
kind,
nodeKind,
p.Name,
fmt.Sprintf("%s-%s-mgw", c.Name, p.Name),
1,
Expand Down
32 changes: 32 additions & 0 deletions testing/deployer/sprawl/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,13 +318,45 @@ func serviceToCatalogRegistration(
Address: node.LocalAddress(),
},
}
if svc.IsMeshGateway {
reg.Service.Kind = api.ServiceKindMeshGateway
reg.Service.Proxy = &api.AgentServiceConnectProxyConfig{
Config: map[string]interface{}{
"envoy_gateway_no_default_bind": true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I don't know what these do or if they're necessary, but consul connect envoy -mesh-gateway set them on registration.

Copy link
Member

@mkeeler mkeeler Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The no default bind one will not bind to the main service address and the bind tagged addrs one then instructs it to bind envoy listeners to all the tagged addrs instead.

You could omit the main service addr form the tagged addresses and use the combination by not specifying the no default bind attribute. Or if you are in k8s then you just bind to the services addr (pod address) and ignore all the tagged addrs.

Basically, the binding of a mgw to specific addrs and ports is very flexible and controlled by these proxy config settings.

"envoy_gateway_bind_tagged_addresses": true,
},
MeshGateway: api.MeshGatewayConfig{
Mode: api.MeshGatewayModeLocal,
},
}
}
if node.HasPublicAddress() {
reg.TaggedAddresses = map[string]string{
"lan": node.LocalAddress(),
"lan_ipv4": node.LocalAddress(),
"wan": node.PublicAddress(),
"wan_ipv4": node.PublicAddress(),
}
// TODO: not sure what the difference is between these, but with just the
Copy link
Member

@mkeeler mkeeler Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it being set in agentful mode?

Looking at the proxy config code I would expect node tagged addresses to generally be ignored for all proxies.

I think the only way to use tagged addresses for service mesh is to set the services tagged addresses like you have done here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared with an agentful mgw in DC1, I see that it has both TaggedAddresses and ServiceTaggedAddresses filled. Compared with an agentful service node in DC1, only TaggedAddresses. Agentless service in DC2 has neither (Service)TaggedAddresses, only Address and ServiceAddress (which makes some sense since it only has a LAN connection).

// top-level set, it appeared to not get set in either :/
reg.Service.TaggedAddresses = map[string]api.ServiceAddress{
"lan": {
Address: node.LocalAddress(),
Port: svc.Port,
},
"lan_ipv4": {
Address: node.LocalAddress(),
Port: svc.Port,
},
"wan": {
Address: node.PublicAddress(),
Port: svc.Port,
},
"wan_ipv4": {
Address: node.PublicAddress(),
Port: svc.Port,
},
}
}
if cluster.Enterprise {
reg.Partition = svc.ID.Partition
Expand Down
3 changes: 2 additions & 1 deletion testing/deployer/sprawl/internal/build/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ COPY --from=0 /bin/consul /bin/consul

// FROM hashicorp/consul-dataplane:latest
// COPY --from=busybox:uclibc /bin/sh /bin/sh
// TODO: busybox:latest doesn't work, see https://hashicorp.slack.com/archives/C03EUN3QF1C/p1691784078972959
const dockerfileDataplane = `
ARG DATAPLANE_IMAGE
FROM busybox:latest
FROM busybox:1.34
FROM ${DATAPLANE_IMAGE}
COPY --from=0 /bin/busybox /bin/busybox
USER 0:0
Expand Down
8 changes: 4 additions & 4 deletions testing/deployer/sprawl/internal/tfgen/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ import (
"github.com/hashicorp/consul/testing/deployer/topology"
)

func (g *Generator) generateAgentHCL(node *topology.Node) (string, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all unrecoverable errors, so I just panic

func (g *Generator) generateAgentHCL(node *topology.Node) string {
if !node.IsAgent() {
return "", fmt.Errorf("not an agent")
panic("generateAgentHCL only applies to agents")
}

cluster, ok := g.topology.Clusters[node.Cluster]
if !ok {
return "", fmt.Errorf("no such cluster: %s", node.Cluster)
panic(fmt.Sprintf("no such cluster: %s", node.Cluster))
}

var b HCLBuilder
Expand Down Expand Up @@ -167,7 +167,7 @@ func (g *Generator) generateAgentHCL(node *topology.Node) (string, error) {
}
}

return b.String(), nil
return b.String()
}

type HCLBuilder struct {
Expand Down
3 changes: 0 additions & 3 deletions testing/deployer/sprawl/internal/tfgen/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"os"
"path/filepath"
"strings"
"text/template"

"github.com/hashicorp/consul/testing/deployer/topology"
"github.com/hashicorp/consul/testing/deployer/util"
Expand Down Expand Up @@ -179,5 +178,3 @@ server IN A %s ; Consul server

return buf.Bytes()
}

var tfCorednsT = template.Must(template.ParseFS(content, "templates/container-coredns.tf.tmpl"))
3 changes: 3 additions & 0 deletions testing/deployer/sprawl/internal/tfgen/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
var invalidResourceName = regexp.MustCompile(`[^a-z0-9-]+`)

func DockerImageResourceName(image string) string {
if image == "" {
panic(`image must not be ""`)
}
return invalidResourceName.ReplaceAllLiteralString(image, "-")
}

Expand Down
Loading
Loading