Skip to content

Commit 4f07f87

Browse files
committed
Fix attribute get and call ParseConfig on child
1 parent 95f9dd0 commit 4f07f87

File tree

2 files changed

+39
-20
lines changed

2 files changed

+39
-20
lines changed

xds/internal/balancer/wrrlocality/balancer.go

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,14 @@ func (bb) Build(cc balancer.ClientConn, bOpts balancer.BuildOptions) balancer.Ba
7474
// shouldn't happen, defensive programming.
7575
return nil
7676
}
77+
wtbCfgParser, ok := builder.(balancer.ConfigParser)
78+
if !ok {
79+
// Shouldn't happen, imported weighted target builder has this method.
80+
return nil
81+
}
7782
wrrL := &wrrLocalityBalancer{
78-
child: wtb,
83+
child: wtb,
84+
configParser: wtbCfgParser,
7985
}
8086

8187
wrrL.logger = prefixLogger(wrrL)
@@ -119,19 +125,11 @@ func (a AddrInfo) String() string {
119125
}
120126

121127
// getAddrInfo returns the AddrInfo stored in the BalancerAttributes field of
122-
// addr. Returns an AddrInfo with LocalityWeight of 1 if no AddrInfo found.
123-
func getAddrInfo(addr resolver.Address) AddrInfo {
128+
// addr. Returns false if no AddrInfo found.
129+
func getAddrInfo(addr resolver.Address) (AddrInfo, bool) {
124130
v := addr.BalancerAttributes.Value(attributeKey{})
125-
var ai AddrInfo
126-
var ok bool
127-
if ai, ok = v.(AddrInfo); !ok {
128-
// Shouldn't happen, but to avoid undefiend behavior of 0 locality
129-
// weight.
130-
return AddrInfo{
131-
LocalityWeight: 1,
132-
}
133-
}
134-
return ai
131+
ai, ok := v.(AddrInfo)
132+
return ai, ok
135133
}
136134

137135
// wrrLocalityBalancer wraps a weighted target balancer, and builds
@@ -145,6 +143,8 @@ type wrrLocalityBalancer struct {
145143
// Other balancer operations you pass through.
146144
child balancer.Balancer
147145

146+
configParser balancer.ConfigParser
147+
148148
logger *grpclog.PrefixLogger
149149
}
150150

@@ -157,24 +157,36 @@ func (b *wrrLocalityBalancer) UpdateClientConnState(s balancer.ClientConnState)
157157

158158
weightedTargets := make(map[string]weightedtarget.Target)
159159
for _, addr := range s.ResolverState.Addresses {
160-
// These gets of attributes could potentially return zero values. This
161-
// shouldn't happen though, and thus don't error out, and just build a
162-
// weighted target with undefined behavior. (For the attribute in this
163-
// package, I defaulted the getter to return a value with defined
164-
// behavior rather than zero value).
160+
// This get of LocalityID could potentially return a zero value. This
161+
// shouldn't happen though (this attribute that is set actually gets
162+
// used to build localities in the first place), and thus don't error
163+
// out, and just build a weighted target with undefined behavior.
165164
locality := internal.GetLocalityID(addr)
166165
localityString, err := locality.ToString()
167166
if err != nil {
168167
// Should never happen.
169168
logger.Infof("failed to marshal LocalityID: %v, skipping this locality in weighted target")
170169
}
171-
ai := getAddrInfo(addr)
170+
ai, ok := getAddrInfo(addr)
171+
if !ok {
172+
return fmt.Errorf("addr: %v is misisng locality weight information", addr)
173+
}
172174
weightedTargets[localityString] = weightedtarget.Target{Weight: ai.LocalityWeight, ChildPolicy: lbCfg.ChildPolicy}
173175
}
174176
wtCfg := &weightedtarget.LBConfig{Targets: weightedTargets}
177+
wtCfgJSON, err := json.Marshal(wtCfg)
178+
if err != nil {
179+
// Shouldn't happen.
180+
return fmt.Errorf("error marshalling prepared wtCfg: %v", wtCfg)
181+
}
182+
var scLBCfg serviceconfig.LoadBalancingConfig
183+
if scLBCfg, err = b.configParser.ParseConfig(wtCfgJSON); err != nil {
184+
return fmt.Errorf("config generated %v by wrr_locality_experimental is invalid: %v", wtCfgJSON, err)
185+
}
186+
175187
return b.child.UpdateClientConnState(balancer.ClientConnState{
176188
ResolverState: s.ResolverState,
177-
BalancerConfig: wtCfg,
189+
BalancerConfig: scLBCfg,
178190
})
179191
}
180192

xds/internal/balancer/wrrlocality/balancer_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,13 @@ func (s) TestUpdateClientConnState(t *testing.T) {
158158
// child.
159159
weightedTargetName = "mock_weighted_target"
160160
stub.Register("mock_weighted_target", stub.BalancerFuncs{
161+
ParseConfig: func(c json.RawMessage) (serviceconfig.LoadBalancingConfig, error) {
162+
var cfg weightedtarget.LBConfig
163+
if err := json.Unmarshal(c, &cfg); err != nil {
164+
return nil, err
165+
}
166+
return &cfg, nil
167+
},
161168
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
162169
wtCfg, ok := ccs.BalancerConfig.(*weightedtarget.LBConfig)
163170
if !ok {

0 commit comments

Comments
 (0)