xds: Clarify NACK detection in protocol spec.#14841
Conversation
Signed-off-by: Mark D. Roth <roth@google.com>
|
For reference, here is an example of where the server cannot detect a NACK by looking at the version and nonce for an API other than LDS and CDS:
|
htuch
left a comment
There was a problem hiding this comment.
Appreciate the clarifications!
/wait
api/xds_protocol.rst
Outdated
| The preferred mechanism for a server to detect a NACK is to look for the presence of the | ||
| :ref:`error_detail <envoy_api_field_DiscoveryRequest.error_detail>` field in the request sent by | ||
| the client. Some older servers may instead detect a NACK by looking at both the version and the | ||
| nonce in the request: if the version in the request is lower than the one sent by the server with |
There was a problem hiding this comment.
There isn't really a notion of lower, instead equal/not-equal, since version is an opaque string.
api/xds_protocol.rst
Outdated
| :ref:`error_detail <envoy_api_field_DiscoveryRequest.error_detail>` field. The :ref:`version_info | ||
| <envoy_api_field_DiscoveryResponse.version_info>` indicates the most recent version that the | ||
| client is using, although that may not be an older version in the case where the client has | ||
| subscribed to a new resource from an existing version and that new resource is invalid. |
There was a problem hiding this comment.
This last sentence is a bit unclear. I think I get at what you mean based on our earlier conversation but not sure other readers will.
There was a problem hiding this comment.
To clarify this, I've added an example above and referred back to it here.
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth <roth@google.com>
|
@markdroth see CI. |
Signed-off-by: Mark D. Roth <roth@google.com>
|
The CI failure showing now looks unrelated to the PR. Harvey, can you (or someone else knowledgeable) please take a look? Thanks! |
|
@markdroth can you merge master and push? Thanks. |
Signed-off-by: Mark D. Roth <roth@google.com>
Signed-off-by: Mark D. Roth roth@google.com
Commit Message: xds: Clarify NACK detection in protocol spec.
Additional Description: Clarify that NACKs should be detected via the presence of the error_details field, rather than by looking at nonce and version; the latter approach does not work for APIs other than LDS or CDS, where the client can change the set of resources it is subscribing to independently of the resource type instance version changing on the server. Also clarify semantics of nonce handling.
Risk Level: Low
Testing: N/A
Docs Changes: Included in PR
Release Notes: N/A
Platform Specific Features: N/A
CC @htuch