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.

Sign up for GitHub

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

Jump to bottom

[NET-5217] [OSS] Derive sidecar proxy locality from parent service #18437

Merged
merged 2 commits into from
Aug 10, 2023
Merged

[NET-5217] [OSS] Derive sidecar proxy locality from parent service #18437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b005940
Add logging to locality policy application
zalimeni Aug 10, 2023
1c1a015
Inherit locality when registering sidecars
zalimeni Aug 10, 2023
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
3 changes: 3 additions & 0 deletions .changelog/18437.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
Inherit locality from services when registering sidecar proxies.
```
6 changes: 6 additions & 0 deletions agent/sidecar_service.go
View file
Open in desktop
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
Open in desktop
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
14 changes: 12 additions & 2 deletions agent/xds/endpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package xds
import (
"errors"
"fmt"
"github.com/hashicorp/go-hclog"
"strconv"

envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
Expand Down Expand Up @@ -136,6 +137,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.
endpoints, ok := cfgSnap.ConnectProxy.PreparedQueryEndpoints[uid]
if ok {
la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand All @@ -161,6 +163,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.
endpoints, ok := cfgSnap.ConnectProxy.DestinationGateways.Get(uid)
if ok {
la := makeLoadAssignment(
s.Logger,
cfgSnap,
name,
nil,
Expand Down Expand Up @@ -229,6 +232,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C
clusterName := connect.GatewaySNI(key.Datacenter, key.Partition, cfgSnap.Roots.TrustDomain)

la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand All @@ -246,6 +250,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C

clusterName := cfgSnap.ServerSNIFn(key.Datacenter, "")
la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -418,6 +423,7 @@ func (s *ResourceGenerator) endpointsFromServicesAndResolvers(
for subsetName, groups := range clusterEndpoints {
clusterName := connect.ServiceSNI(svc.Name, subsetName, svc.NamespaceOrDefault(), svc.PartitionOrDefault(), cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -455,6 +461,7 @@ func (s *ResourceGenerator) makeEndpointsForOutgoingPeeredServices(
groups := []loadAssignmentEndpointGroup{{Endpoints: serviceGroup.Nodes, OnlyPassing: false}}

la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -619,6 +626,7 @@ func (s *ResourceGenerator) makeUpstreamLoadAssignmentForPeerService(
return la, nil
}
la = makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand All @@ -641,6 +649,7 @@ func (s *ResourceGenerator) makeUpstreamLoadAssignmentForPeerService(
return nil, nil
}
la = makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -773,6 +782,7 @@ func (s *ResourceGenerator) endpointsFromDiscoveryChain(
}

la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
ti.PrioritizeByLocality,
Expand Down Expand Up @@ -861,7 +871,7 @@ type loadAssignmentEndpointGroup struct {
OverrideHealth envoy_core_v3.HealthStatus
}

func makeLoadAssignment(cfgSnap *proxycfg.ConfigSnapshot, clusterName string, policy *structs.DiscoveryPrioritizeByLocality, endpointGroups []loadAssignmentEndpointGroup, localKey proxycfg.GatewayKey) *envoy_endpoint_v3.ClusterLoadAssignment {
func makeLoadAssignment(logger hclog.Logger, cfgSnap *proxycfg.ConfigSnapshot, clusterName string, policy *structs.DiscoveryPrioritizeByLocality, endpointGroups []loadAssignmentEndpointGroup, localKey proxycfg.GatewayKey) *envoy_endpoint_v3.ClusterLoadAssignment {
cla := &envoy_endpoint_v3.ClusterLoadAssignment{
ClusterName: clusterName,
Endpoints: make([]*envoy_endpoint_v3.LocalityLbEndpoints, 0, len(endpointGroups)),
Expand All @@ -878,7 +888,7 @@ func makeLoadAssignment(cfgSnap *proxycfg.ConfigSnapshot, clusterName string, po
var priority uint32

for _, endpointGroup := range endpointGroups {
endpointsByLocality, err := groupedEndpoints(cfgSnap.ServiceLocality, policy, endpointGroup.Endpoints)
endpointsByLocality, err := groupedEndpoints(logger, cfgSnap.ServiceLocality, policy, endpointGroup.Endpoints)

if err != nil {
continue
Expand Down
3 changes: 3 additions & 0 deletions agent/xds/endpoints_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package xds

import (
"github.com/hashicorp/go-hclog"
"path/filepath"
"sort"
"testing"
Expand Down Expand Up @@ -213,6 +214,7 @@ func Test_makeLoadAssignment(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := makeLoadAssignment(
hclog.NewNullLogger(),
&proxycfg.ConfigSnapshot{ServiceLocality: tt.locality},
tt.clusterName,
nil,
Expand All @@ -223,6 +225,7 @@ func Test_makeLoadAssignment(t *testing.T) {

if tt.locality == nil {
got := makeLoadAssignment(
hclog.NewNullLogger(),
&proxycfg.ConfigSnapshot{ServiceLocality: &structs.Locality{Region: "us-west-1", Zone: "us-west-1a"}},
tt.clusterName,
nil,
Expand Down
6 changes: 4 additions & 2 deletions agent/xds/locality_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ package xds

import (
"fmt"
"github.com/hashicorp/go-hclog"

"github.com/hashicorp/consul/agent/structs"
)

func groupedEndpoints(locality *structs.Locality, policy *structs.DiscoveryPrioritizeByLocality, csns structs.CheckServiceNodes) ([]structs.CheckServiceNodes, error) {
func groupedEndpoints(logger hclog.Logger, locality *structs.Locality, policy *structs.DiscoveryPrioritizeByLocality, csns structs.CheckServiceNodes) ([]structs.CheckServiceNodes, error) {
switch {
case policy == nil || policy.Mode == "" || policy.Mode == "none":
return []structs.CheckServiceNodes{csns}, nil
case policy.Mode == "failover":
return prioritizeByLocalityFailover(locality, csns), nil
log := logger.Named("locality")
return prioritizeByLocalityFailover(log, locality, csns), nil
default:
return nil, fmt.Errorf("unexpected priortize-by-locality mode %q", policy.Mode)
}
Expand Down
3 changes: 2 additions & 1 deletion agent/xds/locality_policy_oss.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ package xds

import (
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/go-hclog"
)

func prioritizeByLocalityFailover(locality *structs.Locality, csns structs.CheckServiceNodes) []structs.CheckServiceNodes {
func prioritizeByLocalityFailover(_ hclog.Logger, _ *structs.Locality, _ structs.CheckServiceNodes) []structs.CheckServiceNodes {
return nil
}
Loading