-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add initial "NGINX Unit" based image #875
base: master
Are you sure you want to change the base?
Conversation
The Unit configuration I've included here is based on the example provided by the Unit documentation (https://unit.nginx.org/howto/wordpress/), although I did spend time understanding it before blindly trusting it. 😄 The biggest 😬 here (IMO) is that WordPress itself does not know about Unit yet, so the permalink handling is a bit off -- I've added a hack in `wp-config-docker.php` (specifically in the Unit images only) which works around it by teaching WordPress that Unit is effectively NGINX (which does work and is accurate enough for what WordPress needs), but that should probably be an issue or PR to the WordPress community instead.
Diff:$ diff -u <(bashbrew cat wordpress) <(bashbrew cat <(./generate-stackbrew-library.sh))
--- /dev/fd/63 2024-01-12 09:16:41.475066580 -0800
+++ /dev/fd/62 2024-01-12 09:16:41.479066614 -0800
@@ -3,47 +3,52 @@
Tags: 6.4.2-php8.1-apache, 6.4-php8.1-apache, 6-php8.1-apache, php8.1-apache, 6.4.2-php8.1, 6.4-php8.1, 6-php8.1, php8.1
Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, mips64le, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.1/apache
Tags: 6.4.2-php8.1-fpm, 6.4-php8.1-fpm, 6-php8.1-fpm, php8.1-fpm
Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, mips64le, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.1/fpm
Tags: 6.4.2-php8.1-fpm-alpine, 6.4-php8.1-fpm-alpine, 6-php8.1-fpm-alpine, php8.1-fpm-alpine
Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.1/fpm-alpine
Tags: 6.4.2-apache, 6.4-apache, 6-apache, apache, 6.4.2, 6.4, 6, latest, 6.4.2-php8.2-apache, 6.4-php8.2-apache, 6-php8.2-apache, php8.2-apache, 6.4.2-php8.2, 6.4-php8.2, 6-php8.2, php8.2
Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, mips64le, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.2/apache
Tags: 6.4.2-fpm, 6.4-fpm, 6-fpm, fpm, 6.4.2-php8.2-fpm, 6.4-php8.2-fpm, 6-php8.2-fpm, php8.2-fpm
Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, mips64le, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.2/fpm
Tags: 6.4.2-fpm-alpine, 6.4-fpm-alpine, 6-fpm-alpine, fpm-alpine, 6.4.2-php8.2-fpm-alpine, 6.4-php8.2-fpm-alpine, 6-php8.2-fpm-alpine, php8.2-fpm-alpine
Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.2/fpm-alpine
+Tags: 6.4.2-unit, 6.4-unit, 6-unit, unit, 6.4.2-php8.2-unit, 6.4-php8.2-unit, 6-php8.2-unit, php8.2-unit
+Architectures: amd64, arm64v8
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
+Directory: latest/php8.2/unit
+
Tags: 6.4.2-php8.3-apache, 6.4-php8.3-apache, 6-php8.3-apache, php8.3-apache, 6.4.2-php8.3, 6.4-php8.3, 6-php8.3, php8.3
Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, mips64le, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.3/apache
Tags: 6.4.2-php8.3-fpm, 6.4-php8.3-fpm, 6-php8.3-fpm, php8.3-fpm
Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, mips64le, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.3/fpm
Tags: 6.4.2-php8.3-fpm-alpine, 6.4-php8.3-fpm-alpine, 6-php8.3-fpm-alpine, php8.3-fpm-alpine
Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x
-GitCommit: ac65dab91d64f611e4fa89b5e92903e163d24572
+GitCommit: 3610d02f059743d924bcda64dcf0ffe839bd904e
Directory: latest/php8.3/fpm-alpine
Tags: cli-2.9.0-php8.1, cli-2.9-php8.1, cli-2-php8.1, cli-php8.1 |
What I think we need to resolve in order to merge this:
|
For 1., I guess that's really adapting the existing |
That's pretty cool @tianon! Paging @javorszky for assistance - I believe they have been involved with Unit/Wordpress integration lately on our team. |
@tianon hello! I'll be on this on Monday morning GMT timezone :) I'm super excited about this! |
Here's the simplified version of my current hack (for you to review Monday ❤️), since this PR probably isn't the easiest thing in the world to read. 😂 I'm essentially injecting this at the end of // ...
/** Sets up WordPress vars and included files. */
require_once ABSPATH . 'wp-settings.php';
// WordPress does not support NGINX Unit (yet), so we have to do a little hackery to let it know that Unit supports nice permalinks (and prevent it from insisting on injecting "/index.php" in all the permalink options) -- the simplest way is teaching it to recognize Unit as if it were NGINX (which is only semi-true, but true enough for these purposes)
$is_nginx = $is_nginx || str_starts_with($_SERVER["SERVER_SOFTWARE"], "Unit/");
// see also https://github.com/WordPress/WordPress/blob/39f7f558d91afdd2f3afc7f3b049a6a800cd3f80/wp-includes/vars.php#L127 More useful backlinks (ie, how I ended up at the conclusion I needed to patch
I guess a more "upstream-friendly" hack/workaround would be implementing a proper |
{ | ||
apache: [ "apache2-foreground" ], | ||
# https://github.com/nginx/unit/blob/fb33ec86a3b6ca6a844dfa6980bb9e083094abec/pkg/docker/Dockerfile.php8.2#L89 | ||
unit: [ "unitd", "--no-daemon", "--control", "unix:/var/run/control.unit.sock" ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nextcloud/docker#1610 (comment) makes an excellent case for probably removing --control
here 😅
Neat, I had some time to look into this. From what I understand this repository is responsible for generating the individual Dockerfiles for WordPress at the different tags. There are three immediate unit-specific observations:
For upstream work that we need the WordPress folks, I think we need to add two things:
|
Hey @tianon, Small update on where things are on our side:
This PR is an active item in our planning and we keep an eye on it 🙂 |
The Unit configuration I've included here is based on the example provided by the Unit documentation (https://unit.nginx.org/howto/wordpress/), although I did spend time understanding it before blindly trusting it. 😄
The biggest 😬 here (IMO) is that WordPress itself does not know about Unit yet, so the permalink handling is a bit off -- I've added a hack in
wp-config-docker.php
(specifically in the Unit images only) which works around it by teaching WordPress that Unit is effectively NGINX (which does work and is accurate enough for what WordPress needs), but that should probably be an issue or PR to the WordPress community instead.(FYI @thresheek - maybe you have more context or know someone who does about WordPress recognizing Unit natively and not injecting
/index.php
into permalinks when it's the server software? 👀)