Do not create clusters for unrelated services

[davecheney]

Currently Contour creates CDS entries for any Service document visible via the API. This is a problem because a CDS entry a causes Envoy to open a new EDS gRPC stream. This inflates the number of connections to the xds_cluster and inflames issues like #291

We should filter Services to only those that are directly referenced by an active Ingress.

[stevesloka]

I can work on this issue.

[davecheney]

Cool. I haven't thought about how it would work but there would need to be some connection between onadd/update/delete for ingress and service. FWIW I don't think this is super urgent, the issues with connection limits are best solved by adjusting the bootstrap config on the xds_cluster entry.

…

On 27 March 2018 at 07:56, Steve Sloka ***@***.***> wrote: I can work on this issue. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#298 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcA5NXnJfX96xK6udYCWIWsBwGoQzvks5tiVXrgaJpZM4S5GM6> .

[Lookyan]

I think we need to use our local cache slices to determine whether we need endpoints and clusters in envoy. So, we need to store all data about endpoints and ingresses in memory of contour (and it already does it). Otherwise, we can get a race condition when we get new ingress but we don't have any clusters and endpoints for it. Or we should query kube API then to retrieve needed information (besides k8s watchers).

@stevesloka if you don't have proximate plans to realize this feature I can try to implement it. Let me know if you already work on it.

[stevesloka]

@Lookyan I did start a little bit but don't have it fully implemented. I was starting to wire in the SharedInformer since it already maintains the local cache, and allows us to better utilize the update methods and also doesn't require us to maintain the second cache in the translatorCache struct. My work is here: https://github.com/stevesloka/contour/tree/healthcheck

The secondary goal of this issue is to implement health checks since they are implemented via clusters, this means I think we should build a cluster per service per reference in ingress since multiple ingress objects could reference the same service and have differing health checks.

[davecheney]

I appreciate your enthusiasm but please let’s wait until #291 is fixed.

My suspicion is the next blocker will be eds filtering the specific resource being watched (there is an issue, but on phone)

[davecheney]

#291 is now addressed, but 0.5.0 is shipping in two weeks (and one of those weeks I will be on leave) so I am moving this to 0.6.