[BUG] service.running doesn't trigger systemctl daemon-reload as expected

[Xaelias]

Description
Changing a .service file (ubuntu 18.04) does not trigger a daemon-reload.

Setup

install_<service>
  service.dead:
    - name: <service>
    - unless: [...]
  cmd.run:
    - name: [...]
    - onchanges:
      - service: install_<service>

<service>_service:
  file.managed:
    - name: /usr/lib/systemd/system/<service>.service
    - source: salt://<service>/files/<service>.service
    - template: jinja
    - user: root
    - group: root

<service>_conf:
  file.managed:
    - name: /etc/<service>/<service>.conf
    - source: salt://<service>/files/<service>.conf
    - makedirs: true
    - template: jinja
    - user: root
    - group: root

<service>_running:
  service.running:
    - name: <service>
    - enable: true
    - watch:
      - cmd: install_<service>
      - file: <service>_conf
      - file: <service>_service

Expected behavior
I would fully expect a systemctl daemon-reload to be run before restarting the service here.

Logs

debug logs

[INFO    ] Completed state [/usr/lib/systemd/system/<service>.service] at time 17:47:04.630468 (duration_in_ms=35.572)
[INFO    ] Running state [/etc/<service>/<service>.conf] at time 17:47:04.630658
[INFO    ] Executing state file.managed for [/etc/<service>/<service>.conf]
[DEBUG   ] In saltenv '<service>/initial_deploy', looking at rel_path '<service>_conf/files/<service>.conf' to resolve 'salt://<service>_conf/files/<service>.conf'
[DEBUG   ] In saltenv '<service>/initial_deploy', ** considering ** path '/var/cache/salt/minion/files/<service>/initial_deploy/<service>_conf/files/<service>.conf' to resolve 'salt://<service>_conf/files/<service>.conf'
[DEBUG   ] Jinja search path: [u'/var/cache/salt/minion/files/<service>/initial_deploy']
[DEBUG   ] In saltenv '<service>/initial_deploy', looking at rel_path '<service>_conf/<service>.map.jinja' to resolve 'salt://<service>_conf/<service>.map.jinja'
[DEBUG   ] In saltenv '<service>/initial_deploy', ** considering ** path '/var/cache/salt/minion/files/<service>/initial_deploy/<service>_conf/<service>.map.jinja' to resolve 'salt://<service>_conf/<service>.map.jinja'
[DEBUG   ] Creating directory: /etc/<service>
[INFO    ] File changed:
New file
[INFO    ] Completed state [/etc/<service>/<service>.conf] at time 17:47:04.652912 (duration_in_ms=22.253)
[INFO    ] Running state [<service>] at time 17:47:04.653617
[INFO    ] Executing state service.running for [<service>]
[INFO    ] Executing command [u'systemctl', 'is-active', u'<service>.service'] in directory '/home/ubuntu'
[DEBUG   ] stdout: inactive
[DEBUG   ] retcode: 3
[INFO    ] Executing command [u'systemctl', 'is-enabled', u'<service>.service'] in directory '/home/ubuntu'
[DEBUG   ] stdout: enabled
[DEBUG   ] key: systemd.scope, ret: _|-
[INFO    ] Executing command [u'systemd-run', u'--scope', u'systemctl', 'start', u'<service>.service'] in directory '/home/ubuntu'
[DEBUG   ] stderr: Running scope as unit: run-r7a78e744923540dd96e3a3fc486764b2.scope
Warning: The unit file, source configuration file or drop-ins of <service>.service changed on disk. Run 'systemctl daemon-reload' to reload units.
[INFO    ] Executing command [u'systemctl', 'is-active', u'<service>.service'] in directory '/home/ubuntu'
[DEBUG   ] stdout: active
[INFO    ] Executing command [u'systemctl', 'is-enabled', u'<service>.service'] in directory '/home/ubuntu'
[DEBUG   ] stdout: enabled
[INFO    ] Executing command [u'systemctl', 'is-enabled', u'<service>.service'] in directory '/home/ubuntu'
[DEBUG   ] stdout: enabled
[INFO    ] {u'<service>': True}
[INFO    ] Completed state [<service>] at time 17:47:04.727173 (duration_in_ms=73.556)

Versions Report

salt --versions-report

Salt Version:
           Salt: 3000.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: unknown
       dateutil: 2.8.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
         Jinja2: 2.10
        libgit2: 0.26.0
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 2.7.17 (default, Apr 15 2020, 17:20:14)
   python-gnupg: 0.4.1
         PyYAML: 5.3.1
          PyZMQ: 16.0.2
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-1039-aws
         system: Linux
        version: Ubuntu 18.04 bionic

salt-call --versions-report

Salt Version:
           Salt: 2019.2.3

Dependency Versions:
           cffi: 1.14.0
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.20
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.17 (default, Nov  7 2019, 10:07:09)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-1029-aws
         system: Linux
        version: Ubuntu 18.04 bionic

Additional context
It looks like if I specify .service in the service.running stanza it does the right thing.
But I still think this is a bug, especially since all the examples I could find using service.running only say something like nginx, not nginx.service.

If I specify:

<service>_running:
  service.running:
    - name: <service>.service

Then I get the following:

[INFO    ] Executing command [u'systemctl', 'status', u'<service>.service', u'-n', u'0'] in directory '/home/ubuntu'
[DEBUG   ] stdout: Warning: The unit file, source configuration file or drop-ins of <service>.service changed on disk. Run 'systemctl daemon-reload' to reload units.
● <service>.service - <service>
   Loaded: loaded (/usr/lib/systemd/system/<service>.service; enabled; vendor preset: enabled)
   [...]
[DEBUG   ] retcode: 3
[INFO    ] Executing command [u'systemctl', '--system', 'daemon-reload'] in directory '/home/ubuntu'
[INFO    ] Executing command [u'systemctl', 'status', u'<service>.service', u'-n', u'0'] in directory '/home/ubuntu'
[DEBUG   ] stdout: ● <service>.service - <service>
   Loaded: loaded (/usr/lib/systemd/system/<service>.service; enabled; vendor preset: enabled)
   [...]

and things seem to be working fine. I still believe that the behavior should be identical whether or not .service is specified.

Thanks!
Alexis

[xeacott]

Thanks for the report @Xaelias sometimes issues like these stem from interpretations of the docs and sometimes a lot gets lost. I'm going to mark this as a bug, but also with a Doc label so that whoever takes on this ticket knows that the additional context has a lot to say.

Additional context
It looks like if I specify .service in the service.running stanza it does the right thing.
But I still think this is a bug, especially since all the examples I could find using service.running only say something like nginx, not nginx.service.

[sagetherage]

The Core team will not get to this in Aluminium release cycle, putting it back into Approved to plan for this in another release.