-
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
xds/server: fix RDS handling for non-inline route configs #6915
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6915 +/- ##
==========================================
- Coverage 83.65% 83.64% -0.02%
==========================================
Files 286 287 +1
Lines 30756 30920 +164
==========================================
+ Hits 25730 25862 +132
- Misses 3963 3992 +29
- Partials 1063 1066 +3
|
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.
Looks really good overall! Mostly just some style and maintainability comments.
7f59c17
to
fcd71cd
Compare
fcd71cd
to
bb7b733
Compare
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.
I only glanced at the tests so far; will look a bit more closely next round.
Just a few minor things.
All interop tests continue to pass. |
Continuation and squashed based on #6889.
Fix the xDS Server to align with spec in https://github.com/grpc/proposal/blob/master/A36-xds-for-servers.md.
xds/internal/server/listener_wrapper:
xds/internal/server/conn_wrapper: Read routing configuration dynamically through an atomic load of a pointer, also add L7 error representation.
xds/internal/server/rds_handler: Persist rds updates and error conditions for each specific route resource being watched. Add a helper which determines whether all dynamic rds resources needed have been received. RDS errors mean the RDS has received configuration, trigger failure at L7 routing layer against incoming RPCs.
xds/server: Don’t block on a “good update”, immediately start serving on lis passed into Serve once wrapped, if xDS resources aren’t ready or LDS returns resource not found Accept() and Close() in not serving mode. Log any L7 routing errors due to xDS Configuration problems.
server: Call a callback with the server transport once it’s created on the Conn. This gives access to the server transport to xDS layer, which will be gracefully closed on transitions into not serving and transitions to a new LDS configuration. It also guarantees at some point the server transport will be gracefully drained. This replaces the Drain() operation previously present.
All LDS + RDS callbacks from the client are synchronous. The events that can happen asynchronous are the Accepting of a Conn (protected by mutex) and also an RPC on the Conn (protected with an atomic pointer). Thus dropped spawned event goroutines handleServingModeChanges() in the xDS Server, and run() on the listener_wrapper. Handled all the xDS resources inline and synced data structures/behaviors using above methods. L7 error = fail RPC with status code UNAVAILABLE (don’t give back more information due to security risk).
Fixes #6788.
RELEASE NOTES: