Open zzaimeche opened 2 years ago
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. Please be sure to review our Code of Conduct. Also, check out some of our community resources including:
There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!
Description When the user applies a state to set a keyboard keymap that is not available, the command fails, but salt reports a success. I think I have found the cause and am working on a patch.
Setup I think this would affect everyone using this module, but this is where I encountered it:
Steps to Reproduce the behavior
Run:
salt-call --local state.single keyboard.system ca.map.gz
Expected behavior Result should be False, Succeeded should be 0, Failed should be 1.
Screenshots
Versions Report
salt --versions-report
```yaml Salt Version: Salt: 3004 Dependency Versions: cffi: 1.13.2 cherrypy: Not Installed dateutil: Not Installed docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 2.10.1 libgit2: Not Installed M2Crypto: 0.38.0 Mako: Not Installed msgpack: 0.5.6 msgpack-pure: Not Installed mysql-python: Not Installed pycparser: 2.17 pycrypto: Not Installed pycryptodome: Not Installed pygit2: Not Installed Python: 3.6.15 (default, Sep 23 2021, 15:41:43) [GCC] python-gnupg: Not Installed PyYAML: 5.4.1 PyZMQ: 17.1.2 smmap: Not Installed timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.2.3 System Versions: dist: sles 15.4 locale: UTF-8 machine: x86_64 release: 5.14.21-150400.22-default system: Linux version: SLES 15.4```Additional context I think I've worked out why this happens and am working on a fix, over here: https://github.com/saltstack/salt/compare/master...zzaimeche:salt:fix-keyboard-fails I think Salt does not check whether the shell command was successful, but whether the python calling the command ran successfully. So whether the shell command succeeds or fails, Salt reports a success. The issue is in https://github.com/saltstack/salt/blob/master/salt/modules/keyboard.py and https://github.com/saltstack/salt/blob/master/salt/states/keyboard.py . I think Salt needs to check for the retcode, so I used cmd.retcode based on other examples in the codebase, though I couldn't find the documentation for cmd.retcode, so maybe there's a better way. My WIP patch seems to fix the issue on my machine (but my WIP patch will break things on other distros at the moment). I'll make a PR shortly. I'm hoping someone from the Salt team can take a look and advise if there's a preferred approach, since I suspect there might have been similar issues with other modules and there may be a convention for this kind of fix.