-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
eds/lrs: handle nil when LRS is disabled #4086
Conversation
d0a343a
to
48bc9f3
Compare
edsb := newEDSBalancerImpl(cc, nil, lsWrapper, nil) | ||
edsb.enqueueChildBalancerStateUpdate = edsb.updateState | ||
|
||
// Two localities, each with one backend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: 1 locality now
clab1 := testutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil) | ||
clab1.AddLocality(testSubZones[0], 1, 0, testEndpointAddrs[:1], nil) | ||
edsb.handleEDSResponse(parseEDSRespProtoForTesting(clab1.Build())) | ||
sc1 := <-cc.NewSubConnCh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sc
?
// We create an xdsClientWrapper with a dummy xdsClientInterface which only | ||
// implements the LoadStore() method to return the underlying load.Store to | ||
// be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make 100% sense anymore I think.
Checking
nil
before calling the methods. Even though the implementation's receiver checks for nil, the interfacenil
doesn't have the implementation type, and won't call the implementation's receiver.https://play.golang.org/p/LilYpHAr84j