-
Notifications
You must be signed in to change notification settings - Fork 10
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
initContainer to build nginx config? #33
Comments
Can you elaborate on what problem that would solve? |
@krancour : We encountered a problem last week where building the initial config inside the nginx containers was taking longer than the
Maybe it's not worth investing more time in the router if native ingress is the way to go for the future. |
I agree native ingress is a better choice. That said, if there's an immediate need to improve the router while remaining backward compatible, you might consider turning it into a proper k8s controller. That's not necessarily easy, to be honest, but it offers a distinct improvement in that you aren't in an infinite loop of looking at all services and determining what the changes were, as the router does today. Instead you're in an infinite loop, subscribed to discrete changes. It's waaaay more efficient. Again... not necessarily easy (I don't think) to write a controller, but it's the canonical approach to this problem. You could very well consider forking the existing Nginx ingress controller and adapting it to watch annotated services instead of ingresses. |
I think the main issue here would be that livenessProbe might be set to low, because readinessProbe will prevent traffic from reaching the router during startup and livenessProbe should detect if it is hung and needs to be restarted. |
It would be good if we could test this at high load and see if the problem is solved when |
Should we investigate if using initContainer to build the initial nginx config using the Go k8s-client worth the investment in effort? Or even postStart lifecycle hooks? @krancour @kingdonb ?
The text was updated successfully, but these errors were encountered: