Skip to content

Commit

Permalink
Inherit locality when registering sidecars
Browse files Browse the repository at this point in the history 
When sidecar locality is not explicitly configured, inherit locality
from the proxied service.
  • Loading branch information
@zalimeni
zalimeni committed Aug 10, 2023
1 parent 845b71e commit 5e5cc95
Show file tree
Hide file tree
Showing 5 changed files with 249 additions and 94 deletions.
6 changes: 6 additions & 0 deletions agent/sidecar_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ func sidecarServiceFromNodeService(ns *structs.NodeService, token string) (*stru
sidecar.Tags = append(sidecar.Tags, ns.Tags...)
}

// Copy the locality from the original service if locality was not provided
if sidecar.Locality == nil && ns.Locality != nil {
tmp := *ns.Locality
sidecar.Locality = &tmp
}

// Flag this as a sidecar - this is not persisted in catalog but only needed
// in local agent state to disambiguate lineage when deregistering the parent
// service later.
Expand Down
69 changes: 61 additions & 8 deletions agent/sidecar_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,25 +134,78 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
wantToken: "custom-token",
},
{
name: "inherit tags and meta",
name: "inherit locality, tags and meta",
sd: &structs.ServiceDefinition{
ID: "web1",
Name: "web",
Port: 1111,
Tags: []string{"foo"},
Meta: map[string]string{"foo": "bar"},
Locality: &structs.Locality{
Region: "us-east-1",
Zone: "us-east-1a",
},
Connect: &structs.ServiceConnect{
SidecarService: &structs.ServiceDefinition{},
},
},
wantNS: &structs.NodeService{
EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
Kind: structs.ServiceKindConnectProxy,
ID: "web1-sidecar-proxy",
Service: "web-sidecar-proxy",
Port: 0,
Tags: []string{"foo"},
Meta: map[string]string{"foo": "bar"},
EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
Kind: structs.ServiceKindConnectProxy,
ID: "web1-sidecar-proxy",
Service: "web-sidecar-proxy",
Port: 0,
Tags: []string{"foo"},
Meta: map[string]string{"foo": "bar"},
Locality: &structs.Locality{
Region: "us-east-1",
Zone: "us-east-1a",
},
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
DestinationServiceID: "web1",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 1111,
},
},
wantChecks: nil,
},
{
name: "retain locality, tags and meta if explicitly configured",
sd: &structs.ServiceDefinition{
ID: "web1",
Name: "web",
Port: 1111,
Tags: []string{"foo"},
Meta: map[string]string{"foo": "bar"},
Locality: &structs.Locality{
Region: "us-east-1",
Zone: "us-east-1a",
},
Connect: &structs.ServiceConnect{
SidecarService: &structs.ServiceDefinition{
Tags: []string{"bar"},
Meta: map[string]string{"baz": "qux"},
Locality: &structs.Locality{
Region: "us-east-2",
Zone: "us-east-2a",
},
},
},
},
wantNS: &structs.NodeService{
EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
Kind: structs.ServiceKindConnectProxy,
ID: "web1-sidecar-proxy",
Service: "web-sidecar-proxy",
Port: 0,
Tags: []string{"bar"},
Meta: map[string]string{"baz": "qux"},
Locality: &structs.Locality{
Region: "us-east-2",
Zone: "us-east-2a",
},
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
Expand Down
156 changes: 86 additions & 70 deletions command/connect/envoy/envoy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,15 +344,16 @@ func (c *cmd) run(args []string) int {
}
}

var svcForSidecar api.AgentService
if c.proxyID == "" {
switch {
case c.sidecarFor != "":
proxyID, err := proxyCmd.LookupProxyIDForSidecar(c.client, c.sidecarFor)
svcForSidecar, err := proxyCmd.LookupServiceForSidecar(c.client, c.sidecarFor)
if err != nil {
c.UI.Error(err.Error())
return 1
}
c.proxyID = proxyID
c.proxyID = svcForSidecar.ID

case c.gateway != "" && !c.register:
gatewaySvc, err := proxyCmd.LookupGatewayProxy(c.client, c.gatewayKind)
Expand Down Expand Up @@ -394,77 +395,13 @@ func (c *cmd) run(args []string) int {
return 1
}

taggedAddrs := make(map[string]api.ServiceAddress)
lanAddr := c.lanAddress.Value()
if lanAddr.Address != "" {
taggedAddrs[structs.TaggedAddressLAN] = lanAddr
}

wanAddr := c.wanAddress.Value()
if wanAddr.Address != "" {
taggedAddrs[structs.TaggedAddressWAN] = wanAddr
}

tcpCheckAddr := lanAddr.Address
if tcpCheckAddr == "" {
// fallback to localhost as the gateway has to reside in the same network namespace
// as the agent
tcpCheckAddr = "127.0.0.1"
}

var proxyConf *api.AgentServiceConnectProxyConfig
if len(c.bindAddresses.value) > 0 {
// override all default binding rules and just bind to the user-supplied addresses
proxyConf = &api.AgentServiceConnectProxyConfig{
Config: map[string]interface{}{
"envoy_gateway_no_default_bind": true,
"envoy_gateway_bind_addresses": c.bindAddresses.value,
},
}
} else if canBind(lanAddr) && canBind(wanAddr) {
// when both addresses are bindable then we bind to the tagged addresses
// for creating the envoy listeners
proxyConf = &api.AgentServiceConnectProxyConfig{
Config: map[string]interface{}{
"envoy_gateway_no_default_bind": true,
"envoy_gateway_bind_tagged_addresses": true,
},
}
} else if !canBind(lanAddr) && lanAddr.Address != "" {
c.UI.Error(fmt.Sprintf("The LAN address %q will not be bindable. Either set a bindable address or override the bind addresses with -bind-address", lanAddr.Address))
svc, err := c.proxyRegistration(&svcForSidecar)
if err != nil {
c.UI.Error(err.Error())
return 1
}

var meta map[string]string
if c.exposeServers {
meta = map[string]string{structs.MetaWANFederationKey: "1"}
}

// API gateways do not have a default listener or ready endpoint,
// so adding any check to the registration will fail
var check *api.AgentServiceCheck
if c.gatewayKind != api.ServiceKindAPIGateway {
check = &api.AgentServiceCheck{
Name: fmt.Sprintf("%s listening", c.gatewayKind),
TCP: ipaddr.FormatAddressPort(tcpCheckAddr, lanAddr.Port),
Interval: "10s",
DeregisterCriticalServiceAfter: c.deregAfterCritical,
}
}

svc := api.AgentServiceRegistration{
Kind: c.gatewayKind,
Name: c.gatewaySvcName,
ID: c.proxyID,
Address: lanAddr.Address,
Port: lanAddr.Port,
Meta: meta,
TaggedAddresses: taggedAddrs,
Proxy: proxyConf,
Check: check,
}

if err := c.client.Agent().ServiceRegister(&svc); err != nil {
if err := c.client.Agent().ServiceRegister(svc); err != nil {
c.UI.Error(fmt.Sprintf("Error registering service %q: %s", svc.Name, err))
return 1
}
Expand Down Expand Up @@ -542,6 +479,85 @@ func (c *cmd) run(args []string) int {
return 0
}

func (c *cmd) proxyRegistration(svcForSidecar *api.AgentService) (*api.AgentServiceRegistration, error) {
taggedAddrs := make(map[string]api.ServiceAddress)
lanAddr := c.lanAddress.Value()
if lanAddr.Address != "" {
taggedAddrs[structs.TaggedAddressLAN] = lanAddr
}

wanAddr := c.wanAddress.Value()
if wanAddr.Address != "" {
taggedAddrs[structs.TaggedAddressWAN] = wanAddr
}

tcpCheckAddr := lanAddr.Address
if tcpCheckAddr == "" {
// fallback to localhost as the gateway has to reside in the same network namespace
// as the agent
tcpCheckAddr = "127.0.0.1"
}

var proxyConf *api.AgentServiceConnectProxyConfig
if len(c.bindAddresses.value) > 0 {
// override all default binding rules and just bind to the user-supplied addresses
proxyConf = &api.AgentServiceConnectProxyConfig{
Config: map[string]interface{}{
"envoy_gateway_no_default_bind": true,
"envoy_gateway_bind_addresses": c.bindAddresses.value,
},
}
} else if canBind(lanAddr) && canBind(wanAddr) {
// when both addresses are bindable then we bind to the tagged addresses
// for creating the envoy listeners
proxyConf = &api.AgentServiceConnectProxyConfig{
Config: map[string]interface{}{
"envoy_gateway_no_default_bind": true,
"envoy_gateway_bind_tagged_addresses": true,
},
}
} else if !canBind(lanAddr) && lanAddr.Address != "" {
return nil, fmt.Errorf("The LAN address %q will not be bindable. Either set a bindable address or override the bind addresses with -bind-address", lanAddr.Address)
}

var meta map[string]string
if c.exposeServers {
meta = map[string]string{structs.MetaWANFederationKey: "1"}
}

// API gateways do not have a default listener or ready endpoint,
// so adding any check to the registration will fail
var check *api.AgentServiceCheck
if c.gatewayKind != api.ServiceKindAPIGateway {
check = &api.AgentServiceCheck{
Name: fmt.Sprintf("%s listening", c.gatewayKind),
TCP: ipaddr.FormatAddressPort(tcpCheckAddr, lanAddr.Port),
Interval: "10s",
DeregisterCriticalServiceAfter: c.deregAfterCritical,
}
}

// If registering a sidecar for an existing service, inherit the
// locality of that service if it was explicitly configured.
var locality *api.Locality
if c.sidecarFor != "" {
locality = svcForSidecar.Locality
}

return &api.AgentServiceRegistration{
Kind: c.gatewayKind,
Name: c.gatewaySvcName,
ID: c.proxyID,
Address: lanAddr.Address,
Port: lanAddr.Port,
Meta: meta,
TaggedAddresses: taggedAddrs,
Proxy: proxyConf,
Check: check,
Locality: locality,
}, nil
}

var errUnsupportedOS = errors.New("envoy: not implemented on this operating system")

func (c *cmd) findBinary() (string, error) {
Expand Down
77 changes: 77 additions & 0 deletions command/connect/envoy/envoy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,83 @@ func TestEnvoy_GatewayRegistration(t *testing.T) {
}
}

func TestEnvoy_proxyRegistration(t *testing.T) {
t.Parallel()

type args struct {
svcForProxy api.AgentService
cmdFn func(*cmd)
}

cases := []struct {
name string
args args
testFn func(*testing.T, args, *api.AgentServiceRegistration)
}{
{
"locality is inherited from proxied service if configured and using sidecarFor",
args{
svcForProxy: api.AgentService{
ID: "my-svc",
Locality: &api.Locality{
Region: "us-east-1",
Zone: "us-east-1a",
},
},
cmdFn: func(c *cmd) {
c.sidecarFor = "my-svc"
},
},
func(t *testing.T, args args, r *api.AgentServiceRegistration) {
assert.NotNil(t, r.Locality)
assert.Equal(t, args.svcForProxy.Locality, r.Locality)
},
},
{
"locality is not inherited if not using sidecarFor",
args{
svcForProxy: api.AgentService{
ID: "my-svc",
Locality: &api.Locality{
Region: "us-east-1",
Zone: "us-east-1a",
},
},
},
func(t *testing.T, args args, r *api.AgentServiceRegistration) {
assert.Nil(t, r.Locality)
},
},
{
"locality is not set if not configured for proxied service",
args{
svcForProxy: api.AgentService{},
cmdFn: func(c *cmd) {
c.sidecarFor = "my-svc"
},
},
func(t *testing.T, args args, r *api.AgentServiceRegistration) {
assert.Nil(t, r.Locality)
},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ui := cli.NewMockUi()
c := New(ui)

if tc.args.cmdFn != nil {
tc.args.cmdFn(c)
}

result, err := c.proxyRegistration(&tc.args.svcForProxy)
assert.NoError(t, err)
tc.testFn(t, tc.args, result)
})
}
}

// testMockAgent combines testMockAgentProxyConfig and testMockAgentSelf,
// routing /agent/service/... requests to testMockAgentProxyConfig,
// routing /catalog/node-services/... requests to testMockCatalogNodeServiceList
Expand Down
Loading

0 comments on commit 5e5cc95

Please sign in to comment.