gateway icon indicating copy to clipboard operation
gateway copied to clipboard

Gateway API Controllers Should Watch Other Gateway API Resources

Open danehans opened this issue 2 years ago • 5 comments

Currently, Gateway API resources must be installed in a specific order for e2e functionality. A Gateway API controller should watch for changes to dependent resources. For example, the httproute controller should trigger an httproute reconciliation whenever a Gateway resource is CRUD'd.

danehans avatar Aug 31 '22 02:08 danehans

For those interested in helping with this issue, here is the use case that must be resolved.

  • A user creates an HTTPRoute- Which causes the httproute controller to reconcile the HTTPRoute, adds the HTTPRoute and associated service/namespace to the watchable map, and writes status (once #329 is fixed). Since a Gateway doesn't exist, the HTTPRoute status will not be written (even with #329 fixed). Since the HTTPRoute references a Gateway, e.g. spec.parentRefs[], that doesn't exist, I question whether the HTTPRoute should be written to the watchable map. Maybe the httproute controller should check for the availability of each spec.parentRefs[] and not process the route if the ref does not exist? Thoughts @skriss @LukeShu @arkodg?
  • A user creates a Gateway- Which causes the gateway controller to reconcile the Gateway. The Gateway doesn't get added to the watchable map because it's invalid (references a GatewayClass that doesn't exist).
  • A user creates a GatewayClass- Which causes the gatewayclass controller to reconcile the GatewayClass. The GatewayClass is added to the watchable map, status is set to "Accepted=true" but "Ready=true" is not set since the proxy infra has not been provisioned by the Infra Manager due to the invalid Gateway. A notification is not sent over the watchable channel since all 3 resources are required. Now that the GatewayClass has been created, the Gateway is no longer invalid. Since the gateway controller is not watching GatewayClasses, the GatewayClass does not get reconciled until the next periodic reconciliation.

All 3 resources exist but were not processed in the necessary order so the Infra and Xds IRs do not exist until either the next periodic reconciliation or the Gateway is recreated.

If the Gateway changes, e.g. address, status, deleted, etc. HTTPRoute reconciliation should be triggered to update HTTPRoute status.

danehans avatar Sep 15 '22 17:09 danehans

if there is a parent->child relationship b/w Gateway Class and Gateway and Gateway and HTTPRoute and order or accepted GatewayClass or Gateway matters/is sequential, maybe each reconciler could

  • Update itself to save the current accepted resource
  • force a reconcile (maybe by just calling child.Reconcile()) of the child resource
  • store the accepted resource in the watchable map

arkodg avatar Sep 15 '22 18:09 arkodg

All of the Gateway API resources are a system - they're not really meaningful by themselves, so you need to treat them as a system, and check the whole system any time one of the pieces is updated.

There are other resources like ReferenceGrant that can have wide-ranging effects on both Routes and Gateways if they change as well.

It's not a pure parent-child relationship because effects can flow both ways. Looking back, I think this the Contour approach of having a single Gateway API processor goroutine that accepts reconciliation events for all Gateway API resources may be a better idea.

youngnick avatar Sep 16 '22 00:09 youngnick

Currently, when an httproute is created without any add'l Gateway API resources, EG does a bunch of unnecessary work:

2022-09-20T23:47:51.530Z	INFO	kubernetes/httproute.go:122	reconciling httproute	{"runner": "provider", "namespace": "default", "name": "httpbin"}
2022-09-20T23:47:51.530Z	INFO	kubernetes/httproute.go:141	added httproute to resource map	{"runner": "provider", "namespace": "default", "name": "httpbin"}
2022-09-20T23:47:51.631Z	INFO	kubernetes/httproute.go:160	added namespace to resource map	{"runner": "provider", "namespace": "default", "name": "httpbin"}
2022-09-20T23:47:51.632Z	INFO	kubernetes/httproute.go:189	added service to resource map	{"runner": "provider", "namespace": "default", "name": "httpbin"}
2022-09-20T23:47:51.632Z	INFO	kubernetes/httproute.go:213	reconciled httproute	{"runner": "provider", "namespace": "default", "name": "httpbin"}

Since a Gateway doesn't exist, I don't see the benefit of processing an invalid route. Instead, the httproute reconciler should list all gateways and log an error if the parentRefs can't be resolved.

danehans avatar Sep 21 '22 00:09 danehans

All of the Gateway API resources are a system - they're not really meaningful by themselves, so you need to treat them as a system, and check the whole system any time one of the pieces is updated.

There are other resources like ReferenceGrant that can have wide-ranging effects on both Routes and Gateways if they change as well.

It's not a pure parent-child relationship because effects can flow both ways. Looking back, I think this the Contour approach of having a single Gateway API processor goroutine that accepts reconciliation events for all Gateway API resources may be a better idea.

+1 to this, I think it's far simpler to have an event in any one resource that we care about trigger a full re-reconciliation (in the translator) rather than trying to process changes incrementally/in isolation in individual reconcilers. As Nick said, there are wide-ranging effects across resources and I think we'd probably have to end up having all of our reconcilers watch all resource types anyway to be sure that we're covering all the edge cases related to order of creation, deletion, modification across resources.

skriss avatar Sep 21 '22 02:09 skriss