Service nginx not reloaded when changes in configuration

[daks]

Hello,

we are using the nginx.ng part of this formula on Debian 9 with Salt 2018.3.1. We use a wrapper that include the following states

include:
  - nginx.ng.pkg
  - nginx.ng.config
  - nginx.ng.servers_config
  - nginx.ng.servers
  - nginx.ng.certificates
  - nginx.ng.service

I recently added nginx.ng.servers which seems to handle service reload when configuration change (and pillar service.enable is not set to False).

I think it does not work as expected because I don't see any reload of service when I change config, and running salt returns

          ID: nginx_service_reload
    Function: service.running
        Name: nginx
      Result: True
     Comment: The service nginx is already running
     Started: 10:04:19.463741
    Duration: 40.412 ms
     Changes:   

This part of the code https://github.com/saltstack-formulas/nginx-formula/blob/master/nginx/ng/servers.sls#L23 uses reload: True in state service.running but it seems that this parameter does not exist https://docs.saltstack.com/en/latest/ref/states/all/salt.states.service.html#salt.states.service.running on 2018.3 or 2017.7.

[daks]

To broaden the discussion, I'm a bit surprised to see a specific state (servers) to handle service reload: we use this formula since a while and never imagined it would not automatically reload the service.

What I had expected is:

• a pillar service_reload set to True by default (because is what you want when you change config) which can be set to False if needed
• an auto-reload of the service when the state servers_config is included

[kiniou]

Hello @daks, I'm not experiencing this behaviour since when I'm doing changes in nginx.ng.servers pillar, the nginx service is correctly reloaded at the end of a state.apply with the listen requisite of nginx_service_reload.
The reload: True does not exists directly in the service.running state function but is passed as a parameter to mod_watch triggered by the listen requisite (salt doc states mod_wait in salt doc but it is mod_watch in the code).

[noelmcloughlin]

@waynew Is this same issue you describe at #191

[waynew]

@noelmcloughlin Yeah, looks like it - the problem is that when you have a state like this:

start something:
    service.running:
        - name: someservice
        - enable: True

And you've manually started your service, but didn't enable it, like this:

systemctl start nginx
systemctl enable nginx   # <-- don't run this line

Salt enables the service, which says, "Hey this state changed!" mod_watch sees, "Oh, that state has changes, no need to fire off any kind of action"

The correct thing to do in a manual setup is to have two different states, like this:

start service:
  service.running:
    - name: someservice
    - watch:
      - file: some config state

enable service:
  service.enabled:
    - name: someservice

some config state:
  file.managed:
    - name: /path/to/file
    - source: salt://path/to/file

(or, you know, quit fiddling with things manually and manage them via salt )

[noelmcloughlin]

Guys would this patch work for you? It gets triggered by watch too.

+{%- if nginx.restart_cmd %}
+  cmd.run:
+    - name: {{ nginx.restart_cmd }}
+{%- else %}
   service.running:
     - enable: True
     - restart: True
+{%- endif %}
     - watch:
 {% if use_upstart %}
       - file: nginx