From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34004) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e37eg-0002Db-97 for guix-patches@gnu.org; Fri, 13 Oct 2017 17:38:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e37ec-0007UU-6v for guix-patches@gnu.org; Fri, 13 Oct 2017 17:38:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:58082) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1e37ec-0007UO-1t for guix-patches@gnu.org; Fri, 13 Oct 2017 17:38:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1e37eb-0007YQ-RK for guix-patches@gnu.org; Fri, 13 Oct 2017 17:38:01 -0400 Subject: [bug#28769] [PATCH] gnu: services: Add php-fpm. Resent-Message-ID: Date: Fri, 13 Oct 2017 22:37:29 +0100 From: Christopher Baines Message-ID: <20171013223729.2605f33c@cbaines.net> In-Reply-To: <9fe1701f-d78f-ba3a-37eb-64417337a55b@cock.li> References: <9fe1701f-d78f-ba3a-37eb-64417337a55b@cock.li> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; boundary="Sig_/Qc8BM_/nhJCc9gM/Eo9Ab.L"; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: nee Cc: 28769@debbugs.gnu.org --Sig_/Qc8BM_/nhJCc9gM/Eo9Ab.L Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Mon, 9 Oct 2017 23:54:24 +0200 nee wrote: > Subject: [PATCH 2/2] gnu: services: Add php-fpm. Hey, I've never used php-fpm, but I'll have a go at reviewing this :) > * gnu/services/web.scm (, > ): New record types. > (php-fpm-configuration?, > php-fpm-process-manager-configuration?, > php-fpm-service-type, > nginx-php-location): New procedures. > * doc/guix.texi (Web-Services): document php-fpm service. Very minor, but I'd suggest "(Web Services): Document the php-fpm service." here. > --- > doc/guix.texi | 93 +++++++++++++++++++++++++++ > gnu/services/web.scm | 173 > ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, > 264 insertions(+), 2 deletions(-) >=20 > diff --git a/doc/guix.texi b/doc/guix.texi > index f0a59a6b4..ed4336f64 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -14529,6 +14529,99 @@ capability also has to be configured on the > front-end as well. @end deftp > =20 > =20 > +@cindex php-fpm > +PHP-FPM (FastCGI Process Manager) is an alternative PHP FastCGI implemen= tation > +with some additional features useful for sites of any size. If these additional features are worth mentioning, it would be best to be specific as to what they are, and what benefit they provide. > +@defvr {Scheme Variable} php-fpm-service-type > +A Service type for @code{php-fpm}. > +@end defvr > + ... > +@deftp {Data type} php-fpm-process-manager-configuration > +Data Type for php-fpm worker process limits. > +@table @asis > +@item @code {type} (default: @code{"dynamic"}) > +@table @asis > +@item @code{"dynamic"} > +Spare worker processes are kept around based on the set @code{php-fpm-pr= ocess-manager-configuration} limits. > +@item @code{"static"} > +A static number of worker processes is created. > +@item @code{"ondemand"} > +Worker processes are only created on demand. > +@end table > +@item @code {max-children} (default: @code{5}) > +Maximum of worker processes. Applies when the type is @code{"static"}, @= code{"dynamic"}, or @code{"ondemand"}. > +@item @code {start-servers} (default: @code{2}) > +How many worker processes should be started on start-up. Only applies wh= en type is @code{"dynamic"}. > +@item @code {min-spare-servers} (default: @code{1}) > +How many spare worker processes should be kept around at minimum. Only a= pplies when type is @code{"dynamic"}. > +@item @code {max-spare-servers} (default: @code{3}) > +How many spare worker processes should be kept around at maximum. Only a= pplies when type is @code{"dynamic"}. > +@item @code {process-idle-timeout} (default: @code{10}) > +The time in seconds after which a process with no requests is killed. On= ly applies when type is @code{"ondemand"}. > +@end table > +@end deftp Reading this makes me think that maybe having record types per process manager types might be useful, e.g. > +@defvr {Scheme Variable} nginx-php-fpm-location > +A helper function to quickly add php to an > @code{nginx-server-configuration}. +@end defvr > + > +A simple services setup for nginx with php can look like this: > +@example > +(services (cons* (dhcp-client-service) > + (service php-fpm-service-type > + (php-fpm-configuration)) The default-value for the service type means that you can omit the (php-fpm-configuration) here, as (service php-fpm-service-type) should work. > + (service nginx-service-type > + (nginx-server-configuration > + (server-name '("example.com")) > + (root "/srv/http/") > + (locations > + (list (nginx-php-location))) > + (https-port #f) > + (ssl-certificate #f) > + (ssl-certificate-key #f))) > + %base-services)) > +@end example > + > + > @node DNS Services > @subsubsection DNS Services > @cindex DNS (domain name system) > diff --git a/gnu/services/web.scm b/gnu/services/web.scm > index 9d713003c..fd63b15bb 100644 > --- a/gnu/services/web.scm > +++ b/gnu/services/web.scm > @@ -26,8 +26,11 @@ > #:use-module (gnu system shadow) > #:use-module (gnu packages admin) > #:use-module (gnu packages web) > + #:use-module (gnu packages php) > #:use-module (guix records) > #:use-module (guix gexp) > + #:use-module ((guix utils) #:select (version-major)) > + #:use-module ((guix packages) #:select (package-version)) > #:use-module (srfi srfi-1) > #:use-module (ice-9 match) > #:export ( > @@ -76,7 +79,14 @@ > =20 > fcgiwrap-configuration > fcgiwrap-configuration? > - fcgiwrap-service-type)) > + fcgiwrap-service-type > + > + php-fpm-configuration > + php-fpm-configuration? > + php-fpm-process-manager-configuration > + php-fpm-process-manager-configuration? > + php-fpm-service-type > + nginx-php-location)) When using other Guix services, I've run in to problems when field accessors and record types were not exported. The biggest cost I can think of is the lines of code, but I'd still suggest to export everything by default here. > ;;; Commentary: > ;;; > @@ -256,10 +266,12 @@ of index files." > "events {}\n"))) > =20 > (define %nginx-accounts > - (list (user-group (name "nginx") (system? #t)) > + (list (user-group (name "www-data") (system? #t)) > + (user-group (name "nginx") (system? #t)) > (user-account > (name "nginx") > (group "nginx") > + (supplementary-groups '("www-data")) > (system? #t) > (comment "nginx server user") > (home-directory "/var/empty") Pulling in your comment about the accounts... > About the accounts: > Nginx needs write access to the unix socket of php-fpm. I didn't want > to set nginx as default user for php-fpm in case we add other > webservers, so I added the nginx to the newly created www-data group. This sounds like it would work, however, the defaults for the socket-user and socket-group of the php-fpm-configuration are currently "nginx", which seems to disagree with what you say above? Also, the idea that came to my mind for this is to add php-fpm as a supplementary group for the nginx user. In my mind, this seems the neatest way of giving nginx access to the socket. What do you think? I suggest this, as I'm not sure the name of the www-data group does a good job of describing that it's controlling access to a socket. Also, giving nginx explicit access to things owned by the php-fpm group could be more secure than using a more generic group, especially as other services that might need access to the socket, could end up getting it because they need access to other things using the www-data group. If the above sounds like a good idea, I think it could be implemented by adding a configurable list of supplementary groups to the nginx-service-type. Then maybe even adding support for configuring this via the service extensions mechanism, then having the php-fpm-service-type extending the nginx-service-type... The above is definitely to complicated to do all in one go, especially since the nginx-service-type doesn't support anything more complicated than adding server blocks through extension at the moment. > @@ -385,3 +397,160 @@ of index files." > (service-extension account-service-type > fcgiwrap-accounts))) > (default-value (fcgiwrap-configuration)))) > + > +(define-record-type* php-fpm-configuration > + make-php-fpm-configuration > + php-fpm-configuration? > + (php php-fpm-configuration-php ; > + (default php)) > + (socket php-fpm-configuration-socket > + (default (string-append "/var/run/php" > + (version-major > (package-version php)) > + "-fpm.sock"))) > + (user php-fpm-configuration-user > + (default "php-fpm")) > + (group php-fpm-configuration-group > + (default "php-fpm")) > + (socket-user php-fpm-configuration-socket-user > + (default "nginx")) > + (socket-group php-fpm-configuration-socket-group > + (default "nginx")) As above, I'm not sure the use of nginx here matches the description you gave...? > + (process-manager php-fpm-configuration-process-manager > + (default (php-fpm-process-manager-configuration))) > + (display-errors php-fpm-configuration-display-errors > + (default #f)) > + (workers-logfile php-fpm-configuration-workers-logfile > + (default (string-append "/var/log/php" > + (version-major > (package-version php)) > + "-fpm.log"))) I'm not sure the php is adding much to the log file name, but then again I've never used php... what do you think? > + (file php-fpm-configuration-file ;#f | file-like > + (default #f))) > + > +(define-record-type* > php-fpm-process-manager-configuration > + make-php-fpm-process-manager-configuration > + php-fpm-process-manager-configuration? > + (type php-fpm-process-manager-configuration-type > + (default "dynamic")) > + (max-children > php-fpm-process-manager-configuration-max-children > + (default 5)) > + (start-servers > php-fpm-process-manager-configuration-start-servers > + (default 2)) > + (min-spare-servers > php-fpm-process-manager-configuration-min-spare-servers > + (default 1)) > + (max-spare-servers > php-fpm-process-manager-configuration-max-spare-servers > + (default 3)) > + (process-idle-timeout > php-fpm-process-manager-configuration-process-idle-timeout > + (default 10))) > + > +(define php-fpm-accounts > + (match-lambda > + (($ php socket user group socket-user > socket-group _ _ _ _) > + (filter identity > + (list > + (user-group (name "www-data") (system? #t)) > + (and (equal? group "php-fpm") > + (user-group > + (name "php-fpm") > + (system? #t))) > + (and (equal? user "php-fpm") > + (user-account > + (name "php-fpm") > + (group group) > + (supplementary-groups '("www-data")) > + (system? #t) > + (comment "web services group") > + (home-directory "/var/empty") > + (shell (file-append shadow > "/sbin/nologin"))))))))) + As you can specify the user and group in the , I think it might be better to just use the user and group names from there, and always setup a user and group with those names, I'm guessing that this will be what the average user would expect to happen. > +(define (default-php-fpm-config socket user group socket-user > socket-group > + pm display-errors workers-logfile) > + (match > + pm > + (($ pm.type > + pm.max-children > + pm.start-servers > + pm.min-spare-servers > + pm.max-spare-servers > + > pm.process-idle-timeout) > + (apply mixed-text-file "php-fpm.conf" > + "[global]\n" > + "[www]\n" > + "user =3D" user "\n" > + "group =3D" group "\n" > + "listen =3D" socket "\n" > + "listen.owner =3D" socket-user "\n" > + "listen.group =3D" socket-group "\n" > + > + "pm =3D" pm.type "\n" > + "pm.max_children =3D" (number->string pm.max-children) "\n" > + "pm.start_servers =3D" (number->string pm.start-servers) > "\n" > + "pm.min_spare_servers =3D" (number->string > pm.min-spare-servers) "\n" > + "pm.max_spare_servers =3D" (number->string > pm.max-spare-servers) "\n" > + "pm.process_idle_timeout =3D" (number->string > pm.process-idle-timeout) "s\n" + > + "php_flag[display_errors] =3D " (if display-errors "on" > "off") "\n" + > + (if workers-logfile > + (list "catch_workers_output =3D yes\n" > + "php_admin_value[error_log] =3D" workers-logfile > "\n" > + "php_admin_flag[log_errors] =3D on\n") > + (list "catch_workers_output =3D no\n")))))) > + > +(define php-fpm-shepherd-service > + (match-lambda > + (($ php socket user group socket-user > socket-group > + pm display-errors workers-logfile > file) > + (list (shepherd-service > + (provision '(php-fpm)) > + (documentation "Run the php-fpm daemon.") > + (requirement '(networking)) > + (start #~(make-forkexec-constructor > + '(#$(file-append php "/sbin/php-fpm") > + "--nodaemonize" "-p" "/var" "--fpm-config" > + #$(or file > + (default-php-fpm-config socket user > group > + socket-user socket-group pm > display-errors > + workers-logfile))))) > + (stop #~(make-kill-destructor))))))) As php-fpm supports using a pid file, I'd recommend configuring both php-fpm and shepherd to use this by default. It means that the shepherd service will wait until php-fpm creates the PID file before starting services that say they require it, which can prevent some issues. > +(define php-fpm-activation > + (match-lambda > + (($ _ _ user _ _ _ _ _ workers-logfile _) > + #~(begin > + (use-modules (guix build utils)) > + (let ((user (getpwnam #$user)) > + (touch (lambda (file-name) > + (call-with-output-file file-name (const > #t))))) > + ;; prepare writable logfile > + (when #$workers-logfile > + (when (not (file-exists? #$workers-logfile)) > + (touch #$workers-logfile)) > + (chown #$workers-logfile (passwd:uid user) (passwd:gid > user)) > + (chmod #$workers-logfile #o660))))))) > + > + > +(define php-fpm-service-type > + (service-type (name 'php-fpm) > + (extensions > + (list (service-extension shepherd-root-service-type > + php-fpm-shepherd-service) > + (service-extension activation-service-type > + php-fpm-activation) > + (service-extension account-service-type > + php-fpm-accounts))) > + (default-value (php-fpm-configuration)))) Filling in the description (a relatively new field on the service type) would be a great addition here. > +(define* (nginx-php-location > + #:key > + (nginx-package nginx) > + (socket (string-append "/var/run/php" > + (version-major (package-version > php)) > + "-fpm.sock"))) > + "Return a nginx-location-configuration that makes nginx run .php > files." > + (nginx-location-configuration > + (uri "~ \\.php$") > + (body (list > + "fastcgi_split_path_info ^(.+\\.php)(/.+)$;" > + (string-append "fastcgi_pass unix:" socket ";") > + "fastcgi_index index.php;" > + (list "include " nginx-package > "/share/nginx/conf/fastcgi.conf;"))))) This helper seems good, although I think putting the "include" elsewhere would be better, and I recognise that this isn't possible with the nginx service yet. Overall, I think this is great. Excellent use of record types, gexps and match expressions. I think it would be good to think more on the accounts issue before merging, but that is all that comes to mind currently. Also, if you feel like it, as service test would be a great addition to this patch. There is a test for nginx already in gnu/tests/web.scm, and I think you could get most of the benefit by having a test for nginx with php-fpm, as that would give you some coverage over the php-fpm service, as well as the nginx configuration. --Sig_/Qc8BM_/nhJCc9gM/Eo9Ab.L Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlnhMhlfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE 9XcSYg/+NNhbP1jgyrloI41SIMbKWNozRSwcCLIpziGsir8r79x6MYgLnS6Ou9kP ci5GMjEcCwbne6JgbfcDghfvZ244WkuhK8gGtNuRE/80+7xdFM1PtJJNTWlyROfi PVsiOjo3BEi18OBr0on/811rRHeac1Kzc71JfH9mlA4W0xA1bQFs2p33ModPA1RG uKc5eJE4ieeH24C8sxxJz68JOaN4NA9mKx99+b9MMwUR0b7sn9o2DhHizXj5CYcd 8ho++VyZ45AMJlU4BEucW2G5g0gWYC5DeYsyyP8nr+H7l8WZiAVa/J2L/ZcAoh/U +NEPw6+5OiYd67ttFMolcRgLiwaMytklcoHp9B2iHvgXnGWAN5ALCwXkLEs15zRL Io6YnLHsVDf9ELjMXTwquBRV4AjCq/rwPRqqZEez50Nxl+xc48QLo/ElglEgseA0 5Cln6ymzZE2eK0yt7RJr3uMyMYL6d8rgwXEE2VNML4MxfxjFiTAX8OCkBBufx2+b WFpDd9khaYFN3TgKYiaq1od4/J03tgZI9AS7HmJ8fWeKFFkWB556E9es9MNdoJW2 n93Su4KSgI2szHdvaBbOZ7+ET567qQ7x6eCsL6hi9YmbI0UgjHeQu3evOouws9oS MGHzKMbe1potlmfFqs5KDJVVbM/n8zwU/oxxndhMaQmZkBEuhtc= =NPA9 -----END PGP SIGNATURE----- --Sig_/Qc8BM_/nhJCc9gM/Eo9Ab.L--