[bug#28769] [PATCH] gnu: services: Add php-fpm.

[Christopher Baines]

nee writes:

> Hello sorry about not replying for such a long time, and thank you for
> reviewing my patches again.
>
> Am 02.11.2017 um 20:17 schrieb Christopher Baines:
>>
>> I've now attached a system test for php-fpm, that sets it up with
>> nginx, creates a basic php file, and checks that it can be requested.
>
> That's great thanks for saving me a bunch of work with this. ;-)
>
>>
>> This seems to work, which I was a little surprised at, as I was
>> suspecting problems with the socket permissions.
>>
>> I had a look, and while the nginx workers in the test system are not
>> root, the nginx master process is, so maybe that allows it to work...
> I don't think that's the reason, because I remember it not working at
> first when I didn't have the permissions set.

Yep, my mistake. I didn't spot the changes to the nginx service.

>>> I renamed the default workers-log-file to php7-fpm.www.log as it is
>>> usually called. The php-fpm log-file is now called php7-fpm.log.
>>
>> What I think I meant to say here was, I'm not sure the php _version_ is
>> adding much to the log file name (rather than "I'm not sure the php is
>> adding").
>>
>> The php package version is used in a few places, and while I can
>> imagine this being consistent with other distributions, it does add a
>> bit of complexity to the default values in the service, and I'm not
>> sure what benefit it brings?
>>
> If users want to run multiple php versions, they only have to change the
> version in the php-package and pass that package along all the services.
>
> My perception of the php landscape was that the major releases aren't
> 100% reliably backwards compatible and some applications depend on older
> stable releases, so that it is not too uncommon to run multiple php
> versions the same system.
>
> Here is a quote from: https://wordpress.org/about/requirements/
>
> """
> Why do we support older versions?
>
> We strongly recommend the latest versions of PHP and MySQL, but we
> understand that this isn’t right for everyone, and that sometimes hosts
> can be slow or hesitant to upgrade their customers since upgrades to PHP
> and MySQL have historically broken applications.
> """
>
> An alternative could be to include the php-package hash in the socket
> name, but I'm not sure if that would work with nginx and it's currently
> missing reload when a system-reconfigure is done. Or services could be
> generally more isolated somehow.
>
> WDYT?

That sounds good to me. I don't know too much about this area though.

>>>>> +  (file          php-fpm-configuration-file ;#f | file-like
>>>>> +                 (default #f)))
>>
>> ...
>>
>>>>> +                       (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.
>>>>
>>> Ah, yes I'm mostly using the web documentation and other services from
>>> web.scm as reference. Thanks for the update.
>>> What would be a good value for this field? I just used "The php-fpm
>>> service-type." for now.
>>
>> That is ok, but it could probably be more useful. I think ideally it
>> would describe more about what the service offers.
>>
>> Users might encounter this when searching for services for example:
>>
>> → guix system search php
>> name: php-fpm
>> location: gnu/services/web.scm:607:2
>> extends: shepherd-root activate account
>> description: The php-fpm service-type.
>> relevance: 5
>
> I ran `guix sysytem search *` and it seems most descriptions start with
> 'Run' or 'Provide' I changed it to:
>  "Run `php-fpm' to provide a fastcgi socket for calling php through a
> webserver."
>
>
>> I've attached a patch containing a couple of copy+paste fixes, and a
>> system test.
>>
>> It would be good to get your opinion on the system test, does it test
>> the right things?
>>
> The tests look good to me.
> I added another test that adds two numbers and checks for the result in
> the response to see if php actually did something with the file.
>
>> If you like the look of it, I'd suggest including those changes in the
>> commit that adds the service, and then sending the updated patches.
>> I've included a changelog in the commit message.
>>
> Here are the updated patches. Thank you for your tests!

Thanks for picking this up again. Unfortunately it's also take me a long
time to get back around to looking at this.

I've attached some changes that I thought would be good when I was
looking through this. To give a rough summary:

 - Minor improvements to the docs, either content, markup or formatting.
 - Removing trailing whitespace.
 - Removing the changes to the nginx-service, in favour of changing the
   default socket group.
 - Change some indentation to avoid long lines.

By changing the default socket group, the system test passes, even
without the changes to the nginx service. I think this is a bit better,
and while it's definately not perfect, I think it would be ok to merge
with this change.

To also try and move the first patch forward, I've submitted that within
#29629, with an additional patch to get other services using
version-major.

It would be good to get your thoughts on the changes in the attached
patch, and then if you could send an updated set of patches, that would
be great. As far as I remember, the changes to the nginx service were
the only thing I felt needed addressing before merging this.

Thanks,

Chris

diff --git a/doc/guix.texi b/doc/guix.texi
index f2ef941b4..dcab3bd76 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15173,7 +15173,7 @@ with some additional features useful for sites of any 
size.
 These features include:
 @itemize @bullet
 @item Adaptive process spawning
address@hidden Basic statistics (ala Apache's mod_status)
address@hidden Basic statistics (similar to Apache's mod_status)
 @item Advanced process management with graceful stop/start
 @item Ability to start workers with different uid/gid/chroot/environment
 and different php.ini (replaces safe_mode)
@@ -15194,7 +15194,9 @@ A Service type for @code{php-fpm}.
 @deftp {Data Type} php-fpm-configuration
 Data Type for php-fpm service configuration.
 @table @asis
address@hidden @code {socket} (default: @code{(string-append "/var/run/php" 
(version-major (package-version php)) "-fpm.sock")})
address@hidden @code{php} (default: @code{php})
+The php package to use.
address@hidden @code{socket} (default: @code{(string-append "/var/run/php" 
(version-major (package-version php)) "-fpm.sock")})
 The address on which to accept FastCGI requests.  Valid syntaxes are:
 @table @asis
 @item @code{"ip.add.re.ss:port"}
@@ -15205,20 +15207,20 @@ Listen on a TCP socket to all addresses on a specific 
port.
 Listen on a unix socket.
 @end table
 
address@hidden @code {user} (default: @code{php-fpm})
address@hidden @code{user} (default: @code{php-fpm})
 User who will own the php worker processes.
address@hidden @code {group} (default: @code{php-fpm})
address@hidden @code{group} (default: @code{php-fpm})
 Group of the worker processes.
address@hidden @code {socket-user} (default: @code{php-fpm})
address@hidden @code{socket-user} (default: @code{php-fpm})
 User who can speak to the php-fpm socket.
address@hidden @code {socket-group} (default: @code{php-fpm})
address@hidden @code{socket-group} (default: @code{php-fpm})
 Group that can speak to the php-fpm socket.
address@hidden @code {pid-file} (default: @code{(string-append "/var/log/php" 
(version-major (package-version php)) "-fpm.pid")})
address@hidden @code{pid-file} (default: @code{(string-append "/var/run/php" 
(version-major (package-version php)) "-fpm.pid")})
 The process id of the php-fpm process is written to this file
 once the service has started.
address@hidden @code {log-file} (default: @code{(string-append "/var/log/php" 
(version-major (package-version php)) "-fpm.log")})
address@hidden @code{log-file} (default: @code{(string-append "/var/log/php" 
(version-major (package-version php)) "-fpm.log")})
 Log for the php-fpm master process.
address@hidden @code {process-manager} (default: 
@code{(php-fpm-dynamic-process-manager-configuration)})
address@hidden @code{process-manager} (default: 
@code{(php-fpm-dynamic-process-manager-configuration)})
 Detailed settings for the php-fpm process manager.
 Must be either:
 @table @asis
@@ -15226,62 +15228,66 @@ Must be either:
 @item @code{<php-fpm-static-process-manager-configuration>}
 @item @code{<php-fpm-on-demand-process-manager-configuration>}
 @end table
address@hidden @code {display-errors} (default @code{#f})
address@hidden @code{display-errors} (default @code{#f})
 Determines wether php errors and warning should be sent to clients
 and displayed in their browsers.
 This is useful for local php development, but a security risk for public sites,
 as error messages can reveal passwords and personal data.
address@hidden @code {workers-logfile} (default @code{(string-append 
"/var/log/php" (version-major (package-version php)) "-fpm.www.log")})
address@hidden @code{workers-logfile} (default @code{(string-append 
"/var/log/php" (version-major (package-version php)) "-fpm.www.log")})
 This file will log the @code{stderr} outputs of php worker processes.
 Can be set to @code{#f} to disable logging.
address@hidden @code {file} (default @code{#f})
address@hidden @code{file} (default @code{#f})
 An optional override of the whole configuration.
 You can use the @code{mixed-text-file} function or an absolute filepath for it.
 @end table
 @end deftp
 
 @deftp {Data type} php-fpm-dynamic-process-manager-configuration
-Data Type for the @code{dynamic} php-fpm process manager.
-With the @code{dynamic} process manager spare worker processes are kept around
+Data Type for the @code{dynamic} php-fpm process manager.  With the
address@hidden process manager, spare worker processes are kept around
 based on it's configured limits.
 @table @asis
address@hidden @code {max-children} (default: @code{5})
address@hidden @code{max-children} (default: @code{5})
 Maximum of worker processes.
address@hidden @code {start-servers} (default: @code{2})
address@hidden @code{start-servers} (default: @code{2})
 How many worker processes should be started on start-up.
address@hidden @code {min-spare-servers} (default: @code{1})
address@hidden @code{min-spare-servers} (default: @code{1})
 How many spare worker processes should be kept around at minimum.
address@hidden @code {max-spare-servers} (default: @code{3})
address@hidden @code{max-spare-servers} (default: @code{3})
 How many spare worker processes should be kept around at maximum.
 @end table
 @end deftp
 
 @deftp {Data type} php-fpm-static-process-manager-configuration
-Data Type for the @code{static} php-fpm process manager.
-With the @code{static} process manager an unchanging number
-of worker processes is created.
+Data Type for the @code{static} php-fpm process manager.  With the
address@hidden process manager, an unchanging number of worker processes
+are created.
 @table @asis
address@hidden @code {max-children} (default: @code{5})
address@hidden @code{max-children} (default: @code{5})
 Maximum of worker processes.
 @end table
 @end deftp
 
 @deftp {Data type} php-fpm-on-demand-process-manager-configuration
-Data Type for the @code{on-demand} php-fpm process manager.
-With the @code{on-demand} process manager worker processes are only created
-as requests arrive.
+Data Type for the @code{on-demand} php-fpm process manager.  With the
address@hidden process manager, worker processes are only created as
+requests arrive.
 @table @asis
address@hidden @code {max-children} (default: @code{5})
address@hidden @code{max-children} (default: @code{5})
 Maximum of worker processes.
address@hidden @code {process-idle-timeout} (default: @code{10})
address@hidden @code{process-idle-timeout} (default: @code{10})
 The time in seconds after which a process with no requests is killed.
 @end table
 @end deftp
 
 
address@hidden {Scheme Variable} nginx-php-fpm-location
address@hidden {Scheme Procedure} nginx-php-fpm-location @
+       [#:nginx-package nginx] @
+       [socket (string-append "/var/run/php" @
+                              (version-major (package-version php)) @
+                              "-fpm.sock")]
 A helper function to quickly add php to an @code{nginx-server-configuration}.
address@hidden defvr
address@hidden deffn
 
 A simple services setup for nginx with php can look like this:
 @example
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 3b3be215a..00599df6d 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -98,7 +98,7 @@
             php-fpm-configuration-display-errors
             php-fpm-configuration-workers-log-file
             php-fpm-configuration-file
-            
+
             <php-fpm-dynamic-process-manager-configuration>
             php-fpm-dynamic-process-manager-configuration
             make-php-fpm-dynamic-process-manager-configuration
@@ -107,13 +107,13 @@
             php-fpm-dynamic-process-manager-configuration-start-servers
             php-fpm-dynamic-process-manager-configuration-min-spare-servers
             php-fpm-dynamic-process-manager-configuration-max-spare-servers
-            
+
             <php-fpm-static-process-manager-configuration>
             php-fpm-static-process-manager-configuration
             make-php-fpm-static-process-manager-configuration
             php-fpm-static-process-manager-configuration?
             php-fpm-static-process-manager-configuration-max-children
-            
+
             <php-fpm-on-demand-process-manager-configuration>
             php-fpm-on-demand-process-manager-configuration
             make-php-fpm-on-demand-process-manager-configuration
@@ -302,12 +302,10 @@ of index files."
           "events {}\n")))
 
 (define %nginx-accounts
-  (list (user-group (name "php-fpm") (system? #t))
-        (user-group (name "nginx") (system? #t))
+  (list (user-group (name "nginx") (system? #t))
         (user-account
          (name "nginx")
          (group "nginx")
-         (supplementary-groups '("php-fpm"))
          (system? #t)
          (comment "nginx server user")
          (home-directory "/var/empty")
@@ -450,7 +448,7 @@ of index files."
   (socket-user      php-fpm-configuration-socket-user
                     (default "php-fpm"))
   (socket-group     php-fpm-configuration-socket-group
-                    (default "php-fpm"))
+                    (default "nginx"))
   (pid-file         php-fpm-configuration-pid-file
                     (default (string-append "/var/run/php"
                                             (version-major (package-version 
php))
@@ -542,13 +540,13 @@ of index files."
               "pm.start_servers =" (number->string pm.start-servers) "\n"
               "pm.min_spare_servers =" (number->string pm.min-spare-servers) 
"\n"
               "pm.max_spare_servers =" (number->string pm.max-spare-servers) 
"\n"))
-            
+
             (($ <php-fpm-static-process-manager-configuration>
                 pm.max-children)
              (list
               "pm = static\n"
               "pm.max_children =" (number->string pm.max-children) "\n"))
-            
+
             (($ <php-fpm-on-demand-process-manager-configuration>
                 pm.max-children
                 pm.process-idle-timeout)
@@ -604,16 +602,19 @@ of index files."
 
 
 (define php-fpm-service-type
-  (service-type (name 'php-fpm)
-                (description "Run `php-fpm' to provide a fastcgi socket for 
calling php through a webserver.")
-                (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))))
+  (service-type
+   (name 'php-fpm)
+   (description
+    "Run @command{php-fpm} to provide a fastcgi socket for calling php through
+a webserver.")
+   (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))))
 
 (define* (nginx-php-location
           #:key

signature.asc
Description: PGP signature