Skip to content

Commit 90f571d

Browse files
arjan-balarjan-bal
authored
xds: remove references to ResolverState.Addresses (#8841)
This change removes the usages of `ResolverState.Addresses` in xds code. Prior to Dualstack support, xDS LB policies relied exclusively on the `Addresses` field. As part of the Dualstack initiative, these policies were updated to support both `Endpoints` and `Addresses`. This PR removes the `Addresses` propagation logic in favor of relying solely on `Endpoints`. RELEASE NOTES: * balancer/weightedtarget: Remove handling of `Addresses` and only handle `Endpoints` in resolver updates.
1 parent 679565f commit 90f571d

14 files changed

Lines changed: 92 additions & 207 deletions

File tree

‎balancer/ringhash/ringhash_e2e_test.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,9 @@ func (s) TestRingHash_AggregateClusterFallBackFromRingHashToLogicalDnsAtStartup(
475475
}
476476

477477
dnsR := replaceDNSResolver(t)
478-
dnsR.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backends[0]}}})
478+
dnsR.UpdateState(resolver.State{
479+
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: backends[0]}}}},
480+
})
479481

480482
if err := xdsServer.Update(ctx, updateOpts); err != nil {
481483
t.Fatalf("Failed to update xDS resources: %v", err)
@@ -553,7 +555,9 @@ func (s) TestRingHash_AggregateClusterFallBackFromRingHashToLogicalDnsAtStartupN
553555
}
554556

555557
dnsR := replaceDNSResolver(t)
556-
dnsR.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backends[0]}}})
558+
dnsR.UpdateState(resolver.State{
559+
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: backends[0]}}}},
560+
})
557561

558562
if err := xdsServer.Update(ctx, updateOpts); err != nil {
559563
t.Fatalf("Failed to update xDS resources: %v", err)

‎balancer/weightedtarget/weightedtarget.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,7 @@ func (b *weightedTargetBalancer) UpdateClientConnState(s balancer.ClientConnStat
106106
if !ok {
107107
return fmt.Errorf("unexpected balancer config with type: %T", s.BalancerConfig)
108108
}
109-
addressesSplit := hierarchy.Group(s.ResolverState.Addresses)
110-
endpointsSplit := hierarchy.GroupEndpoints(s.ResolverState.Endpoints)
109+
endpointsSplit := hierarchy.Group(s.ResolverState.Endpoints)
111110

112111
b.stateAggregator.PauseStateUpdates()
113112
defer b.stateAggregator.ResumeStateUpdates()
@@ -154,7 +153,6 @@ func (b *weightedTargetBalancer) UpdateClientConnState(s balancer.ClientConnStat
154153
// TODO: handle error? How to aggregate errors and return?
155154
_ = b.bg.UpdateClientConnState(name, balancer.ClientConnState{
156155
ResolverState: resolver.State{
157-
Addresses: addressesSplit[name],
158156
Endpoints: endpointsSplit[name],
159157
ServiceConfig: s.ResolverState.ServiceConfig,
160158
Attributes: s.ResolverState.Attributes.WithValue(localityKey, name),

‎internal/hierarchy/hierarchy.go‎

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -60,70 +60,7 @@ func SetInEndpoint(endpoint resolver.Endpoint, path []string) resolver.Endpoint
6060
return endpoint
6161
}
6262

63-
// Get returns the hierarchical path of addr.
64-
func Get(addr resolver.Address) []string {
65-
attrs := addr.BalancerAttributes
66-
if attrs == nil {
67-
return nil
68-
}
69-
path, _ := attrs.Value(pathKey).(pathValue)
70-
return ([]string)(path)
71-
}
72-
73-
// Set overrides the hierarchical path in addr with path.
74-
func Set(addr resolver.Address, path []string) resolver.Address {
75-
addr.BalancerAttributes = addr.BalancerAttributes.WithValue(pathKey, pathValue(path))
76-
return addr
77-
}
78-
79-
// Group splits a slice of addresses into groups based on
80-
// the first hierarchy path. The first hierarchy path will be removed from the
81-
// result.
82-
//
83-
// Input:
84-
// [
85-
//
86-
// {addr0, path: [p0, wt0]}
87-
// {addr1, path: [p0, wt1]}
88-
// {addr2, path: [p1, wt2]}
89-
// {addr3, path: [p1, wt3]}
90-
//
91-
// ]
92-
//
93-
// Addresses will be split into p0/p1, and the p0/p1 will be removed from the
94-
// path.
95-
//
96-
// Output:
97-
//
98-
// {
99-
// p0: [
100-
// {addr0, path: [wt0]},
101-
// {addr1, path: [wt1]},
102-
// ],
103-
// p1: [
104-
// {addr2, path: [wt2]},
105-
// {addr3, path: [wt3]},
106-
// ],
107-
// }
108-
//
109-
// If hierarchical path is not set, or has no path in it, the address is
110-
// dropped.
111-
func Group(addrs []resolver.Address) map[string][]resolver.Address {
112-
ret := make(map[string][]resolver.Address)
113-
for _, addr := range addrs {
114-
oldPath := Get(addr)
115-
if len(oldPath) == 0 {
116-
continue
117-
}
118-
curPath := oldPath[0]
119-
newPath := oldPath[1:]
120-
newAddr := Set(addr, newPath)
121-
ret[curPath] = append(ret[curPath], newAddr)
122-
}
123-
return ret
124-
}
125-
126-
// GroupEndpoints splits a slice of endpoints into groups based on
63+
// Group splits a slice of endpoints into groups based on
12764
// the first hierarchy path. The first hierarchy path will be removed from the
12865
// result.
12966
//
@@ -155,7 +92,7 @@ func Group(addrs []resolver.Address) map[string][]resolver.Address {
15592
//
15693
// If hierarchical path is not set, or has no path in it, the endpoint is
15794
// dropped.
158-
func GroupEndpoints(endpoints []resolver.Endpoint) map[string][]resolver.Endpoint {
95+
func Group(endpoints []resolver.Endpoint) map[string][]resolver.Endpoint {
15996
ret := make(map[string][]resolver.Endpoint)
16097
for _, endpoint := range endpoints {
16198
oldPath := FromEndpoint(endpoint)

‎internal/hierarchy/hierarchy_test.go‎

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -26,126 +26,126 @@ import (
2626
"google.golang.org/grpc/resolver"
2727
)
2828

29-
func TestGet(t *testing.T) {
29+
func TestFromEndpoint(t *testing.T) {
3030
tests := []struct {
3131
name string
32-
addr resolver.Address
32+
ep resolver.Endpoint
3333
want []string
3434
}{
3535
{
3636
name: "not set",
37-
addr: resolver.Address{},
37+
ep: resolver.Endpoint{},
3838
want: nil,
3939
},
4040
{
4141
name: "set",
42-
addr: resolver.Address{
43-
BalancerAttributes: attributes.New(pathKey, pathValue{"a", "b"}),
42+
ep: resolver.Endpoint{
43+
Attributes: attributes.New(pathKey, pathValue{"a", "b"}),
4444
},
4545
want: []string{"a", "b"},
4646
},
4747
}
4848
for _, tt := range tests {
4949
t.Run(tt.name, func(t *testing.T) {
50-
if got := Get(tt.addr); !cmp.Equal(got, tt.want) {
51-
t.Errorf("Get() = %v, want %v", got, tt.want)
50+
if got := FromEndpoint(tt.ep); !cmp.Equal(got, tt.want) {
51+
t.Errorf("FromEndpoint() = %v, want %v", got, tt.want)
5252
}
5353
})
5454
}
5555
}
5656

57-
func TestSet(t *testing.T) {
57+
func TestSetInEndpoint(t *testing.T) {
5858
tests := []struct {
5959
name string
60-
addr resolver.Address
60+
ep resolver.Endpoint
6161
path []string
6262
}{
6363
{
6464
name: "before is not set",
65-
addr: resolver.Address{},
65+
ep: resolver.Endpoint{},
6666
path: []string{"a", "b"},
6767
},
6868
{
6969
name: "before is set",
70-
addr: resolver.Address{
71-
BalancerAttributes: attributes.New(pathKey, pathValue{"before", "a", "b"}),
70+
ep: resolver.Endpoint{
71+
Attributes: attributes.New(pathKey, pathValue{"before", "a", "b"}),
7272
},
7373
path: []string{"a", "b"},
7474
},
7575
}
7676
for _, tt := range tests {
7777
t.Run(tt.name, func(t *testing.T) {
78-
newAddr := Set(tt.addr, tt.path)
79-
newPath := Get(newAddr)
78+
newEP := SetInEndpoint(tt.ep, tt.path)
79+
newPath := FromEndpoint(newEP)
8080
if !cmp.Equal(newPath, tt.path) {
81-
t.Errorf("path after Set() = %v, want %v", newPath, tt.path)
81+
t.Errorf("path after SetInEndpoint() = %v, want %v", newPath, tt.path)
8282
}
8383
})
8484
}
8585
}
8686

8787
func TestGroup(t *testing.T) {
8888
tests := []struct {
89-
name string
90-
addrs []resolver.Address
91-
want map[string][]resolver.Address
89+
name string
90+
eps []resolver.Endpoint
91+
want map[string][]resolver.Endpoint
9292
}{
9393
{
9494
name: "all with hierarchy",
95-
addrs: []resolver.Address{
96-
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
97-
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
98-
{Addr: "b0", BalancerAttributes: attributes.New(pathKey, pathValue{"b"})},
99-
{Addr: "b1", BalancerAttributes: attributes.New(pathKey, pathValue{"b"})},
95+
eps: []resolver.Endpoint{
96+
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
97+
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
98+
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: attributes.New(pathKey, pathValue{"b"})},
99+
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: attributes.New(pathKey, pathValue{"b"})},
100100
},
101-
want: map[string][]resolver.Address{
101+
want: map[string][]resolver.Endpoint{
102102
"a": {
103-
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
104-
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
103+
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{})},
104+
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{})},
105105
},
106106
"b": {
107-
{Addr: "b0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
108-
{Addr: "b1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
107+
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: attributes.New(pathKey, pathValue{})},
108+
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: attributes.New(pathKey, pathValue{})},
109109
},
110110
},
111111
},
112112
{
113-
// Addresses without hierarchy are ignored.
113+
// Endpoints without hierarchy are ignored.
114114
name: "without hierarchy",
115-
addrs: []resolver.Address{
116-
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
117-
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
118-
{Addr: "b0", BalancerAttributes: nil},
119-
{Addr: "b1", BalancerAttributes: nil},
115+
eps: []resolver.Endpoint{
116+
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
117+
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
118+
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: nil},
119+
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: nil},
120120
},
121-
want: map[string][]resolver.Address{
121+
want: map[string][]resolver.Endpoint{
122122
"a": {
123-
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
124-
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
123+
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{})},
124+
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{})},
125125
},
126126
},
127127
},
128128
{
129129
// If hierarchy is set to a wrong type (which should never happen),
130-
// the address is ignored.
130+
// the endpoint is ignored.
131131
name: "wrong type",
132-
addrs: []resolver.Address{
133-
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
134-
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
135-
{Addr: "b0", BalancerAttributes: attributes.New(pathKey, "b")},
136-
{Addr: "b1", BalancerAttributes: attributes.New(pathKey, 314)},
132+
eps: []resolver.Endpoint{
133+
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
134+
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
135+
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: attributes.New(pathKey, "b")},
136+
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: attributes.New(pathKey, 314)},
137137
},
138-
want: map[string][]resolver.Address{
138+
want: map[string][]resolver.Endpoint{
139139
"a": {
140-
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
141-
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
140+
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{})},
141+
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{})},
142142
},
143143
},
144144
},
145145
}
146146
for _, tt := range tests {
147147
t.Run(tt.name, func(t *testing.T) {
148-
if got := Group(tt.addrs); !cmp.Equal(got, tt.want, cmp.AllowUnexported(attributes.Attributes{})) {
148+
if got := Group(tt.eps); !cmp.Equal(got, tt.want, cmp.AllowUnexported(attributes.Attributes{})) {
149149
t.Errorf("Group() = %v, want %v", got, tt.want)
150150
t.Errorf("diff: %v", cmp.Diff(got, tt.want, cmp.AllowUnexported(attributes.Attributes{})))
151151
}
@@ -165,28 +165,28 @@ func TestGroupE2E(t *testing.T) {
165165
},
166166
}
167167

168-
var addrsWithHierarchy []resolver.Address
168+
var epsWithHierarchy []resolver.Endpoint
169169
for p, wts := range hierarchy {
170170
path1 := pathValue{p}
171171
for wt, addrs := range wts {
172172
path2 := append(pathValue(nil), path1...)
173173
path2 = append(path2, wt)
174174
for _, addr := range addrs {
175-
a := resolver.Address{
176-
Addr: addr,
177-
BalancerAttributes: attributes.New(pathKey, path2),
175+
a := resolver.Endpoint{
176+
Addresses: []resolver.Address{{Addr: addr}},
177+
Attributes: attributes.New(pathKey, path2),
178178
}
179-
addrsWithHierarchy = append(addrsWithHierarchy, a)
179+
epsWithHierarchy = append(epsWithHierarchy, a)
180180
}
181181
}
182182
}
183183

184184
gotHierarchy := make(map[string]map[string][]string)
185-
for p1, wts := range Group(addrsWithHierarchy) {
185+
for p1, wts := range Group(epsWithHierarchy) {
186186
gotHierarchy[p1] = make(map[string][]string)
187-
for p2, addrs := range Group(wts) {
188-
for _, addr := range addrs {
189-
gotHierarchy[p1][p2] = append(gotHierarchy[p1][p2], addr.Addr)
187+
for p2, eps := range Group(wts) {
188+
for _, ep := range eps {
189+
gotHierarchy[p1][p2] = append(gotHierarchy[p1][p2], ep.Addresses[0].Addr)
190190
}
191191
}
192192
}

‎internal/xds/balancer/cdsbalancer/e2e_test/aggregate_cluster_test.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ func (s) TestAggregateCluster_WithEDSAndDNS(t *testing.T) {
589589
}
590590

591591
// Update DNS resolver with test backend addresses.
592-
dnsR.UpdateState(resolver.State{Addresses: addrs[1:]})
592+
dnsR.UpdateState(resolver.State{Endpoints: addrsToEndpoints(addrs[1:])})
593593

594594
// Make an RPC and ensure that it gets routed to the first backend since the
595595
// EDS cluster is of higher priority than the LOGICAL_DNS cluster.
@@ -761,7 +761,7 @@ func (s) TestAggregateCluster_BadEDS_GoodToBadDNS(t *testing.T) {
761761
}
762762

763763
// Update DNS resolver with test backend addresses.
764-
dnsR.UpdateState(resolver.State{Addresses: addrs})
764+
dnsR.UpdateState(resolver.State{Endpoints: addrsToEndpoints(addrs)})
765765

766766
// Ensure that RPCs start getting routed to the first backend since the
767767
// child policy for a LOGICAL_DNS cluster is pick_first by default.
@@ -1244,3 +1244,11 @@ func (s) TestAggregateCluster_Fallback_EDS_ResourceNotFound(t *testing.T) {
12441244
t.Fatalf("EmptyCall() routed to backend %q, want %q", peer.Addr, server.Address)
12451245
}
12461246
}
1247+
1248+
func addrsToEndpoints(addrs []resolver.Address) []resolver.Endpoint {
1249+
endpoints := make([]resolver.Endpoint, len(addrs))
1250+
for i, addr := range addrs {
1251+
endpoints[i] = resolver.Endpoint{Addresses: []resolver.Address{addr}}
1252+
}
1253+
return endpoints
1254+
}

‎internal/xds/balancer/cdsbalancer/e2e_test/eds_impl_test.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,18 +1004,15 @@ func (s) TestEDS_EndpointWithMultipleAddresses(t *testing.T) {
10041004
name string
10051005
dualstackEndpointsEnabled bool
10061006
wantEndpointPorts []uint32
1007-
wantAddrPorts []uint32
10081007
}{
10091008
{
10101009
name: "flag_enabled",
10111010
dualstackEndpointsEnabled: true,
10121011
wantEndpointPorts: ports,
1013-
wantAddrPorts: ports[:1],
10141012
},
10151013
{
10161014
name: "flag_disabled",
10171015
wantEndpointPorts: ports[:1],
1018-
wantAddrPorts: ports[:1],
10191016
},
10201017
}
10211018

@@ -1109,14 +1106,6 @@ func (s) TestEDS_EndpointWithMultipleAddresses(t *testing.T) {
11091106
if diff := cmp.Diff(gotEndpointPorts, tc.wantEndpointPorts); diff != "" {
11101107
t.Errorf("Unexpected endpoint address ports in resolver update, diff (-got +want): %v", diff)
11111108
}
1112-
1113-
gotAddrPorts := []uint32{}
1114-
for _, a := range gotState.Addresses {
1115-
gotAddrPorts = append(gotAddrPorts, testutils.ParsePort(t, a.Addr))
1116-
}
1117-
if diff := cmp.Diff(gotAddrPorts, tc.wantAddrPorts); diff != "" {
1118-
t.Errorf("Unexpected address ports in resolver update, diff (-got +want): %v", diff)
1119-
}
11201109
})
11211110
}
11221111
}

0 commit comments

Comments
 (0)