Problem with requisites when using includes

[Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk]

Starting nginx on minion and modifying nginx.conf - adding erroneous directive, so nginx -t returns error and non-zero exit code. Running a highstate:

test.sls

nginx-conf-check:
  cmd.run:
    - name: nginx -t
    - prereq:
      - service: nginx-service

nginx-service:
  service.running:
    - name: nginx
    - enable: True

include:
  - .test2

test2.sls

emulate-changes:
  cmd.run:
    - name: echo 'changed=true'
    - stateful: True
    - watch_in:
      - service: nginx-service

# salt --state-verbose=true --state-output=full 'minion' state.highstate
minion:
----------
          ID: emulate-changes
    Function: cmd.run
        Name: echo 'changed=true'
      Result: True
     Comment:
     Started: 22:52:21.334729
    Duration: 33.6 ms
     Changes:
              ----------
              changed:
                  true
              pid:
                  25644
              retcode:
                  0
              stderr:
              stdout:
                  changed=tru
----------  
          ID: nginx-conf-check
    Function: cmd.run
        Name: nginx -t
      Result: True
     Comment: No changes detected
     Started:
    Duration:
     Changes:
----------  
          ID: nginx-service
    Function: service.running
        Name: nginx
      Result: False
     Comment: Failed to restart the service
     Started: 22:52:22.297196
    Duration: 110.531 ms
     Changes:
              ----------
              nginx:
                  False

Summary for minion
------------
Succeeded: 2 (changed=2)
Failed:    1
------------
Total states run:     3
Total run time: 144.131 ms
ERROR: Minions returned with non-zero exit code

Checking process list - nginx is absent. Why did nginx-service state start ? Why nginx-conf-check state was not started ?
Trying the same with single sls file:

nginx-conf-check:
  cmd.run:
    - name: nginx -t
    - prereq:
      - service: nginx-service

nginx-service:
  service.running:
    - name: nginx
    - enable: True

emulate-changes:
  cmd.run:
    - name: echo 'changed=true'
    - stateful: True
    - watch_in:
      - service: nginx-service

Running a highstate:

# salt --state-verbose=true --state-output=full 'minion' state.highstate
minion:
----------  
          ID: emulate-changes
    Function: cmd.run
        Name: echo 'changed=true'
      Result: True
     Comment:
     Started: 23:09:10.022617
    Duration: 16.91 ms
     Changes:
              ----------
              changed:
                  true
              pid:
                  25787
              retcode:
                  0
              stderr:
              stdout:
                  changed=tru
----------
          ID: nginx-conf-check
    Function: cmd.run
        Name: nginx -t
      Result: False
     Comment: Command "nginx -t" run
     Started: 23:09:10.124155
    Duration: 21.025 ms
     Changes:   
              ----------
              pid:
                  25795
              retcode:
                  1
              stderr:
                  nginx: [emerg] unknown directive "123123123" in /etc/nginx/nginx.conf:1
                  nginx: configuration file /etc/nginx/nginx.conf test failed
              stdout:
----------  
          ID: nginx-service
    Function: service.running
        Name: nginx
      Result: None
     Comment: The service nginx is already running
     Started:
    Duration:
     Changes:
              ----------
              watch:
                  watch

Summary for minion
------------
Succeeded: 2 (unchanged=1, changed=3)
Failed:    1
------------
Total states run:     3
Total run time:  37.935 ms
ERROR: Minions returned with non-zero exit code

Now we see a proper behavior: check state was started, nginx was not restarted, it is still present in process list.
It seems something is wrong in processing includes by salt.

P.S. Why do we see

stdout:
                  changed=tru

in salt output ? Where is the last letter (truE) ?

[Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk]

salt-2016.3.3 (both master and minion)

[Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk]

Updated to latest 2016.3.4 (Boron) - result is the same, problem still exists.

[Ch3LL]

@Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk hmmm with the include I"m getting the following output:

thecakeisalie:
----------
          ID: emulate-changes
    Function: cmd.run
        Name: echo 'changed=true'
      Result: True
     Comment: 
     Started: 17:13:42.180331
    Duration: 8.41 ms
     Changes:   
              ----------
              changed:
                  true
              pid:
                  29488
              retcode:
                  0
              stderr:
              stdout:
                  changed=tru
----------
          ID: nginx-conf-check
    Function: cmd.run
        Name: nginx -t
      Result: False
     Comment: Command "nginx -t" run
     Started: 17:13:42.542456
    Duration: 16.457 ms
     Changes:   
              ----------
              pid:
                  29528
              retcode:
                  1
              stderr:
                  nginx: [emerg] unknown directive "122323" in /etc/nginx/nginx.conf:5
                  nginx: configuration file /etc/nginx/nginx.conf test failed
              stdout:
----------
          ID: nginx-service
    Function: service.running
        Name: nginx
      Result: None
     Comment: Service nginx is set to start
     Changes:   

Summary for thecakeisalie
------------
Succeeded: 2 (unchanged=1, changed=2)
Failed:    1
------------
Total states run:     3
Total run time:  24.867 ms
ERROR: Minions returned with non-zero exit code

Im suprised in your second example that it states nginx is already started if there is a directive in the nginx.conf that is incorrect. I would expect that nginx would not start.

can you include a salt --versions-report. Mostly just looking for which OS you are running on to try to replicate this behavior.

[Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk]

The main difference in our tests is that nginx must be running already before we start a highstate action. Situation: we have nginx running on several servers. After some time we decide to change its configuration and modify some pillar values. After that some states (in my example it is called emulate-changes) do the changes and report status changed=true to nginx-service state. That is the only why nginx-service state sees changes and tries to restart nginx.
My main idea is not to restart if we have erroneous changes. Because while we will inspect where the error is all the sites will be down. So I expect nginx-conf-check to be executed just before nginx restart.

In your case state nginx-service sees some work to be done (i.e. "changes") from the beginning, because nginx is stopped. In my case nginx is running so it will see changes only after emulate-changes state execution.

When all these states are in single sls file - all works as expected, but when I move edit states to separate file and include it from main nginx-conf-check ceases to run. So we have the case when some state sees changes and executes, but state which prerequires it - doesn't. I think include must not break behavior in that way.

[Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk]

I am using master and minion both updated to the latest stable version and both running on ubuntu 14.04.
Version report:

master# salt --versions-report
Salt Version:
           Salt: 2016.3.4

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.3.0
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.4

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-88-generic
         system: Linux
        version: Ubuntu 14.04 trusty

minion# salt-call --versions-report
Salt Version:
           Salt: 2016.3.4

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-88-generic
         system: Linux
        version: Ubuntu 14.04 trusty

[Z9n2JktHlZDmlhSvqc9X2MmL3BwQG7tk]

Is it something related to #38235 ?

[Ch3LL]

Yeah I believe this directly related. I'm guessing you are getting different results because the include creates a different order. Although there is a different bug here I believe with the test=True not displaying the entire True. So I will labelt his a bug for that issue

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.