xrfc: xdstp:// xRFC (TP1) #6
Conversation
This ports https://docs.google.com/document/d/1zZav-IYxMO0mP2A7y5XHBa9v0eXyirw1CxrLLSUqWR8/edit# to xRFC format and updates to match changes that have occurred during Envoy implementation. Signed-off-by: Harvey Tuch <htuch@google.com>
|
CC @cncf/xds-wg |
| resource_names_subscribe: | ||
| - xdstp://some-cloud-authority/envoy.config.endpoint.v3.ClusterLoadAssignment/foo | ||
| ``` | ||
|
|
There was a problem hiding this comment.
can you expand on this. What does this mean?
There was a problem hiding this comment.
Which bit is unclear? Happy to expand on anything here, just need to figure out what is missing :)
There was a problem hiding this comment.
How is the fallback to 'some-onprem-authority' configured?
There was a problem hiding this comment.
The presence of the #alt directive above describes the fallback relationship. It's also mentioned that there is a mapping from some-onprem-authority to ConfigSource in the bootstrap.
proposals/TP1-xds-transport-next.md
Outdated
| Two `xdstp://` URNs are considered equivalent if they match component-wise | ||
| modulo context parameter ordering. | ||
|
|
||
| ## Context parameters |
There was a problem hiding this comment.
How are we going to handle duplicate context params from different process? Say one from URL has same key with the one from dynamic context param added by client.
There was a problem hiding this comment.
There should not be any conflicts of this nature, because the names are designed to prevent this kind of conflict. Node-derived context params start with xds.node.; client feature parameters start with xds.client_feature.; per-resource-type parameters start with xds.resource.; and parameters from the URL should not start with xds. at all.
There was a problem hiding this comment.
@markdroth did we end up with any rules around how URL parameters (i.e. free form) are supposed to work? I.e. "they can never starts with xds." or "they can optionally start with xds. if deliberately overriding other context parameters"?
There was a problem hiding this comment.
I think it will be clear to have some rules for context parameters from URL.
There was a problem hiding this comment.
Sure. @markdroth can you comment on #6 (comment)?
There was a problem hiding this comment.
I think where we wound up is that the ad-hoc context params can't start with xds., so there will basically never be any conflicts.
proposals/TP1-xds-transport-next.md
Outdated
| Two `xdstp://` URNs are considered equivalent if they match component-wise | ||
| modulo context parameter ordering. | ||
|
|
||
| ## Context parameters |
There was a problem hiding this comment.
There should not be any conflicts of this nature, because the names are designed to prevent this kind of conflict. Node-derived context params start with xds.node.; client feature parameters start with xds.client_feature.; per-resource-type parameters start with xds.resource.; and parameters from the URL should not start with xds. at all.
proposals/TP1-xds-transport-next.md
Outdated
|
|
||
| ### Discovery request and responses | ||
|
|
||
| State-of-the-world (SotW) `Discovery{Request,Response}` are not supported with |
There was a problem hiding this comment.
Now that we're using strings instead of structured protos for URIs on the wire, I don't think there's actually any reason to not support xdstp names in SotW. I think the only thing that doesn't work in SotW is glob collections, because there's no way for the server to delete a resource from the collection.
There was a problem hiding this comment.
True, but I rather think about this additively; land the delta spec and add SotW as needed.
There was a problem hiding this comment.
I'm okay with that, but I do think that we'll need another xRFC for the new naming scheme in SotW in the near future, because gRPC is planning to implement that. And that xRFC will be very short; it will basically say "everything from the original xRFC except for glob collections works for StoW too".
There was a problem hiding this comment.
I think I'd like to renew my request to not explicitly exclude SotW here. gRPC is definitely going to be supporting xdstp names in SotW, and it seems silly to have to publish a follow-up xRFC just to say "everything in xRFC TP1 works for SotW too".
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
| resource_names_subscribe: | ||
| - xdstp://some-cloud-authority/envoy.config.endpoint.v3.ClusterLoadAssignment/foo | ||
| ``` | ||
|
|
There was a problem hiding this comment.
How is the fallback to 'some-onprem-authority' configured?
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
| ``` | ||
|
|
||
| Under normal conditions, the requested resource is returned as above. If the | ||
| config source for `some-cloud-authority` is unreachable, then the client issues |
There was a problem hiding this comment.
Should the on-prem server be reached if the fail-over is temporary?
Should the client try accessing the original server periodically, and if it's alive change the configuration to the one it receives from the original server?
There was a problem hiding this comment.
This is a good question; I'm tempted to punt as we're not implementing this right now and it gets complicated. What about leaving it as a client specific how failover and restoration of original resource fetch is decided?
@markdroth also interested in your take on this one.
There was a problem hiding this comment.
I added a note stating this is client specific.
There was a problem hiding this comment.
I think the client should always be trying to contact the original server, with appropriate backoff delay. Whenever the original server recovers enough to return a result, then the client should switch back to data from that server.
That having been said, I think I would be in favor of removing this from this xRFC, because we don't plan to implement this part in either gRPC or in Envoy in the near future. I think we can put together a separate xRFC for this once we actually have a need for it.
There was a problem hiding this comment.
(Unresolving this conversation so that my last comment gets addressed.)
There was a problem hiding this comment.
Added a deferral sentence.
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
|
I've updated based on feedback. @fuqianggao @norafang52 @markdroth @adisuissa PTAL. |
proposals/TP1-xds-transport-next.md
Outdated
|
|
||
| ### Discovery request and responses | ||
|
|
||
| State-of-the-world (SotW) `Discovery{Request,Response}` are not supported with |
There was a problem hiding this comment.
I'm okay with that, but I do think that we'll need another xRFC for the new naming scheme in SotW in the near future, because gRPC is planning to implement that. And that xRFC will be very short; it will basically say "everything from the original xRFC except for glob collections works for StoW too".
| ``` | ||
|
|
||
| Under normal conditions, the requested resource is returned as above. If the | ||
| config source for `some-cloud-authority` is unreachable, then the client issues |
There was a problem hiding this comment.
I think the client should always be trying to contact the original server, with appropriate backoff delay. Whenever the original server recovers enough to return a result, then the client should switch back to data from that server.
That having been said, I think I would be in favor of removing this from this xRFC, because we don't plan to implement this part in either gRPC or in Envoy in the near future. I think we can put together a separate xRFC for this once we actually have a need for it.
|
Sorry, this has idled a bit, I will update today. |
Signed-off-by: Harvey Tuch <htuch@google.com>
proposals/TP1-xds-transport-next.md
Outdated
| of `major.minor.patch` values, e.g. `"1.2.3"`. | ||
|
|
||
| * Context parameters from the URL, in the above example | ||
| `xds.bar=baz`. These must not overlap with any other context parameter name, |
There was a problem hiding this comment.
@markdroth should we put this in some namespace, e.g. xds.params. to future proof other parts of the xds. hierarchy in case we want them?
There was a problem hiding this comment.
I think we should say that the context params from the URI must not start with xds.. The params in the URI are for control planes to set however they want; the params in the xds. namespace should not be used for that, since they are reserved for future use by the API.
We are currently documenting xds.node. and xds.resource., which means that we are effectively reserving the entire remainder of the xds. namespace for future use.
proposals/TP1-xds-transport-next.md
Outdated
| An example computed URN following the above example is | ||
| `xdstp://some-authority/some.type/foo?xds.node.metadata.foo=bar&xds.shard_id=1234&xds.bar=baz&xds.client_feature.lb.least_loaded=false&xds.resource.vip=96.54.3.1`. | ||
|
|
||
| This proposal reserves the `@` prefix for context parameter values for future |
There was a problem hiding this comment.
@markdroth should we still keep this one?
There was a problem hiding this comment.
I suspect that we won't wind up using this anytime soon, but it's probably better to be safe than sorry.
Can we do something a bit more general, like maybe reserving all non-alphanumeric characters, just for the first character of the value?
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
|
@markdroth @fuqianggao @mattklein123 @norafang52 any further feedback or can we merge? I'd like to get the baseline RFC landed for us to iterate later. |
|
/lgtm |
|
LGTM to ship and iterate. |
proposals/TP1-xds-transport-next.md
Outdated
| of `major.minor.patch` values, e.g. `"1.2.3"`. | ||
|
|
||
| * Context parameters from the URL, in the above example | ||
| `xds.bar=baz`. These must not overlap with any other context parameter name, |
There was a problem hiding this comment.
I think we should say that the context params from the URI must not start with xds.. The params in the URI are for control planes to set however they want; the params in the xds. namespace should not be used for that, since they are reserved for future use by the API.
We are currently documenting xds.node. and xds.resource., which means that we are effectively reserving the entire remainder of the xds. namespace for future use.
proposals/TP1-xds-transport-next.md
Outdated
| An example computed URN following the above example is | ||
| `xdstp://some-authority/some.type/foo?xds.node.metadata.foo=bar&xds.shard_id=1234&xds.bar=baz&xds.client_feature.lb.least_loaded=false&xds.resource.vip=96.54.3.1`. | ||
|
|
||
| This proposal reserves the `@` prefix for context parameter values for future |
There was a problem hiding this comment.
I suspect that we won't wind up using this anytime soon, but it's probably better to be safe than sorry.
Can we do something a bit more general, like maybe reserving all non-alphanumeric characters, just for the first character of the value?
| ``` | ||
|
|
||
| Under normal conditions, the requested resource is returned as above. If the | ||
| config source for `some-cloud-authority` is unreachable, then the client issues |
There was a problem hiding this comment.
(Unresolving this conversation so that my last comment gets addressed.)
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
This ports
https://docs.google.com/document/d/1zZav-IYxMO0mP2A7y5XHBa9v0eXyirw1CxrLLSUqWR8/edit#
to xRFC format and updates to match changes that have occurred during
Envoy implementation.
This is xRFC number TP1.
Signed-off-by: Harvey Tuch htuch@google.com