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

Ingress controller leaves replacement proxy containers without pushed configuration #2107

Closed
1 task done
seh opened this issue Dec 16, 2021 · 7 comments · Fixed by Kong/kong#8214 or #2343
Closed
1 task done
Assignees
Labels
bug Something isn't working priority/high

Comments

@seh
Copy link
Contributor

seh commented Dec 16, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Please see Kong/kong#8152 for where this started—in particular, starting with Kong/kong#8152 (comment). To summarize, if the ingress controller starts up successfully, pushes configuration to the proxy container at least once, and then the proxy container exits (perhaps killed by the kubelet due to its liveness probe failing), but when a replacement proxy container later starts, the ingress controller will not bother to push any configuration to the proxy until some observed cluster state changes materially. This is only the case when the "reverse sync" feature is disabled, as it is by default.

The ingress controller decides against pushing the configuration to the proxy as an optimization: Finding that the serialized configuration's message digest hasn't changed since the last successful push, there's seemingly no reason to push what would be the same configuration again. Unfortunately, the proxy container on the other side of that exchange is a different one from the last one that received the intended configuration.

Since the ingress controller's readiness gate is latching, once it's pushed one configuration successfully, it remains ready forevermore. Ideally we'd clear this flag each time that we detect that the proxy container has either no configuration or has fallen behind by some number of attempted updates.

Expected Behavior

The ingress controller should have a way to detect that the current proxy container lacks the intended configuration. There are several potential approaches; assuming that we wish to retain the current optimization, all of these straw men are more complicated than what we have today:

  • Create an emptyDir volume in the pod, mount it into both the ingress controller and proxy containers, have the ingress controller create and listen on a domain socket, and have the proxy container connect to it. When the proxy container hangs up, the ingress controller should assume that the next proxy container needs fresh configuration.
  • Create an emptyDir volume, mount it into both the ingress controller and proxy containers, have the proxy container read its container ID from the /proc/self/cgroup file, and write it to a file in that shared volume. The ingress controller can watch that file's content and react to the container ID changing, assuming that a change means that the next proxy container needs fresh configuration.
  • Grant RBAC permission for Kong's ingress controller to watch pods. The ingress controller would demand its own pod namespace and name via the Downward API, and it would open a watch on its own pod. From the updates, it can read the "status.containerStatuses[1].containerID" field to learn when the proxy container gets replaced.

Steps To Reproduce

  1. Create a pod with both the ingress controller and proxy containers.
  2. Confirm that the ingress controller becomes ready.
  3. Use Kong's admin API to confirm that it has a route and service for at least one set of Kubernetes Ingress and Service objects.
  4. Ensure that the observable cluster state remains stable.
  5. Kill the proxy container.
  6. Confirm that a replacement proxy container starts.
  7. Wait at least three seconds.
  8. Use Kong's admin API to confirm that it has no routes or services.

Kong Ingress Controller version

v2.0.6

Kubernetes version

version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.9", GitCommit:"9dd794e454ac32d97cde41ae10be801ae98f75df", GitTreeState:"clean", BuildDate:"2021-03-18T01:00:06Z", GoVersion:"go1.15.8", Compiler:"gc", Platform:"linux/amd64"}
@rainest
Copy link
Contributor

rainest commented Dec 16, 2021

Tentative plans are to add a Kong admin API endpoint that returns status code feedback as to whether a config has been loaded or not, and have the controller watch that, as well as use it as the Kong readiness endpoint instead of /status (which is more suited to liveness).

Lack of config is relevant to non-controller-managed environments also, so we want to add something that supports them as well.

@mflendrich
Copy link
Contributor

Maybe a container hook notifying KIC when a proxy container restarts could be of use here.

@mflendrich
Copy link
Contributor

mflendrich commented Jan 11, 2022

blocked on Kong/kong#8256

@mflendrich
Copy link
Contributor

update: Kong/kong#8214 is an alternative solution recommended by @dndx over Kong/kong#8256

therefore, now blocked on unclosing and getting Kong/kong#8214 across the line

@jcam
Copy link

jcam commented Feb 28, 2022

^ Kong/kong#8214 has been merged. Do you need contributors here?

@rainest
Copy link
Contributor

rainest commented Feb 28, 2022

It has been merged, not released. We have code for this staged at https://github.com/Kong/kubernetes-ingress-controller/tree/feat/watch-config-hash but can't merge it into our repos until 2.8 is actually out, since nightly builds lack semver versions, and we need those to run the tests normally.

@jcam
Copy link

jcam commented Feb 28, 2022

Ah okay. Well glad to see you're already on it! I'll be taking this as soon as I can :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment