saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.2k stars 5.48k forks source link

[BUG] Unless requisite returns wrong result on cmd with non-existing cwd #67055

Open cebe opened 13 hours ago

cebe commented 13 hours ago

Description

The unless check does not fail when executed in a non-existing directory.

I'd expect either an error or the correct result of the unless check. In the following example I get Comment: unless condition is true.

Setup

Classic Salt Master-Minion Setup.

Steps to Reproduce the behavior

State SLS file:

/tmp/bugreport:
  file.directory

/tmp/doesnotexist:
  file.absent

command_create_file:
  cmd.run:
    - name: test -s /tmp/bugreport/validation.key || echo 'randomkey' > /tmp/bugreport/validation.key
    - cwd: /tmp/doesnotexist
    #- cwd: /tmp
    - unless: test -f /tmp/bugreport/validation.key

Output:

# salt 'papierdev.layer5.net' state.apply services.layer5-ng.bug
papierdev.layer5.net:
----------
          ID: /tmp/bugreport
    Function: file.directory
      Result: True
     Comment: 
     Started: 08:53:20.688123
    Duration: 9.919 ms
     Changes:   
              ----------
              /tmp/bugreport:
                  ----------
                  directory:
                      new
----------
          ID: /tmp/doesnotexist
    Function: file.absent
      Result: True
     Comment: File /tmp/doesnotexist is not present
     Started: 08:53:20.698183
    Duration: 0.468 ms
     Changes:   
----------
          ID: command_create_file
    Function: cmd.run
        Name: test -s /tmp/bugreport/validation.key || echo 'randomkey' > /tmp/bugreport/validation.key
      Result: True
     Comment: unless condition is true
     Started: 08:53:20.699459
    Duration: 1336.724 ms
     Changes:   

Summary for papierdev.layer5.net
------------
Succeeded: 3 (changed=1)
Failed:    0
------------
Total states run:     3
Total run time:   1.347 s

Expected behavior

After changing the cwd to /tmp it works as expected:

# salt 'papierdev.layer5.net' state.apply services.layer5-ng.bug
papierdev.layer5.net:
----------
          ID: /tmp/bugreport
    Function: file.directory
      Result: True
     Comment: The directory /tmp/bugreport is in the correct state
     Started: 09:10:52.320037
    Duration: 8.631 ms
     Changes:   
----------
          ID: /tmp/doesnotexist
    Function: file.absent
      Result: True
     Comment: File /tmp/doesnotexist is not present
     Started: 09:10:52.328795
    Duration: 0.461 ms
     Changes:   
----------
          ID: command_create_file
    Function: cmd.run
        Name: test -s /tmp/bugreport/validation.key || echo 'randomkey' > /tmp/bugreport/validation.key
      Result: True
     Comment: Command "test -s /tmp/bugreport/validation.key || echo 'randomkey' > /tmp/bugreport/validation.key" run
     Started: 09:10:52.329977
    Duration: 1254.758 ms
     Changes:   
              ----------
              pid:
                  3270072
              retcode:
                  0
              stderr:
              stdout:

Summary for papierdev.layer5.net
------------
Succeeded: 3 (changed=1)
Failed:    0
------------
Total states run:     3
Total run time:   1.264 s

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Master: Salt Version: Salt: 3007.1 Python Version: Python: 3.10.14 (main, Apr 3 2024, 21:30:09) [GCC 11.2.0] Dependency Versions: cffi: 1.16.0 cherrypy: unknown dateutil: 2.8.2 docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 3.1.4 libgit2: Not Installed looseversion: 1.3.0 M2Crypto: Not Installed Mako: Not Installed msgpack: 1.0.7 msgpack-pure: Not Installed mysql-python: Not Installed packaging: 23.1 pycparser: 2.21 pycrypto: Not Installed pycryptodome: 3.19.1 pygit2: Not Installed python-gnupg: 0.5.2 PyYAML: 6.0.1 PyZMQ: 25.1.2 relenv: 0.16.0 smmap: Not Installed timelib: 0.3.0 Tornado: 6.3.3 ZMQ: 4.3.4 Salt Package Information: Package Type: onedir System Versions: dist: debian 12.8 bookworm locale: utf-8 machine: x86_64 release: 6.1.0-26-amd64 system: Linux version: Debian GNU/Linux 12.8 bookworm ``` Salt Minion: ```yaml # salt-minion --versions-report Salt Version: Salt: 3006.8 Python Version: Python: 3.10.14 (main, Apr 3 2024, 21:33:04) [GCC 11.2.0] Dependency Versions: cffi: 1.14.6 cherrypy: 18.6.1 dateutil: 2.8.1 docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 3.1.3 libgit2: Not Installed looseversion: 1.0.2 M2Crypto: Not Installed Mako: Not Installed msgpack: 1.0.2 msgpack-pure: Not Installed mysql-python: Not Installed packaging: 22.0 pycparser: 2.21 pycrypto: Not Installed pycryptodome: 3.19.1 pygit2: Not Installed python-gnupg: 0.4.8 PyYAML: 6.0.1 PyZMQ: 23.2.0 relenv: 0.16.0 smmap: Not Installed timelib: 0.2.4 Tornado: 4.5.3 ZMQ: 4.3.4 System Versions: dist: debian 12 bookworm locale: utf-8 machine: aarch64 release: 6.1.0-21-arm64 system: Linux version: Debian GNU/Linux 12 bookworm ```
cebe commented 12 hours ago

While trying to figure out how a fix could be implemented I found that there is a test for a similar case, where the user to run the command as does not exist. The expected result in the test is to return True. Why is that?

https://github.com/saltstack/salt/blob/9233e1cc3b6b072a61b445d285ba856fc642ef3b/tests/pytests/unit/state/test_state_compiler.py#L190-L210

tjyang commented 6 hours ago

Thanks for this detail bug report, as a salt user, I confirm this issue also on 3006.9 environment. cwd to /tmp make this state file works.