Procfile: run nginx via docker #1035
Conversation
This should help to standardise the nginx version running on different dev machines. Closes getodk#1033
| daemon off; | ||
| pid ./.nginx/nginx.pid; | ||
| error_log /dev/stdout; | ||
| pid /tmp/nginx.pid; |
There was a problem hiding this comment.
Maybe error_log /dev/stderr?
| proxy_temp_path ./.nginx/proxy_temp 1 2; | ||
| access_log /dev/stdout; | ||
| client_body_temp_path /tmp/client_body_temp; | ||
| proxy_temp_path /tmp/proxy_temp 1 2; |
There was a problem hiding this comment.
is this necessary?
There was a problem hiding this comment.
The temp paths?
In #1017 it shows that nginx can have default configuration for those paths (baked in even, perhaps?) in non-user-writable locations, which then gives problems running it unprivileged (as one does).
There was a problem hiding this comment.
The rationale for this PR was the breakage that you experienced on nginx 1.18 as found in Ubuntu 20.04 LTS (from 2020, current version is 1.27, note that support for 20.04 ends in six months…). Fair enough! Let's aim to be broadly compatible and minimally invasive.
With that mindset, in the culprit PR that introduced the breakage for you (#1017) it says:
Why is this the best possible solution? Were any other approaches considered?
Theoretically we could do without 4527b7b, but that leaves (at least in my case) a warning in the nginx output, which (even though apparently harmless) then either makes people freak out, or can detract from an actual problem, or trains people to ignore warnings from nginx, all of which are undesirable.
Reverting 4527b7b, which was a mostly cosmetic change (and for that reason a separate commit in PR #1017), should fix your woes, right?
So could we just do that instead of running containers? I can't review a whole container 😆 while the bring-your-own-nginx leaves it up to the user (the dev) to source their nginx ethically and organically and safely.
Yet if there has to be a container, could we have it as an option via the Makefile? Eg "make dev-docker" or something.
If that is also not acceptable, could we then at least have a make dev-BYON or somesuch to do what make dev currently does (running the bring-your-own-nginx)?
So my envisioned resolutions, in order of preference are:
- No docker. We just revert 4527b7b. ✨ Order is restored in the galaxy.
- A docker container, but it's behind
make dev-docker(or somesuch). Someone will have to review that container. - A docker container (that someone will have to review), behind
make dev, and whatmake devused to do becomesmake dev-oldschoolormake dev-bring-your-own-nginxor somesuch (imagine a good name).
@ktuite @matthew-white please, co-opine. 🤔
That would work for now 👍 I think a dockerised approach will be more robust long-term. Getting local nginx working on non-OSX was work (#621); avoiding future problems would be really handy.
What's the argument for allowing dev environments to easily deviate? |
I'm on 22.04 which is supported until 2027: https://packages.ubuntu.com/search?keywords=nginx&searchon=names&suite=jammy§ion=all |
Pardon me ;-) my research towards where one would find nginx 1.18 wasn't very thorough. |
Aren't we confusing policy and implementation? We can easily have the policy "let's use nginx 1.18 and up". |
|
I agree that if we have a Docker option, we should also have a non-Docker option. Historically for Central, we've made it possible to run things locally without Docker. Not everyone uses a lot of Docker, and I like the idea of developers being able to get started on Central without digging into Docker. That said, I also know that Docker can be very convenient.
I definitely want it to be the case that someone starting on Central is able to get nginx up and running with minimal effort ("install nginx with version X or above"). That said, if reverting 4527b7b is enough to get nginx running on our variously configured machines, maybe that shows that our nginx config is already battle-hardened. On a separate note, I really like the idea of moving the nginx config files to their own directory and out of the repo's root directory. I've had to add some files to the root directory, so it's been feeling a little cluttered lately. |
That sounds right to me that if 1.18 is LTS, we should try to support it. One option is that we revert 4527b7b now, but plan to add it back once |
I'll open a PR for this. |
nginx's -e switch was added in v1.19.5. Recent Ubuntu LTS versions (20.04, 22.04) bundle nginx version 1.18.0. See: https://nginx.org/en/CHANGES See: getodk#1035 nginx 1.18.0, distributed with Ubuntu LTS 20.04 & 22.04
nginx's -e switch was added in v1.19.5. Recent Ubuntu LTS versions (20.04, 22.04) bundle nginx version 1.18.0. See: https://nginx.org/en/CHANGES See: #1035 nginx 1.18.0, distributed with Ubuntu LTS 20.04 & 22.04 Closes #1033
This should help to standardise the nginx version running on different dev machines.
Closes #1033