[bug#47704] [PATCH] services: mysql: Add extra-environment as configuration option.

[david larsson]

Hi Maxime!

On 2021-04-11 17:33, Maxime Devos wrote:
> Please corect the galera package to refer to the coreutils (ls, stat, 
> ...)
> by absolute file name instead, using something like
>
> (add-after 'install
>   (substitute* "INSTALL-LOCATION/wsrep_sst_rsync"
>     (("\\bls\\b") (string-append (assoc-ref inputs "coreutils") 
> "/bin/ls"))
>     ...))
>
> (Likewise for rsync, gawk, iproute ...)
>
> Don't use (which "ls") instead of string-append + assoc-ref! (which 
> "ls") is
> incorrect when cross-compiling;
>
> That way, people don't have to fiddle with PATH in their configuration 
> file.

I think you misundestood here - these rsync, gawk, iproute etc are 
executed as part of scripts in the mysqld/bin package output folder (see 
`ls -la $(dirname $(readlink -f $(which mysqld)))/` ) because the Galera 
add-on was (at least partially) merged into the regular mariadb sources, 
so the scripts where the binaries are needed are not from the galera 
package outputs but the mysql one. The galera package in guix master 
today only provides the #$galera "/lib/libgalera_smm.so file. I agree it 
would be nice to patch all the shell scripts in the $#mysql/bin folder 
but this would 1. require maintaining all the additional patching of 
those scripts as part of the mysql package and 2. since custom 
wsrep_sst_<X> shell scripts are possible and are usually added to 
/usr/bin/wsrep_sst_<X> those binaries will also require setting 
additional environment variables or the same type of patching (which 
would therefore "require" this patch to make it easier).

> If possible, consider writing a "system test" automatically testing
> some very basic functionality of mariadb + galera 
> (gnu/tests/databases.scm).

I would have liked to, but Im sorry - I do not have the time/skill for 
this for the time being. I hope the current patch is simple enough that 
an additional test is not currently needed.

> > I ran these commands to test:
> > guix pull --url=/home/user1/src/guix --profile=/tmp/guix.master
> > --disable-authentication --allow-downgrades ;
> > GUIX_PROFILE="/tmp/guix.master" ; . "$GUIX_PROFILE/etc/profile" ; guix
> > system reconfigure config.scm --fallback --allow-downgrades
> >
> > ------------------------------------------------------------------
> >
> > (extra-environment #~(list ...
> >
> > "USER=mysql"
> > "SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt"
> > "SSL_CERT_DIR=/run/current-system/profile/etc/ssl/certs"))
>
> It seems extra-environment is still useful.  "USER=mysql" should 
> probably be
> added automatically, though (see my proposal below).

Yes, in particular for custom wsrep scripts. The "USER=mysql" may not be 
needed per se for the example snippet config I sent earlier as I did not 
make sure that the list of environment variables there were the minimal 
amount needed; I mainly hoped to provide a repeatable example of 
something to test with.

> Perhaps you could extend "mysql-configuration" with a "galera" field
> (with #f as default)?  Theoretical example:
>
> (mysql-configuration
>   (port A-DIFFERENT-PORT)
>   ;; [...] other fields
>   (galera
>     (package my-version-of-galera) ; optional
>     (cluster-name "test_cluster")
>     (cluster-address "gcom://...")
>     (synchronization-method 'rsync)
>     (node-adress "redacted")
>     (node-name "librem13v3guixsd")))
>
> .. and modify mysql-service-type to insert appropriate configuration 
> entries
> and perhaps add things to the PATH of the shepherd service as 
> appropriate.
>
> Escape hatches like "extra-content" are useful, but this seems a bit
> neater.

This looks nice! I agree that something like what you show here is the 
better option, at the same time I don't have the time/skill to provide a 
patch with a well-featured interface to the galera options for the time 
being. The "escape hatch" would therefore be very useful for now, and 
less hassle to maintain - compare with for example the vpn service and 
the amount of emails in the lists regarding lack of options that could 
have easily been added via some g-expression strings. In general I don't 
see the harm in providing both "the escape hatch" way to add options to 
a configuration file and the guile interface which is otherwise nicer 
(IMO).

> > ------------------------------------------------------------------
> >
> > Please someone also review [bug#47517] [PATCH] gnu: nginx: Enable 
> > stream
> > module
>
> I'll take a look at it.

Thanks!

Best regards,
David