config: clarify xDS DiscoveryRequest behavior#8334
Conversation
Signed-off-by: Fred Douglas <fredlas@google.com>
| // to a previous DeltaDiscoveryResponse, the response_nonce must be the | ||
| // nonce in the DeltaDiscoveryResponse. | ||
| // Otherwise response_nonce must be omitted. | ||
| // Otherwise (unlike in DiscoveryRequest) response_nonce must be omitted. |
There was a problem hiding this comment.
What are the possibility of otherwise?
First request on the wire is an obvious one. What else?
There was a problem hiding this comment.
When the DeltaDiscoveryRequest is being used as an actual request, i.e., "here are names of resources I am newly (un)interested in". This is getting into what I believe @htuch has mentioned, that these (Delta)DiscoveryRequest messages are a mix of two functionalities that would perhaps be better as explicitly separate messages: informing the server of Envoy's interest in resources, and ACKing server messages. With such a separation, this sort of rule wouldn't even need to be mentioned.
There was a problem hiding this comment.
Yeah, in UDPA we will make this separation. I think the clarifying comments are helpful in v2 incremental xDS, thanks @fredlas.
| // to a previous DeltaDiscoveryResponse, the response_nonce must be the | ||
| // nonce in the DeltaDiscoveryResponse. | ||
| // Otherwise response_nonce must be omitted. | ||
| // Otherwise (unlike in DiscoveryRequest) response_nonce must be omitted. |
There was a problem hiding this comment.
Yeah, in UDPA we will make this separation. I think the clarifying comments are helpful in v2 incremental xDS, thanks @fredlas.
|
@fredlas can you merge master and potentially |
Signed-off-by: Fred Douglas <fredlas@google.com>
|
Done. |
Clarifies comments in discovery.proto about when response_nonce is(n't) set for (Delta)DiscoveryRequest. Also left a TODO in grpc_mux_impl_test.cc where spec-deviating behavior is expected. Risk Level: none Testing: comment-only change Signed-off-by: Fred Douglas <fredlas@google.com>
Clarifies comments in discovery.proto about when response_nonce is(n't) set for (Delta)DiscoveryRequest. Also left a TODO in grpc_mux_impl_test.cc where spec-deviating behavior is expected.
Risk Level: none
Testing: comment-only change