- 
                Notifications
    
You must be signed in to change notification settings  - Fork 3.9k
 
xds: allow grpclb balancer addresses for backward compatibility #5823
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
Conversation
| 
           This work is done in the XdsLoadBalancer -- doesn't it require client to be upgraded to xDS in the first place?  | 
    
| LbConfig fallbackPolicy) { | ||
| this.fallbackServers = servers; | ||
| String fallbackPolicyName = fallbackPolicy.getPolicyName(); | ||
| if (!fallbackPolicyName.equals("grpclb") && !fallbackPolicyName.equals(XDS_POLICY_NAME)) { | 
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.
Probably worth a comment here.
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.
Done.
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.
PTAL
| servers = backends.build(); | ||
| } | ||
| 
               | 
          ||
| // FIXME: this is a temporary hack. | 
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.
Please add TODO(zhangkun83) and refer to #5496
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.
Done.
| } | ||
| } | ||
| 
               | 
          ||
| private void handleFallbackAddressesOrNameResolutionError() { | 
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.
Why does the method name contain "NameResolutionError"? It's only called from fallback-related methods. What this method does is passing the latest fallback addresses to the fallback balancer. The handling of empty addresses is just part of it. Maybe "propagateFallbackAddresses()".
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.
Renamed
During migration, the name resolver may not know when the client has been upgraded to xds, so it may still send grpclb v1 addresses with a list of policies including both grpclb v1 and xds.