Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 6 additions & 2 deletions balancer/ringhash/ringhash_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,9 @@ func (s) TestRingHash_AggregateClusterFallBackFromRingHashToLogicalDnsAtStartup(
}

dnsR := replaceDNSResolver(t)
dnsR.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backends[0]}}})
dnsR.UpdateState(resolver.State{
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: backends[0]}}}},
})

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

dnsR := replaceDNSResolver(t)
dnsR.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: backends[0]}}})
dnsR.UpdateState(resolver.State{
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: backends[0]}}}},
})

if err := xdsServer.Update(ctx, updateOpts); err != nil {
t.Fatalf("Failed to update xDS resources: %v", err)
Expand Down
4 changes: 1 addition & 3 deletions balancer/weightedtarget/weightedtarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ func (b *weightedTargetBalancer) UpdateClientConnState(s balancer.ClientConnStat
if !ok {
return fmt.Errorf("unexpected balancer config with type: %T", s.BalancerConfig)
}
addressesSplit := hierarchy.Group(s.ResolverState.Addresses)
endpointsSplit := hierarchy.GroupEndpoints(s.ResolverState.Endpoints)
endpointsSplit := hierarchy.Group(s.ResolverState.Endpoints)

b.stateAggregator.PauseStateUpdates()
defer b.stateAggregator.ResumeStateUpdates()
Expand Down Expand Up @@ -154,7 +153,6 @@ func (b *weightedTargetBalancer) UpdateClientConnState(s balancer.ClientConnStat
// TODO: handle error? How to aggregate errors and return?
_ = b.bg.UpdateClientConnState(name, balancer.ClientConnState{
ResolverState: resolver.State{
Addresses: addressesSplit[name],
Endpoints: endpointsSplit[name],
ServiceConfig: s.ResolverState.ServiceConfig,
Attributes: s.ResolverState.Attributes.WithValue(localityKey, name),
Expand Down
67 changes: 2 additions & 65 deletions internal/hierarchy/hierarchy.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,70 +60,7 @@ func SetInEndpoint(endpoint resolver.Endpoint, path []string) resolver.Endpoint
return endpoint
}

// Get returns the hierarchical path of addr.
func Get(addr resolver.Address) []string {
attrs := addr.BalancerAttributes
if attrs == nil {
return nil
}
path, _ := attrs.Value(pathKey).(pathValue)
return ([]string)(path)
}

// Set overrides the hierarchical path in addr with path.
func Set(addr resolver.Address, path []string) resolver.Address {
addr.BalancerAttributes = addr.BalancerAttributes.WithValue(pathKey, pathValue(path))
return addr
}

// Group splits a slice of addresses into groups based on
// the first hierarchy path. The first hierarchy path will be removed from the
// result.
//
// Input:
// [
//
// {addr0, path: [p0, wt0]}
// {addr1, path: [p0, wt1]}
// {addr2, path: [p1, wt2]}
// {addr3, path: [p1, wt3]}
//
// ]
//
// Addresses will be split into p0/p1, and the p0/p1 will be removed from the
// path.
//
// Output:
//
// {
// p0: [
// {addr0, path: [wt0]},
// {addr1, path: [wt1]},
// ],
// p1: [
// {addr2, path: [wt2]},
// {addr3, path: [wt3]},
// ],
// }
//
// If hierarchical path is not set, or has no path in it, the address is
// dropped.
func Group(addrs []resolver.Address) map[string][]resolver.Address {
ret := make(map[string][]resolver.Address)
for _, addr := range addrs {
oldPath := Get(addr)
if len(oldPath) == 0 {
continue
}
curPath := oldPath[0]
newPath := oldPath[1:]
newAddr := Set(addr, newPath)
ret[curPath] = append(ret[curPath], newAddr)
}
return ret
}

// GroupEndpoints splits a slice of endpoints into groups based on
// Group splits a slice of endpoints into groups based on
// the first hierarchy path. The first hierarchy path will be removed from the
// result.
//
Expand Down Expand Up @@ -155,7 +92,7 @@ func Group(addrs []resolver.Address) map[string][]resolver.Address {
//
// If hierarchical path is not set, or has no path in it, the endpoint is
// dropped.
func GroupEndpoints(endpoints []resolver.Endpoint) map[string][]resolver.Endpoint {
func Group(endpoints []resolver.Endpoint) map[string][]resolver.Endpoint {
ret := make(map[string][]resolver.Endpoint)
for _, endpoint := range endpoints {
oldPath := FromEndpoint(endpoint)
Expand Down
112 changes: 56 additions & 56 deletions internal/hierarchy/hierarchy_test.go
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doesn't have to be part of this PR, but looks like this test could be moved to hierarchy_test package since it only uses exported functions and doing so will ensure that in the future as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged.

Original file line number Diff line number Diff line change
Expand Up @@ -26,126 +26,126 @@ import (
"google.golang.org/grpc/resolver"
)

func TestGet(t *testing.T) {
func TestFromEndpoint(t *testing.T) {
tests := []struct {
name string
addr resolver.Address
ep resolver.Endpoint
want []string
}{
{
name: "not set",
addr: resolver.Address{},
ep: resolver.Endpoint{},
want: nil,
},
{
name: "set",
addr: resolver.Address{
BalancerAttributes: attributes.New(pathKey, pathValue{"a", "b"}),
ep: resolver.Endpoint{
Attributes: attributes.New(pathKey, pathValue{"a", "b"}),
},
want: []string{"a", "b"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := Get(tt.addr); !cmp.Equal(got, tt.want) {
t.Errorf("Get() = %v, want %v", got, tt.want)
if got := FromEndpoint(tt.ep); !cmp.Equal(got, tt.want) {
t.Errorf("FromEndpoint() = %v, want %v", got, tt.want)
}
})
}
}

func TestSet(t *testing.T) {
func TestSetInEndpoint(t *testing.T) {
tests := []struct {
name string
addr resolver.Address
ep resolver.Endpoint
path []string
}{
{
name: "before is not set",
addr: resolver.Address{},
ep: resolver.Endpoint{},
path: []string{"a", "b"},
},
{
name: "before is set",
addr: resolver.Address{
BalancerAttributes: attributes.New(pathKey, pathValue{"before", "a", "b"}),
ep: resolver.Endpoint{
Attributes: attributes.New(pathKey, pathValue{"before", "a", "b"}),
},
path: []string{"a", "b"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
newAddr := Set(tt.addr, tt.path)
newPath := Get(newAddr)
newEP := SetInEndpoint(tt.ep, tt.path)
newPath := FromEndpoint(newEP)
if !cmp.Equal(newPath, tt.path) {
t.Errorf("path after Set() = %v, want %v", newPath, tt.path)
t.Errorf("path after SetInEndpoint() = %v, want %v", newPath, tt.path)
}
})
}
}

func TestGroup(t *testing.T) {
tests := []struct {
name string
addrs []resolver.Address
want map[string][]resolver.Address
name string
eps []resolver.Endpoint
want map[string][]resolver.Endpoint
}{
{
name: "all with hierarchy",
addrs: []resolver.Address{
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
{Addr: "b0", BalancerAttributes: attributes.New(pathKey, pathValue{"b"})},
{Addr: "b1", BalancerAttributes: attributes.New(pathKey, pathValue{"b"})},
eps: []resolver.Endpoint{
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: attributes.New(pathKey, pathValue{"b"})},
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: attributes.New(pathKey, pathValue{"b"})},
},
want: map[string][]resolver.Address{
want: map[string][]resolver.Endpoint{
"a": {
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{})},
},
"b": {
{Addr: "b0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addr: "b1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: attributes.New(pathKey, pathValue{})},
},
},
},
{
// Addresses without hierarchy are ignored.
// Endpoints without hierarchy are ignored.
name: "without hierarchy",
addrs: []resolver.Address{
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
{Addr: "b0", BalancerAttributes: nil},
{Addr: "b1", BalancerAttributes: nil},
eps: []resolver.Endpoint{
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: nil},
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: nil},
},
want: map[string][]resolver.Address{
want: map[string][]resolver.Endpoint{
"a": {
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{})},
},
},
},
{
// If hierarchy is set to a wrong type (which should never happen),
// the address is ignored.
// the endpoint is ignored.
name: "wrong type",
addrs: []resolver.Address{
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{"a"})},
{Addr: "b0", BalancerAttributes: attributes.New(pathKey, "b")},
{Addr: "b1", BalancerAttributes: attributes.New(pathKey, 314)},
eps: []resolver.Endpoint{
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{"a"})},
{Addresses: []resolver.Address{{Addr: "b0"}}, Attributes: attributes.New(pathKey, "b")},
{Addresses: []resolver.Address{{Addr: "b1"}}, Attributes: attributes.New(pathKey, 314)},
},
want: map[string][]resolver.Address{
want: map[string][]resolver.Endpoint{
"a": {
{Addr: "a0", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addr: "a1", BalancerAttributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "a0"}}, Attributes: attributes.New(pathKey, pathValue{})},
{Addresses: []resolver.Address{{Addr: "a1"}}, Attributes: attributes.New(pathKey, pathValue{})},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := Group(tt.addrs); !cmp.Equal(got, tt.want, cmp.AllowUnexported(attributes.Attributes{})) {
if got := Group(tt.eps); !cmp.Equal(got, tt.want, cmp.AllowUnexported(attributes.Attributes{})) {
t.Errorf("Group() = %v, want %v", got, tt.want)
t.Errorf("diff: %v", cmp.Diff(got, tt.want, cmp.AllowUnexported(attributes.Attributes{})))
}
Expand All @@ -165,28 +165,28 @@ func TestGroupE2E(t *testing.T) {
},
}

var addrsWithHierarchy []resolver.Address
var epsWithHierarchy []resolver.Endpoint
for p, wts := range hierarchy {
path1 := pathValue{p}
for wt, addrs := range wts {
path2 := append(pathValue(nil), path1...)
path2 = append(path2, wt)
for _, addr := range addrs {
a := resolver.Address{
Addr: addr,
BalancerAttributes: attributes.New(pathKey, path2),
a := resolver.Endpoint{
Addresses: []resolver.Address{{Addr: addr}},
Attributes: attributes.New(pathKey, path2),
}
addrsWithHierarchy = append(addrsWithHierarchy, a)
epsWithHierarchy = append(epsWithHierarchy, a)
}
}
}

gotHierarchy := make(map[string]map[string][]string)
for p1, wts := range Group(addrsWithHierarchy) {
for p1, wts := range Group(epsWithHierarchy) {
gotHierarchy[p1] = make(map[string][]string)
for p2, addrs := range Group(wts) {
for _, addr := range addrs {
gotHierarchy[p1][p2] = append(gotHierarchy[p1][p2], addr.Addr)
for p2, eps := range Group(wts) {
for _, ep := range eps {
gotHierarchy[p1][p2] = append(gotHierarchy[p1][p2], ep.Addresses[0].Addr)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func (s) TestAggregateCluster_WithEDSAndDNS(t *testing.T) {
}

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

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

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

// Ensure that RPCs start getting routed to the first backend since the
// child policy for a LOGICAL_DNS cluster is pick_first by default.
Expand Down Expand Up @@ -1244,3 +1244,11 @@ func (s) TestAggregateCluster_Fallback_EDS_ResourceNotFound(t *testing.T) {
t.Fatalf("EmptyCall() routed to backend %q, want %q", peer.Addr, server.Address)
}
}

func addrsToEndpoints(addrs []resolver.Address) []resolver.Endpoint {
endpoints := make([]resolver.Endpoint, len(addrs))
for i, addr := range addrs {
endpoints[i] = resolver.Endpoint{Addresses: []resolver.Address{addr}}
}
return endpoints
}
11 changes: 0 additions & 11 deletions internal/xds/balancer/cdsbalancer/e2e_test/eds_impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1004,18 +1004,15 @@ func (s) TestEDS_EndpointWithMultipleAddresses(t *testing.T) {
name string
dualstackEndpointsEnabled bool
wantEndpointPorts []uint32
wantAddrPorts []uint32
}{
{
name: "flag_enabled",
dualstackEndpointsEnabled: true,
wantEndpointPorts: ports,
wantAddrPorts: ports[:1],
},
{
name: "flag_disabled",
wantEndpointPorts: ports[:1],
wantAddrPorts: ports[:1],
},
}

Expand Down Expand Up @@ -1109,14 +1106,6 @@ func (s) TestEDS_EndpointWithMultipleAddresses(t *testing.T) {
if diff := cmp.Diff(gotEndpointPorts, tc.wantEndpointPorts); diff != "" {
t.Errorf("Unexpected endpoint address ports in resolver update, diff (-got +want): %v", diff)
}

gotAddrPorts := []uint32{}
for _, a := range gotState.Addresses {
gotAddrPorts = append(gotAddrPorts, testutils.ParsePort(t, a.Addr))
}
if diff := cmp.Diff(gotAddrPorts, tc.wantAddrPorts); diff != "" {
t.Errorf("Unexpected address ports in resolver update, diff (-got +want): %v", diff)
}
})
}
}
Loading