Aggregate ServiceImports

[vthapar]

Currently ServiceImports are distributed as individual copy from each source cluster and exist that way. Aggregation is done in Plugin code, which is not optimal and not in compliance with the MCS API spec. Agent should aggregate ServiceImports into a single resource on destination clusters and plugin should use those. This will also make it easy to troubleshoot.

See enhancement proposal for details.

[stale]

This issue has been automatically marked as stale because it has not had activity for 60 days. It will be closed if no further activity occurs. Please make a comment if this issue/pr is still valid. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had activity for 60 days. It will be closed if no further activity occurs. Please make a comment if this issue/pr is still valid. Thank you for your contributions.

[tpantelis]

bump

[dfarrell07]

Discussed on the PR scrub today. This seems important for folks that want to use Lighthouse service discovery with something else to provide connectivity, see #733. This also relates to namespaces, making an export in SubM makes the service available in all clusters vs only the clusters with a matching namespace. Also scale, having aggregate service imports would reduce the data to sync.

[dfarrell07]

It seems a user is asking for this in #957.

[tpantelis]

@vthapar @aswinsuryan I think we should do this for 0.15. I can work on it with your SME guidance. WDYT? cc @nyechiel

[aswinsuryan]

ya, it is a long pending task, it would be good if we can get this done. Since lighthouse coredns is not going to consume this we may not have an impact in DNS resolution logic. Also to start with we can leave the IP field in the ServiceImport blank and focus on conflict resolution, like validating if all the ServiceImport with the same name and in the same namespace have the same service type.

[tpantelis]

Aggregating would reduce the data to sync but we'd also lose the cluster-specific port information which is needed for per-cluster DNS requests. The spec doesn't cover that.

Also lighthouse assigns per-cluster IPs to the ServiceImports and not a clusterset-wide VIP as the spec seems to assume. So the IP field in an aggregated ServiceImport wouldn't be relevant. We could leave it blank but that might not be compliant either.

Re: port conflicts, the spec states:

A derived service will be accessible with the clusterset IP at the ports dictated by child services. If the external properties of service ports for a set of exported services don’t match, the clusterset service will expose the union of service ports declared on its constituent services.

We currently assume the ports on each cluster service match. We can still detect a mismatch in the agent controller and report a conflict condition on the local ServiceExport without aggregating. In the coredns plugin, we can perform the port union in memory.

I propose we not aggregate ServiceImports, at least not now. This comment above cites a potential use case but it seems to require a clusterset-wide VIP, in which case we'd need additional work to support it beyond just aggregating. I don't think we should create an aggregate just for the sake of it, especially if an empty IP pretty much renders it useless.

[tpantelis]

@aswinsuryan The K8s Endpoints and EndpointSlice objects have port information that are derived from the service. For a headless service, we obtain the ports from the EndpointSlice for SRV queries. It seems we should be able to do the same for non-headless services. If we also sync the service IP via the EndpointSlice then we really wouldn't need the per-cluster ServiceImports.

On the MCS SIG call this week, it was stated that the intention was for per-cluster information to be communicated via EndpointSlices, which is what Google's implementation does (they also assign per-cluster service IPs although it's unclear to me how they store that in the EndpointSlice, probably as an Endpoint or perhaps an annotation). That said, they also discussed moving away from that paradigm and storing the per-cluster info in the ServiceImport.