istio: Pilot MCP sink shouldn't expect all collection types from source server
Describe the feature request
When the source server only provides a subset of collections (e.g. istio/networking/v1alpha3/VirtualService), pilot operates on just the data provided, without throwing this error for the collections not in the subset Error receiving MCP response: rpc error: code = InvalidArgument desc = unsupported collection "istio/mixer/v1/config/client/quotaspecs".
Describe alternatives you’ve considered The source server would have to send empty collection resources for all of the collections not being used. As new collection types are added, source servers for MCP resources need to send empty resources for these as well.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Reactions: 2
- Comments: 16 (13 by maintainers)
We need to consider the fact that not all the environments (K8s, CF, Consul, etc) are equal in the type of configuration they provide. Having a source that sends empty resources to bypass MCP errors does not seem very semantic. Discovery process seems very reasonable where a source and a server can establish a contract on what collection types they should exchange. Also, this will solidify the MCP contract and increases the efficiency (potentially) between source and the server and perhaps makes debugging less cumbersome. Currently, MCP controller initializes with an assumption that the source is going to provide all istio config types, but in the case of CloudFoundry, there are only a handful of types (ServiceEntries, Virtual Services, Gateways and destination rules) that are currently useful to the environment. For other types CF source server is forced to forward an empty resource. This also introduces code instability where the source server code has to change/break every time a new type is introduced in pilot.
Given our current plans moving away from MCP I don’t think we will implement this
Unsupported collections can occur as transient or permanent errors. Transient errors occur during upgrade/downgrade and should eventually resolve. Service may be disrupted until source/sink agree again. Permanent errors occur in CF-type use cases. Here are the solutions I’ve seen proposed so far:
(1) is the most complex and most explicit. Requires code changes to source and sink.
(2) is also explicit and is what we support today. Requires minimal code changes to sink if we want to skip over unsupported collections.
(3) requires the least amount of code change. It’s also the least explicit. The client can’t differentiate unsupported from empty collections. Requires minimal code changes to source.
I’d argue for enhancing (2) with better sink/client handling of unsupported collections. It’s simpler than (1) and doesn’t hide errors like (3). In practice, MCP connections will reset when either source or sink is upgraded, giving an opportunity for the list of supported collections to be reset. (2) could be further enhanced so that the sink doesn’t request collections it knows it doesn’t need (CF case).
I think it is reasonable to configure Pilot to only support (listen for) types that source can provide. This could also be a configuration where default is all the istio config types so we can guarantee that it will not impact config path for galley or other components, if one decides not to provide this config property. @ozevren & @ayj WDYT?