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

onlyif` has different result for single condition and a list of this condition (only for undefined?) #15351

Closed
marbx opened this issue Aug 28, 2014 · 5 comments
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt P2 Priority 2 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@marbx
Copy link
Contributor

marbx commented Aug 28, 2014

onlyif accecpts a single condition or a list of conditions. (At least the compiler accepts both)

In the first fragment onlyif has a list of one condition.
In the second fragment onlyif has a single condition.
The condition is undefined.

DESIRED BEHAVIOUR: the result of the two fragements is identical.
ACTUAL BEHAVIOUR: Saltstack decides that the list is always true, the single condition always false.

Daniel Jagszent suggested that the first fragment uses "wrong syntax"
https://groups.google.com/d/msg/salt-users/7DlOwfQaLcA/WfIWcFpJNSkJ

Should this be actually the case then:
DESIRED BEHAVIOUR: Saltstack rejects "wrong syntax"
ACTUAL BEHAVIOUR: Saltstack ignores "wrong syntax"

Disclaimer: I have only tested this for an undefined test, and for Windows. Someone should use a decidable test.

always_executed:
  cmd:
    - run
    - cwd: /
    - name: notepad
    - onlyif:
      - test -f /plan/salt/YesTrigger.txt

never_executed:
  cmd:
    - run
    - cwd: /plan/salt/
    - name: notepad YES.txt
    - onlyif: test -f /plan/salt/YesTrigger.txt
@basepi
Copy link
Contributor

basepi commented Aug 29, 2014

We will definitely investigate this. Thanks for the report.

@basepi basepi added this to the Approved milestone Aug 29, 2014
@SaltDBray SaltDBray added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Core relates to code central or existential to Salt P2 Priority 2 and removed severity-low 4th level, cosemtic problems, work around exists labels Jun 30, 2015
@oliver-dungey
Copy link

@markuskramerIgitt I've also noticed that it's hard to debug onlyif lists as the logging doesn't tell you which condition failed (or I'm a numpty and don't know what I'm doing, quite possible).

@oliver-dungey
Copy link

I've just tested the logic again to make sure I know whats going on and it's all looking ok on 2016.3.0

test1:
  cmd.run:
  - name: echo 'Test 1 - Should pass onlif test'
  - onlyif: test -d /tmp

test2:
  cmd.run:
  - name: echo 'Test 2 - Should pass onlyif test'
  - onlyif:
    - test -d /tmp

test3:
  cmd.run:
  - name: echo 'Test 3 - Should pass onlyif test'
  - onlyif:
    - test -d /tmp
    - test -d /usr

test4:
  cmd.run:
  - name: echo 'Test 4 - Should fail onlyif test'
  - onlyif:
    - test -d /tmp
    - test -d /bogusdirectory

test5:
  cmd.run:
  - name: echo 'Test 5 - Should fail onlyif test'
  - onlyif:
    - test -d /bogusdirectory
    - test -d /tmp

And here are the results (that all look good i.e. no bug):

local:
----------
          ID: test1
    Function: cmd.run
        Name: echo 'Test 1 - Should pass onlif test'
      Result: True
     Comment: Command "echo 'Test 1 - Should pass onlif test'" run
     Started: 13:30:02.924822
    Duration: 10.505 ms
     Changes:
              ----------
              pid:
                  10586
              retcode:
                  0
              stderr:
              stdout:
                  Test 1 - Should pass onlif test
----------
          ID: test2
    Function: cmd.run
        Name: echo 'Test 2 - Should pass onlyif test'
      Result: True
     Comment: Command "echo 'Test 2 - Should pass onlyif test'" run
     Started: 13:30:02.935586
    Duration: 8.155 ms
     Changes:
              ----------
              pid:
                  10588
              retcode:
                  0
              stderr:
              stdout:
                  Test 2 - Should pass onlyif test
----------
          ID: test3
    Function: cmd.run
        Name: echo 'Test 3 - Should pass onlyif test'
      Result: True
     Comment: Command "echo 'Test 3 - Should pass onlyif test'" run
     Started: 13:30:02.943966
    Duration: 13.3 ms
     Changes:
              ----------
              pid:
                  10591
              retcode:
                  0
              stderr:
              stdout:
                  Test 3 - Should pass onlyif test
----------
          ID: test4
    Function: cmd.run
        Name: echo 'Test 4 - Should fail onlyif test'
      Result: True
     Comment: onlyif execution failed
     Started: 13:30:02.957531
    Duration: 10.82 ms
     Changes:
----------
          ID: test5
    Function: cmd.run
        Name: echo 'Test 5 - Should fail onlyif test'
      Result: True
     Comment: onlyif execution failed
     Started: 13:30:02.968808
    Duration: 4.76 ms
     Changes:

Summary for local
------------
Succeeded: 5 (changed=3)
Failed:    0
------------

@oliver-dungey
Copy link

I suspect this issue should be closed as it is looking fixed to me.

@basepi
Copy link
Contributor

basepi commented Apr 25, 2016

Thanks! I'll go ahead and close this. If it's still an issue for you @markuskramerIgitt, please keep us posted!

@basepi basepi closed this as completed Apr 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt P2 Priority 2 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

4 participants