Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onchanges acts like onchanges_any #56133

Open
jtraub91 opened this issue Feb 11, 2020 · 11 comments
Open

onchanges acts like onchanges_any #56133

jtraub91 opened this issue Feb 11, 2020 · 11 comments
Milestone

Comments

@jtraub91
Copy link
Contributor

Description of Issue

onchanges acts like onchanges_any should

Setup

Normal salt installation. 1 master, 1 minion

Steps to Reproduce Issue

Create orch file

install_jq:
  pkg.installed:
    - name: jq

install_tmux:
  pkg.installed:
    - name: tmux
    - require:
      - install_jq

install_htop:
  pkg.installed:
    - name: htop
    - require:
      - install_tmux

execute_runner:
  salt.runner:
    - name: config.get
    - key: file_roots
    - onchanges:
      - install_jq
      - install_tmux
      - install_htop

Yet execute_runner is executed with only install_jq having changes

root@ubuntu-test-1:/srv/salt# salt-run state.orch orch.install
ubuntu-test-1_master:
----------
          ID: install_jq
    Function: pkg.installed
        Name: jq
      Result: True
     Comment: The following packages were installed/updated: jq
     Started: 22:20:30.609458
    Duration: 10469.665 ms
     Changes:
              ----------
              jq:
                  ----------
                  new:
                      1.5+dfsg-2
                  old:
              libjq1:
                  ----------
                  new:
                      1.5+dfsg-2
                  old:
              libonig4:
                  ----------
                  new:
                      6.7.0-1
                  old:
----------
          ID: install_tmux
    Function: pkg.installed
        Name: tmux
      Result: True
     Comment: All specified packages are already installed
     Started: 22:20:41.090867
    Duration: 1164.657 ms
     Changes:
----------
          ID: install_htop
    Function: pkg.installed
        Name: htop
      Result: True
     Comment: All specified packages are already installed
     Started: 22:20:42.256054
    Duration: 9.206 ms
     Changes:
----------
          ID: execute_runner
    Function: salt.runner
        Name: config.get
      Result: True
     Comment: Runner function 'config.get' executed.
     Started: 22:20:42.268726
    Duration: 399.839 ms
     Changes:
              ----------
              return:
                  ----------
                  base:
                      - /srv/salt
                      - /srv/spm/salt

Summary for ubuntu-test-1_master
------------
Succeeded: 4 (changed=2)
Failed:    0
------------
Total states run:     4
Total run time:  12.043 s

Versions Report

root@ubuntu-test-1:/srv/salt# salt --versions-report
Salt Version:
           Salt: 3000

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
         Jinja2: 2.10
        libgit2: Not Installed
       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: 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
          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-66-generic
         system: Linux
        version: Ubuntu 18.04 bionic

@DmitryKuzmenko
Copy link
Contributor

@jtraub91 excuse me I'm no quite understand whats wrong in your case. Both onchanges and onchanges_any will be triggered if at least one of the watched states made a change. The difference between these requisites is that onchanges will be triggered if the "result" is True for all the watched states when onchanges_any -- if at least one of the watched state "result" is True.

@DmitryKuzmenko DmitryKuzmenko added the info-needed waiting for more info label Feb 12, 2020
@DmitryKuzmenko DmitryKuzmenko added this to the Blocked milestone Feb 12, 2020
@OrangeDog
Copy link
Contributor

The confusion is it's natural to assume the any bit refers to onchanges. Quite probably the documentation can be clearer on the multiple outputs states have (success and changed) and how they affect requisites.

@alan-cugler
Copy link
Contributor

alan-cugler commented Feb 13, 2020

if that's the case, could someone leave a strong and clear definition for these requisites?

I will have the @saltstack/docs-working-group make adjustments to the relevant docs page based on those definitions after some saltstack/community endorsement that the definitions are correct.

@waynew
Copy link
Contributor

waynew commented Feb 19, 2020

If a state has multiple onchanges requisites then the state will trigger if any of the watched states changes.

That's directly from https://docs.saltstack.com/en/latest/ref/states/requisites.html

Since 2014.

onchanges_any is from 2018. I'm not sure exactly why there are two requisites that behave exactly the same, it would require some research with git blame & history to track down what they motivation was. But both of these things behave exactly as documented.

@OrangeDog
Copy link
Contributor

They’re not exactly the same, because a state can return True without having changes.

@stale
Copy link

stale bot commented Mar 20, 2020

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.

@stale stale bot added the stale label Mar 20, 2020
@stale
Copy link

stale bot commented Mar 20, 2020

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.

@stale stale bot closed this as completed Mar 27, 2020
@sagetherage sagetherage removed info-needed waiting for more info stale labels Apr 7, 2020
@sagetherage sagetherage reopened this Apr 7, 2020
@sagetherage sagetherage removed this from the Blocked milestone Apr 7, 2020
@sagetherage
Copy link
Contributor

@waynew this staled out and needs followup and likely re-triaged, please.

@sagetherage
Copy link
Contributor

Right! this is a big enough change that it needs a SEP

@OrangeDog
Copy link
Contributor

@sagetherage what change? There is no request or problem here as far as I can see, except perhaps unclear wording in the documentation.

@garethgreenaway
Copy link
Contributor

When onchanges_any was added it was done along with a handful of other _any requisites, and with the assumption that onchanges required ALL state functions to return True before it was triggered, when in fact it triggers when ANY of the state functions returned True. So now we have two requisites doing the same thing. I've added the following SEP (saltstack/salt-enhancement-proposals#27) proposing that onchanges is changed to trigger when ALL state functions returned True and onchanges_any is left as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants