Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of [API Gateway] Fix infinite loop in controller and binding non-accepted routes and gateways into release/1.15.x #16378

Conversation

hc-github-team-consul-core
Copy link
Contributor

Backport

This PR is auto-generated from #16377 to be assessed for backporting due to the inclusion of the label backport/1.15.

The below text is copied from the body of the original PR.


Description

We were allowing gateways to bind routes that were in a "non-accepted" state in our gateway reconciliation path and not allowing them in the route reconciliation path. Because of the fact that a TCP route being added to a gateway and forcing the gateway into a "conflicted" state causes a status update to the gateway and a status update to the route, we could trigger an infinite loop where the route status was being written as "bound" and the gateway status as "conflicted" when the gateway was reconciled, and then it would trigger a route reconciliation marking the route as "unbound" and the gateway no longer as "conflicted". The mutual status setting would cause reconciliation to be triggered indefinitely.

The fix is to add a check inside of our binding logic that makes sure a route and gateway are both marked as "accepted" prior to binding them. This does mean that on their first pass through the reconciler neither gateways, nor routes will actually bind anything because they aren't considered "accepted" until the end of the reconciliation loop. This is ok, because a second reconciliation run will be triggered when the status gets updated and routes will bind on the second pass.

Testing & Reproduction steps

Created the config entries in the following order:

Service one defaults - tcp protocol:

kind = "service-defaults"
name = "service-one"
protocol = "tcp"

Service two defaults - http protocol:

kind = "service-defaults"
name = "service-two"
protocol = "http"

TCPRoute one (should be valid):

kind = "tcp-route"
name = "api-gateway-route-one"
services = [
  {
    name = "service-one"
  }
]

parents = [
  {
    sectionName = "listener-one"
    name = "api-gateway"
  }
]

TCPRoute two (should be invalid due to mismatched protocol on the service):

kind = "tcp-route"
name = "api-gateway-route-two"
services = [
  {
    name = "service-two"
  }
]

parents = [
  {
    sectionName = "listener-one"
    name = "api-gateway"
  }
]

Gateway:

kind = "api-gateway"
name = "api-gateway"
listeners = [
  {
    name = "listener-one"
    port     = 9999
    protocol = "tcp"
  }
]

The final write for the gateway kicks off the death loop. It binds both TCPRoutes and sets itself in a "conflicted" state because it was ignoring whether or not the route was actually valid. Binding the routes triggers their reconciliation loops since we write to the route bound status. The route reconciliation would then unbind, taking the gateway out of conflicted, and triggering the gateway reconciliation -- this continued on mutually triggering re-reconciliation infinitely.

PR Checklist

  • updated test coverage
  • external facing docs updated
  • not a security concern

Overview of commits

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/fix-non-binding-routes-and-gateways/formally-sensible-bluegill branch 2 times, most recently from 6d757a5 to 3a9893f Compare February 22, 2023 19:55
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto approved Consul Bot automated PR

@hc-github-team-consul-core hc-github-team-consul-core merged commit 61aeb81 into release/1.15.x Feb 22, 2023
@hc-github-team-consul-core hc-github-team-consul-core deleted the backport/fix-non-binding-routes-and-gateways/formally-sensible-bluegill branch February 22, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants