-
Notifications
You must be signed in to change notification settings - Fork 1.9k
allow dynamic ip resolve on php handler #2413
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
Conversation
|
We use to copy the example from https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx-root.conf.sample and apply fixes directly in the upstream documentation. This is an edge case as it affects only the very specific case with nginx and backends running as container but it's not specific to Nextcloud. The examples are not meant to be production ready configurations but they make it easier to develop a custom configuration. I'm undecided whether it's worth to cover this in the examples or just refer to the documentation and examples of nginx. |
|
@J0WI The linked "nginx-root.conf.sample" actually doesn't have this issue, because the "upstream" block doesn't contain any domains, but rather a static IP address. This also prevents a DNS entry from being used for longer than permitted. Yes, I understand that my specific case is very specific. In general, the "docker-compose.yml" file has a problem as long as only the "app" container updates, and not the nginx proxy, which therefore still caches the old IP. I don't think that's a good idea. I can understand that you want to stick as close to the original template as possible, and my change represents a more or less significant deviation. I can also understand that the DNS entry in nginx should actually only be cached for the time specified by the TTL (at least according to my intuition), and that the problem could most elegantly be solved by nginx itself. But Nginx distinguishes between static and dynamic configuration, and once a domain is resolved in the static configuration, it won't be re-resolved, regardless of whether the information is out of date or not. And this is a property of nginx and its configuration as well as its performance optimization, and this is unlikely to change. The following applies to me right now:
Alternatively, other measures could be taken, but they would probably be just as unpopular.
I'm open to better suggestions for solving this problem. I just wanted to share my problem and my solution, because I think others might encounter this issue as well. I know my suggested solution isn't perfect, but I haven't come up with anything better. I'm happy to hear a better idea! |
|
https://blog.nginx.org/blog/dynamic-dns-resolution-open-sourced-in-nginx Ah - as I was writing this text, I looked for solutions again and saw that Nginx has open-sourced a "dynamic resolve" feature for the upstream. They mention the end of November 2024 in the blog, but I haven't checked which version this will be available from, or whether it might be an alternative solution to the problem. |
Signed-off-by: SebastianRzk <[email protected]>
|
Okay, I checked everything again and updated my branch accordingly. The PR now uses the new resolve feature. @J0WI What do you think? |
|
Hej @J0WI I've updated the PR based on your comment, and thanks to the new Nginx feature, I was able to keep the changes minimal. I still believe the changes make a valuable contribution to deployment with Docker Compose. Do you have any feedback on the current status? If not, we can close the PR and the issue. |
Problem:
When using
upstreamin the nginx config, the ip forappis resolved once. So if theappcontainer gets updated (e.g. throughwatchtower, the ip changes and the serverphp-handleris not reachable anymore.Fix:
set), to trigger the dns lookupSource / Inspiration:
bug on the watchtower github