docs: Improve xDS API documentation. #9029
Conversation
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
api/xds_protocol.rst
Outdated
|
|
||
| In the SotW protocol variants, all resource types except for :ref:`Listener <envoy_api_msg_Listener>` and :ref:`Cluster | ||
| <envoy_api_msg_Cluster>` are grouped into responses in the same way as in the incremental protocol variants. However, | ||
| :ref:`Listener <envoy_api_msg_Listener>` and :ref:`Cluster <envoy_api_msg_Cluster>` resource types are handled differently: |
There was a problem hiding this comment.
I think this is a bit misleading, since for SoTW EDS, all hosts have to be sent even if one is being added/removed, right? Can you rephrase?
There was a problem hiding this comment.
All hosts need to be sent, but not all ClusterLoadAssignments, in an EDS update. I.e. if we have 4 clusters, {A, B, C, D}, we can send one EDS update with two ClusterLoadAssignments for {A, B}, with all the hosts for A and B in the respective messages. Then we can do the same for {C, D}.
There was a problem hiding this comment.
Right, I would just clarify the text a bit if possible.
There was a problem hiding this comment.
Can you suggest what wording you'd like to see here? This document is talking about the set of resource types defined at the top, and individual hosts are not one of those types, so it's not clear to me how this could be misread to assume that individual hosts could be sent separately. Why would a reader think that hosts are any different from any other repeated field in the API?
There was a problem hiding this comment.
I think my confusion here was basically related to the discussion starting around here: #8400 (comment). I had forgotten that we have a nested CLA message and that we are still in a situation in which all hosts have to be sent either in incremental or SoTW. I think this is such a common thing and a confusion point that if you are willing to explicitly call it out that would be really great.
There was a problem hiding this comment.
Ah, okay. I've added a note about this.
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
There was a problem hiding this comment.
Thanks this LGTM with a small nit. This is really awesome work and a great improvement. I think we should ship and iterate as needed. @htuch any further comments?
/wait
api/xds_protocol.rst
Outdated
| `Cluster` resources. In effect, every `Listener` or `Cluster` resource is a root to part of Envoy's | ||
| configuration tree. | ||
|
|
||
| A non-proxy client such as gRPC will start by fetching only the specific `Listener` resources |
There was a problem hiding this comment.
nit: s/will start/might start (to account for the multiple listener Envoy Mobile, etc. case)
Signed-off-by: Mark D. Roth <roth@google.com>
Description: Improve xDS API documentation.
Risk Level: Low
Testing: N/A
Docs Changes: Included in PR.
Release Notes: N/A
@htuch and @mattklein123, please scrutinize this carefully to make sure I'm not inadvertantly getting anything wrong here. Thanks!