MDEV-19210: use environment file in systemd units for _WSREP_* #1143

eworm-de · 2019-01-27T23:33:40Z

We used to run systemctl set-environment to pass
_WSREP_START_POSITION. This is bad because:

it clutter systemd's environment (yes, pid 1)
it requires root privileges

Let's just create an environment file in ExecStartPre=, that is read
before ExecStart= kicks in. We have _WSREP_START_POSITION around for the
main process without any downsides.

eworm-de · 2019-01-27T23:35:24Z

I have filled out the contribution agreement and sent it.

grooverdan

I think this is a great improvement in deducing the need for PermissionStartOnly=true.

Final question for clarity, is what is the cleaning policy on support-files/tmpfiles.conf.in that contain d @MYSQL_UNIX_DIR@?

grooverdan · 2019-01-29T07:08:39Z

support-files/mariadb.service.in

@@ -84,7 +83,7 @@ ExecStart=@sbindir@/mysqld $MYSQLD_OPTS $_WSREP_NEW_CLUSTER $_WSREP_START_POSITI
 @SYSTEMD_EXECSTARTPOST@

 # Unset _WSREP_START_POSITION environment variable.
-ExecStartPost=/bin/sh -c "systemctl unset-environment _WSREP_START_POSITION"
+ExecStartPost=@bindir@/rm @MYSQL_UNIX_DIR@/wsrep-start-position


@bindir@ is referring to the mariadb location, so I think an absolute /bin/rm is safe here, especially as systemd is linux only.

I was not certain about what path to choose. Can we be sure /bin/rm will not break?
(For Arch it is fine.)

Just a few tests:

# docker run ubuntu:18.04 ls -la /bin/rm /usr/bin/rm ls: cannot access '/usr/bin/rm': No such file or directory -rwxr-xr-x. 1 root root 63704 Jan 18 2018 /bin/rm # docker run ubuntu:16.04 ls -la /bin/rm /usr/bin/rm ls: cannot access '/usr/bin/rm': No such file or directory -rwxr-xr-x. 1 root root 60272 Mar 2 2017 /bin/rm # docker run ubuntu:14.04 ls -la /bin/rm /usr/bin/rm ls: cannot access /usr/bin/rm: No such file or directory -rwxr-xr-x. 1 root root 60160 Mar 10 2016 /bin/rm # docker run alpine ls -la /bin/rm /usr/bin/rm ls: /usr/bin/rm: No such file or directory lrwxrwxrwx 1 root root 12 Jan 9 2018 /bin/rm -> /bin/busybox # docker run debian:stable ls -la /bin/rm /usr/bin/rm ls: cannot access '/usr/bin/rm': No such file or directory -rwxr-xr-x. 1 root root 64424 Feb 22 2017 /bin/rm # docker run debian:oldstable ls -la /bin/rm /usr/bin/rm ls: cannot access /usr/bin/rm: No such file or directory -rwxr-xr-x. 1 root root 60072 Mar 14 2015 /bin/rm # docker run centos:6 ls -la /bin/rm /usr/bin/rm ls: cannot access /usr/bin/rm: No such file or directory -rwxr-xr-x. 1 root root 53592 Mar 22 2017 /bin/rm # docker run centos:7 ls -la /bin/rm /usr/bin/rm -rwxr-xr-x. 1 root root 62864 Nov 5 2016 /bin/rm -rwxr-xr-x. 1 root root 62864 Nov 5 2016 /usr/bin/rm # docker run fedora:29 ls -la /bin/rm /usr/bin/rm -rwxr-xr-x. 1 root root 86432 Nov 7 15:14 /bin/rm -rwxr-xr-x. 1 root root 86432 Nov 7 15:14 /usr/bin/rm

support-files/mariadb@.service.in

support-files/mariadb.service.in

eworm-de · 2019-01-29T09:37:21Z

I think this is a great improvement in deducing the need for PermissionStartOnly=true.

Final question for clarity, is what is the cleaning policy on support-files/tmpfiles.conf.in that contain d @MYSQL_UNIX_DIR@?

A non-capital d just creates the directory: man page: tmpfiles.d

eworm-de · 2019-01-29T09:58:23Z

Just added another commit to update galera_new_cluster.

eworm-de · 2019-02-12T15:43:46Z

Hey @fauust, looks like you are doing some systemd work. Is this something for you?

fauust · 2019-02-12T16:22:51Z

@eworm-de, I do Debian packaging and debug for MariaDB foundation and systemd too, so yes I will look at this too.

grooverdan · 2019-03-13T02:16:47Z

Note this accidentally(?) reverts and removes the a+x permissions on scripts/galera_new_cluster.sh within git so maybe the added chmod isn't needed.

galera_new_cluster needs a multi-instance argument ($2?) so it can write to @mysqlunixdir@wsrep-new-cluster%I with the right contents and start the right instance.

eworm-de · 2019-03-13T08:28:54Z

Note this accidentally(?) reverts and removes the a+x permissions on scripts/galera_new_cluster.sh within git so maybe the added chmod isn't needed.

We have the literal @mysqlunixdir@ in the script, making it a template. To make sure nobody copies or executes it as is I removed the executable bit.

galera_new_cluster needs a multi-instance argument ($2?) so it can write to @mysqlunixdir@wsrep-new-cluster%I with the right contents and start the right instance.

Right, that's not yet implemented. However #510 removes wsrep support from multi-instance unit. Any decision on whether or not that goes in?

grooverdan · 2019-03-13T12:01:07Z

No idea what excuse is being applied for not merging #510. In MDEV-18863 Geoff is advocating there are users of galera systemd multiinstance so hence me re-looking at this one to re-add galera support while keeping PermissionsStartOnly=true out of it. While it won't be an simple merge, it also shouldn't be too bad.

eworm-de · 2019-03-13T12:15:33Z

About to update the script for multi-instance support then.

eworm-de · 2019-03-13T13:38:25Z

Updated... I hope this covers all cases now.

grooverdan · 2019-03-13T23:03:11Z

scripts/galera_new_cluster.sh

@@ -21,11 +21,11 @@ EOF
    exit 0
 fi

-systemctl set-environment _WSREP_NEW_CLUSTER='--wsrep-new-cluster' && \
+echo _WSREP_NEW_CLUSTER='--wsrep-new-cluster' > @mysqlunixdir@/"wsrep-new-cluster-${1:-mariadb}" && \


- before $ breaks it for mariadb@service

Maybe something like:

SERVICE=${1:-mariadb@service} FILE=@mysqlunixdir@/wsrep-new-cluster-${SERVICE} echo _WSREP_NEW_CLUSTER='--wsrep-new-cluster' > "${FILE}" ... .. systemctl start "$SERVICE" rm -f "$FILE"

And then have both mariadb.service.in and mariadb@service.in the same with:

EnvironmentFile=-@mysqlunixdir@/wsrep-new-cluster-%n EnvironmentFile=-@mysqlunixdir@/wsrep-start-position-%n

Let's have a look at what my code does. ${1:-mariadb} evaluates to:

mariadb when argument is omitted -> the main unit

mariadb for mariadb -> the main unit

mariadb@foo for mariadb@foo - > instantiated unit

Within the unit files I use %N, which is the full unit name with type suffix removed.

I still think this works as-is. Or did I get anything wrong?

Ah, missed default expansion and a more specific indicator that the filename was incorrect. So mariadb.service.in needs @mysqlunixdir@/wsrep-start-position-mariadb rather than @mysqlunixdir@/wsrep-start-position.

No, that's not true. The file @mysqlunixdir@/wsrep-start-position is generated in main unit in StartExecPre= without -mariadb suffix.

(In contrast to file @mysqlunixdir@/wsrep-new-cluster-%N, which has the suffix as it is generated in script galera_new_cluster.)

mariadb.service:

@mysqlunixdir@/wsrep-start-position

@mysqlunixdir@/wsrep-new-cluster-%N

mariadb@.service:

@mysqlunixdir@/wsrep-start-position-%N

@mysqlunixdir@/wsrep-new-cluster-%N

oh, so sorry. Going to sleep now.

CLAassistant · 2019-07-24T20:38:13Z

All committers have signed the CLA.

grooverdan · 2019-11-06T23:08:30Z

🐌

eworm-de · 2019-11-07T07:26:21Z

No idea why the CLA stuff popped up lately... Just (re-)signed.
Any remaining issues?

eworm-de · 2019-11-07T07:32:35Z

Just rebased...

LinuxJedi · 2023-07-26T13:14:32Z

Hi @eworm-de,

I'm trying to get older pull requests to some kind of resolution. As a result, I want to get this one into a place where it can be reviewed properly. I've tried rebasing it against 10.4, and I am having problems with mariadb@.service.in as it has diverged so much. I'm not enough of a systemd expert to know what the end result would look like.

Could you please rebase against 10.4 for us? We can review it for you as a matter of priority.

eworm-de · 2023-07-26T14:09:35Z

Rebased for current 10.3...

eworm-de · 2023-07-26T14:10:49Z

Support for Galera in multi-instance service has been dropped in 91f1694. So we can just drop that part.

eworm-de · 2023-07-26T14:16:29Z

Rebased for current 10.4...
I did not test, but should work.

We used to run `systemctl set-environment` to pass _WSREP_START_POSITION. This is bad because: * it clutter systemd's environment (yes, pid 1) * it requires root privileges * options (like LimitNOFILE=) are not applied Let's just create an environment file in ExecStartPre=, that is read before ExecStart= kicks in. We have _WSREP_START_POSITION around for the main process without any downsides.

Now that the systemd unit files use an environment file to pass _WSREP_START_POSITION we have to update galera_new_cluster as well.

LinuxJedi

Nice work, many thanks!

LinuxJedi · 2023-08-08T14:48:07Z

@eworm-de unfortunately I had to revert this because it was breaking the build on some of the Debian builders. For example: https://buildbot.mariadb.net/buildbot/builders/kvm-deb-buster-aarch64/builds/3742

eworm-de · 2023-08-08T23:59:01Z

Looks like this is the message from command that fails:

mysqld[2545]: /usr/bin/install: cannot change owner and permissions of ‘/var/run/mysqld’: No such file or directory

But there is nothing in mariadb.service that calls /usr/bin/install. So where is this from? Does Debian modify the service?

Please note that @MYSQL_UNIX_DIR@ (I think that represents /var/run/mysqld here) is created by systemd-tmpfiles. The configuration for that is shipped in support-files/tmpfiles.conf.in.

eworm-de · 2023-08-09T00:05:46Z

Yes, added here...

server/cmake/systemd.cmake

Lines 48 to 49 in 161ce04

    
           IF(DEB) 
        
             SET(SYSTEMD_EXECSTARTPRE "ExecStartPre=/usr/bin/install -m 755 -o mysql -g root -d /var/run/mysqld")

Broken just for Debian. 😜

eworm-de · 2023-08-09T00:16:23Z

I guess for Debian we should not remove this:

# Execute pre and post scripts as root, otherwise it does it as User=
PermissionsStartOnly=true

Is there an easy way to verify?

eworm-de · 2023-08-09T00:32:32Z

I think something like #2724 should resolve this properly.

eworm-de · 2023-08-09T00:34:18Z

How to proceed here? Do I have to open a new pull request?

eworm-de · 2023-08-10T06:58:23Z

Just opened #2726... Let's continue there.

eworm-de mentioned this pull request Jan 27, 2019

systemd 240 fails to start mariadb + galera on Arch Linux systemd/systemd#11510

Closed

an3l added the license-mca Contributed under the MCA label Jan 28, 2019

an3l added this to the 10.3 milestone Jan 28, 2019

an3l assigned cvicentiu Jan 28, 2019

eworm-de force-pushed the systemd-environment branch from 4dabf8c to 4ec2c15 Compare January 28, 2019 10:51

grooverdan reviewed Jan 29, 2019

View reviewed changes

eworm-de force-pushed the systemd-environment branch from 4ec2c15 to 3fbae48 Compare January 29, 2019 09:10

eworm-de force-pushed the systemd-environment branch from 3fbae48 to fa34e83 Compare January 29, 2019 18:55

eworm-de changed the title ~~use environment file in systemd units for _WSREP_START_POSITION~~ use environment file in systemd units for _WSREP_* Jan 29, 2019

eworm-de force-pushed the systemd-environment branch from fa34e83 to 9995053 Compare January 29, 2019 23:22

grooverdan mentioned this pull request Feb 8, 2019

MDEV-11670: systemd multi-instance #510

Closed

eworm-de mentioned this pull request Feb 26, 2019

MDEV-17028 Fix glitches with systemd sysusers and tmpfiles #530

Closed

eworm-de force-pushed the systemd-environment branch from 9995053 to 745502d Compare March 13, 2019 08:32

eworm-de force-pushed the systemd-environment branch from 745502d to d77e660 Compare March 13, 2019 12:43

grooverdan reviewed Mar 13, 2019

View reviewed changes

an3l changed the title use environment file in systemd units for _WSREP_* MDEV-19210: use environment file in systemd units for _WSREP_* Apr 7, 2019

eworm-de force-pushed the systemd-environment branch from d77e660 to b939738 Compare July 24, 2019 20:38

eworm-de force-pushed the systemd-environment branch from b939738 to ed5c5c3 Compare November 7, 2019 07:32

eworm-de force-pushed the systemd-environment branch from ed5c5c3 to 61ad108 Compare January 16, 2020 12:43

eworm-de force-pushed the systemd-environment branch from 61ad108 to 456fb66 Compare February 17, 2020 08:54

ottok mentioned this pull request Apr 10, 2020

Deb: Add support for legacy init systems #1463

Merged

eworm-de force-pushed the systemd-environment branch from 456fb66 to bb09521 Compare July 26, 2023 14:05

eworm-de force-pushed the systemd-environment branch from bb09521 to a3bf388 Compare July 26, 2023 14:15

eworm-de changed the base branch from 10.3 to 10.4 July 26, 2023 14:20

eworm-de added 2 commits August 2, 2023 15:39

update galera_new_cluster to use environment file

fd256d2

Now that the systemd unit files use an environment file to pass _WSREP_START_POSITION we have to update galera_new_cluster as well.

LinuxJedi force-pushed the systemd-environment branch from a3bf388 to fd256d2 Compare August 2, 2023 14:39

LinuxJedi approved these changes Aug 2, 2023

View reviewed changes

LinuxJedi merged commit b54e4bf into MariaDB:10.4 Aug 2, 2023

LinuxJedi mentioned this pull request Aug 8, 2023

Revert MDEV-19210 #2722

Merged

eworm-de mentioned this pull request Aug 9, 2023

use special executable prefix for Debian #2724

Closed

4 tasks

eworm-de deleted the systemd-environment branch November 22, 2023 19:56

MDEV-19210: use environment file in systemd units for _WSREP_* #1143

MDEV-19210: use environment file in systemd units for _WSREP_* #1143

Conversation

eworm-de commented Jan 27, 2019

eworm-de commented Jan 27, 2019

grooverdan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eworm-de commented Jan 29, 2019

eworm-de commented Jan 29, 2019

eworm-de commented Feb 12, 2019

fauust commented Feb 12, 2019

grooverdan commented Mar 13, 2019

eworm-de commented Mar 13, 2019

grooverdan commented Mar 13, 2019

eworm-de commented Mar 13, 2019

eworm-de commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Jul 24, 2019 • edited Loading

grooverdan commented Nov 6, 2019

eworm-de commented Nov 7, 2019

eworm-de commented Nov 7, 2019

LinuxJedi commented Jul 26, 2023

eworm-de commented Jul 26, 2023

eworm-de commented Jul 26, 2023

eworm-de commented Jul 26, 2023

LinuxJedi left a comment

Choose a reason for hiding this comment

LinuxJedi commented Aug 8, 2023

eworm-de commented Aug 8, 2023 • edited Loading

eworm-de commented Aug 9, 2023 • edited Loading

eworm-de commented Aug 9, 2023

eworm-de commented Aug 9, 2023

eworm-de commented Aug 9, 2023

eworm-de commented Aug 10, 2023

CLAassistant commented Jul 24, 2019 •

edited

Loading

eworm-de commented Aug 8, 2023 •

edited

Loading

eworm-de commented Aug 9, 2023 •

edited

Loading