search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.16k stars 5.48k forks source link

Invalid systemd service unit on RHEL7/CentOS7 #31034

Closed eliasp closed 8 years ago

eliasp commented 8 years ago

The system service unit for salt-minion 2015.8.5 from repo.saltstack.com (salt-master might be affected in the same way) contains an invalid directive:

[Unit]
Description=The Salt Minion
After=network.target

[Service]
EnvironmentFile=/etc/default/salt-minion
Type=simple
LimitNOFILE=8192
ExecStart=/usr/bin/salt-minion
KillMode=process
Restart=$RESTART

[Install]
WantedBy=multi-user.target

The problem is Restart=$RESTART, where $RESTART is most likely the result of a problem in the build process of the package. The value should be one of:

See also the systemd.service documentation.

Version information:

    Salt Version:
               Salt: 2015.8.5

    Dependency Versions:
             Jinja2: 2.7.2
           M2Crypto: 0.21.1
               Mako: Not Installed
             PyYAML: 3.11
              PyZMQ: 14.7.0
             Python: 2.7.5 (default, Nov 20 2015, 02:00:19)
               RAET: Not Installed
            Tornado: 4.2.1
                ZMQ: 4.0.5
               cffi: Not Installed
           cherrypy: Not Installed
           dateutil: 1.5
              gitdb: Not Installed
          gitpython: Not Installed
              ioflo: Not Installed
            libgit2: Not Installed
            libnacl: Not Installed
       msgpack-pure: Not Installed
     msgpack-python: 0.4.6
       mysql-python: Not Installed
          pycparser: Not Installed
           pycrypto: 2.6.1
             pygit2: Not Installed
       python-gnupg: Not Installed
              smmap: Not Installed
            timelib: Not Installed

    System Versions:
               dist: centos 7.2.1511 Core
            machine: x86_64
            release: 3.10.0-123.8.1.el7.x86_64
             system: CentOS Linux 7.2.1511 Core
gtmanfred commented 8 years ago

$RESTART should be in the environment file /etc/default/salt-minion but I don't see that file anywhere in the spec file.

jfindlay commented 8 years ago

@eliasp, thanks for reporting. You are welcome to submit issues (and pull requests) about packaging against https://github.com/saltstack/salt-pack.

vutny commented 8 years ago

The /etc/default/salt-minion was installed on my system:

$ cat /etc/redhat-release 
CentOS Linux release 7.2.1511 (Core)
$ cat /etc/default/salt-minion
# Controls whether or not service is restarted automatically when it exits.
# See the manpage for systemd.service(5) for possible values for the "Restart="
# option.
RESTART=no
$ salt-minion --version
salt-minion 2015.8.7 (Beryllium)

From systemd.service(5) man page:

     Restart=
           Takes one of no, on-success, on-failure, on-abnormal, on-watchdog, on-abort, or always. If set to no (the default), the service will not be restarted.

Basic environment variable substitution is supported in systemd version 219 from CentOS 7.2. Everything looks good. Also, I saw /etc/default/salt-minion is present in earlier RPMs, 2015.8.5 for example.

I guess some problems with those configs may possibly appear during automated upgrades of salt-minion and systemd done by Salt itself, like mentioned here: #30928. But that's a known limitation.

@eliasp Have you seen any warning messages from systemd about Restart=$RESTART line? Try sudo systemctl status salt-minion. Thanks in advance for any feedback!

eliasp commented 8 years ago

Still seeing this for salt-minion.service from 2015.8.7 on CentOS 7.2 where systemctl status -l salt-minion reports:

● salt-minion.service - The Salt Minion
   Loaded: loaded (/usr/lib/systemd/system/salt-minion.service; disabled; vendor preset: disabled)
   Active: inactive (dead)

Mar 10 17:57:59 centos7.2-base-65fa766833bad70d systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Mar 10 17:58:09 centos7.2-base-65fa766833bad70d systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Mar 10 17:58:16 centos7.2-base-65fa766833bad70d systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART

/etc/default/salt-minion which is specified as EnvironmentFile= in the service unit exists and contains RESTART=no, but as systemd.service(5) states, variable substitution is not supported for Restart=:

This section describes command line parsing and variable and specifier substitutions for ExecStart=, ExecStartPre=, ExecStartPost=, ExecReload=, ExecStop=, and ExecStopPost= options.

vutny commented 8 years ago

@eliasp Hm... This is very strange. You're right, the manual doesn't describe explicit possibitily to use variable substitutions in restart specifier. But I'm running a lot of CentOS 7 systems and everywhere salt-minion unit works just fine.

There is small chance, that somehow during upgrade/installation systemd hasn't seen or was unable to read the /etc/default/salt-minion file. Have you tried sudo systemctl daemon-reload? Maybe that could help.

eliasp commented 8 years ago

@vutny I made sure to execute systemctl daemon-reload - so systemd definitely "knew" about the content of the EnvironmentFile=.

It's not like salt-minion doesn't work - it starts & works just fine - it's just a warning in the service's journal.

vutny commented 8 years ago

@eliasp I saw above someone filled the issue with exactly the same problem, but for me it looks like something related to the SystemD, and has nothing to do with salt-minion unit.

This is one of my CentOS7 machines:

$ sudo systemctl restart salt-minion
$ sudo systemctl status salt-minion
* salt-minion.service - The Salt Minion
   Loaded: loaded (/usr/lib/systemd/system/salt-minion.service; enabled; vendor preset: disabled)
   Active: active (running) since Tue 2016-03-15 09:10:46 UTC; 6s ago
 Main PID: 14337 (salt-minion)
   CGroup: /system.slice/salt-minion.service
           |-14337 /bin/python2 /usr/bin/salt-minion
           `-14342 /bin/python2 /usr/bin/salt-minion

Mar 15 09:10:46 centos7 systemd[1]: Started The Salt Minion.
Mar 15 09:10:46 centos7 systemd[1]: Starting The Salt Minion...
$ cat /usr/lib/systemd/system/salt-minion.service
[Unit]
Description=The Salt Minion
After=network.target

[Service]
EnvironmentFile=/etc/default/salt-minion
Type=simple
LimitNOFILE=8192
ExecStart=/usr/bin/salt-minion
KillMode=process
Restart=$RESTART

[Install]
WantedBy=multi-user.target

By the way, do you use SELinux?

The bad thing about this issue: where is no clear way to reproduce it, at least for me.

maxhq commented 8 years ago

Maybe it has to do with the systemd version used?

vutny commented 8 years ago

I have this version running without any issues:

$ rpm -qi systemd | head
Name        : systemd
Version     : 219
Release     : 19.el7_2.4
Architecture: x86_64
Install Date: Fri 04 Mar 2016 06:12:19 AM UTC
Group       : Unspecified
Size        : 22314482
License     : LGPLv2+ and MIT and GPLv2+
Signature   : RSA/SHA256, Tue 16 Feb 2016 10:17:33 PM UTC, Key ID 24c6a8a7f4a80eb5
Source RPM  : systemd-219-19.el7_2.4.src.rpm
vutny commented 8 years ago

@eliasp Have you tried to reproduce your issue with a fresh clean CentOS 7 system on some other machine? For example, use some of publicly available VM appliances (vagrant box?) or cloud images. Still, consider to check your SELinux settings and policies. Maybe some audit logs would testify a root cause.

eliasp commented 8 years ago

@vutny I can reproduce this issue:

vutny commented 8 years ago

@eliasp I'm still not familiar with systemd-nspawn yet... So let's investigate the EC2 case. Are you using public AWS Marketplace AMI from CentOS team or some custom/third-party? Could you share your ami-id please if that's possible?

vutny commented 8 years ago

By the way, systemd was updated to the 219-19.el7_2.7 version in the downstream repos. It's worth to test against it.

eliasp commented 8 years ago

Just tested on a fresh CentOS7 image, updated to 219-19.el7_2.7 and installed salt-minion: it's still the same:

# systemctl status salt-minion
● salt-minion.service - The Salt Minion
   Loaded: loaded (/usr/lib/systemd/system/salt-minion.service; enabled; vendor preset: disabled)
   Active: inactive (dead)

Apr 15 11:52:26 centos7.2-base-39b2203f24166306 systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Apr 15 11:52:27 centos7.2-base-39b2203f24166306 systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Apr 15 11:52:27 centos7.2-base-39b2203f24166306 systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Apr 15 11:52:27 centos7.2-base-39b2203f24166306 systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Apr 15 11:52:27 centos7.2-base-39b2203f24166306 systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
Apr 15 11:56:52 centos7.2-base-39b2203f24166306 systemd[1]: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART

As already outlined in my previous comment I'm still convinced using variable substition is simply not supported in Restart=.

maxhq commented 8 years ago

I agree with @eliasp: Looking at the systemd sources shows that for Restart only a simple string lookup is done in a table. Full reference:

Also the manpage explicitely mentions the parameters where variable substitution is accepted and Restart is not amongst them.

djsly commented 8 years ago

Same problem here

# salt-minion --version
salt-minion 2015.8.8.2 (Beryllium)
# cat /etc/*release
CentOS Linux release 7.1.1503 (Core) 
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/"
BUG_REPORT_URL="https://bugs.centos.org/"

CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"

CentOS Linux release 7.1.1503 (Core) 
CentOS Linux release 7.1.1503 (Core) 
systemd: [/usr/lib/systemd/system/salt-minion.service:11] Failed to parse service restart specifier, ignoring: $RESTART
# yum info salt-minion
Loaded plugins: fastestmirror
Loading mirror speeds from cached hostfile
 * base: less.cogeco.net
 * epel: mirror.csclub.uwaterloo.ca
 * extras: less.cogeco.net
 * updates: mirror.csclub.uwaterloo.ca
Installed Packages
Name        : salt-minion
Arch        : noarch
Version     : 2015.8.8
Release     : 2.el7
Size        : 59 k
Repo        : installed
From repo   : saltstack-pkgrepo
Summary     : Client component for Salt, a parallel remote execution system
URL         : http://saltstack.org/
License     : ASL 2.0
Description : The Salt minion is the agent component of Salt. It listens for instructions
            : from the master, runs jobs, and returns results back to the master.
sastorsl commented 8 years ago

This also applies to Red Hat Enterprise Linux Server 7.2 with salt 2015.8.8.2

Salt Version:
           Salt: 2015.8.8.2

Dependency Versions:
         Jinja2: 2.7.2
       M2Crypto: 0.21.1
           Mako: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.7.0
         Python: 2.7.5 (default, Oct 11 2015, 17:47:16)
           RAET: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
        libgit2: Not Installed
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.7
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
   python-gnupg: Not Installed
          smmap: Not Installed
        timelib: Not Installed

System Versions:
           dist: redhat 7.2 Maipo
        machine: x86_64
        release: 3.10.0-327.13.1.el7.x86_64
         system: Red Hat Enterprise Linux Server 7.2 Maipo
maxhq commented 8 years ago

This is classified as a severe bug and open for 3.5 months now. I really think the $RESTART variable should be replaced by the desired default no and /etc/default/salt-minion should be removed as this is obviously not working according to the systemd specification.

dmurphy18 commented 8 years ago

@eliasp @maxhq @sastorsl @djsly @vutny Systemd support has been revamped in Salt 2015.8.12 and 2016.3.3 and the service units have been rewritten and no longer make use of the environment variable $RESTART, and type=simple has been changed to type=notify appropriately. Upgrading salt from earlier versions to the latest branch versions specified above, the minion's now automatically restart.

I would like to close this issue if the recent updates resolve your issues.

ahammond commented 8 years ago

@dmurphy18 any links to the PRs or commits? I took a look on an Ubuntu 16.04 machine with salt-minion 2016.3.3 on it and see:

root@somebox:/lib/systemd/system# cat salt-minion.service
[Unit]
Description=The Salt Minion
After=network.target

[Service]
Type=notify
LimitNOFILE=8192
ExecStart=/usr/bin/salt-minion

[Install]
WantedBy=multi-user.target

Looks fine. It'd be nice to see Restart support and even watchdog, but... crawl before walking and all that.

dmurphy18 commented 8 years ago

@ahammond need to look in salt-pack that is where things get built. https://github.com/saltstack/salt-pack/pull/138. @terminalmage did the changes in salt for the unit files that are being picked up from salt-pack, for example: commit 6cb0fb47f35078aaa47960ca6af3add2030fbbbd

terminalmage commented 8 years ago

@ahammond Unfortunately, Restart= is not tunable via a variable defined in an EnvironmentFile, these only appear to be supported for the execution environment (ExecStart, etc.). This is why it was removed as something defined in the environment file.

If you want to change the Restart= value or setup watchdog support, you would need to copy the unit file to /etc/systemd/system, make the modifications, and then do a systemctl daemon-reload.

sastorsl commented 8 years ago

Create a salt-minion.service.d directory and add custom settings there.

Stein Arne Storslett

On 15 Sep 2016 21:42, "Erik Johnson" notifications@github.com wrote:

@ahammond https://github.com/ahammond Unfortunately, Restart= is not tunable via a variable defined in an EnvironmentFile, these only appear to be supported for the execution environment (ExecStart, etc.). This is why it was removed as something defined in the environment file.

If you want to change the Restart= value or setup watchdog support, you would need to copy the unit file to /etc/systemd/system, make the modifications, and then do a systemctl daemon-reload.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/saltstack/salt/issues/31034#issuecomment-247430734, or mute the thread https://github.com/notifications/unsubscribe-auth/ALqxs0Rq68N99n_NFtEfAWwU8vJQlLLXks5qqZ-egaJpZM4HWOjP .

eliasp commented 8 years ago

Create a salt-minion.service.d directory and add custom settings there.

systemctl edit salt-minion will do that for you and also spawn an editor to edit the created salt-minion.d/override.conf right away.

dmurphy18 commented 8 years ago

@eliasp This issue appears to be resolved, if you are satisfied with the outcome, can you close it ?, or let me know of further issues to be addressed.

eliasp commented 8 years ago

Looks good! Thanks for the fix. Closing…