Skip to content

Commit

Permalink
Revert "[NET-5217] [OSS] Derive sidecar proxy locality from parent se…
Browse files Browse the repository at this point in the history 
…rvice (#18437)"

This reverts commit 05604ee.
  • Loading branch information
@absolutelightning
absolutelightning committed Sep 13, 2023
1 parent d297523 commit d7c568e
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 271 deletions.
3 changes: 0 additions & 3 deletions .changelog/18437.txt
View file

This file was deleted.

6 changes: 0 additions & 6 deletions agent/sidecar_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,6 @@ 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: 8 additions & 61 deletions agent/sidecar_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,78 +134,25 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) {
wantToken: "custom-token",
},
{
name: "inherit locality, tags and meta",
name: "inherit 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"},
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",
},
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"},
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
Expand Down
13 changes: 2 additions & 11 deletions agent/xds/endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.
endpoints, ok := cfgSnap.ConnectProxy.PreparedQueryEndpoints[uid]
if ok {
la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand All @@ -164,7 +163,6 @@ 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 @@ -233,7 +231,6 @@ 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 @@ -251,7 +248,6 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C

clusterName := cfgSnap.ServerSNIFn(key.Datacenter, "")
la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -424,7 +420,6 @@ 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 @@ -462,7 +457,6 @@ func (s *ResourceGenerator) makeEndpointsForOutgoingPeeredServices(
groups := []loadAssignmentEndpointGroup{{Endpoints: serviceGroup.Nodes, OnlyPassing: false}}

la := makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -627,7 +621,6 @@ func (s *ResourceGenerator) makeUpstreamLoadAssignmentForPeerService(
return la, nil
}
la = makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand All @@ -650,7 +643,6 @@ func (s *ResourceGenerator) makeUpstreamLoadAssignmentForPeerService(
return nil, nil
}
la = makeLoadAssignment(
s.Logger,
cfgSnap,
clusterName,
nil,
Expand Down Expand Up @@ -785,7 +777,6 @@ func (s *ResourceGenerator) endpointsFromDiscoveryChain(
}

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

func makeLoadAssignment(logger hclog.Logger, cfgSnap *proxycfg.ConfigSnapshot, clusterName string, policy *structs.DiscoveryPrioritizeByLocality, endpointGroups []loadAssignmentEndpointGroup, localKey proxycfg.GatewayKey) *envoy_endpoint_v3.ClusterLoadAssignment {
func makeLoadAssignment(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 @@ -891,7 +882,7 @@ func makeLoadAssignment(logger hclog.Logger, cfgSnap *proxycfg.ConfigSnapshot, c
var priority uint32

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

if err != nil {
continue
Expand Down
2 changes: 0 additions & 2 deletions agent/xds/endpoints_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ 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 @@ -225,7 +224,6 @@ 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: 2 additions & 4 deletions agent/xds/locality_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,16 @@ package xds

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

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

func groupedEndpoints(logger hclog.Logger, locality *structs.Locality, policy *structs.DiscoveryPrioritizeByLocality, csns structs.CheckServiceNodes) ([]structs.CheckServiceNodes, error) {
func groupedEndpoints(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":
log := logger.Named("locality")
return prioritizeByLocalityFailover(log, locality, csns), nil
return prioritizeByLocalityFailover(locality, csns), nil
default:
return nil, fmt.Errorf("unexpected priortize-by-locality mode %q", policy.Mode)
}
Expand Down
3 changes: 1 addition & 2 deletions agent/xds/locality_policy_ce.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ package xds

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

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

0 comments on commit d7c568e

Please sign in to comment.