Some states need concurrent when running salt-minion as a non-root user - includes possible solution
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description of Issue/Question
When salt-minion is configured to run as a non-root user with sudo privileges, some states require concurrent to be set to True, as the minion starts another process to actually execute the state. When using a sudo setup, some states are executed by issuing a sudo command to salt-call, which then detects that another state is already running. This result is then propagated up the chain, and the result of the state is that another state is already running. Unfortunate, as the actual work should have been performed by the child process.
The workaround/solution to this issue is to force concurrent to True for the states that behaves in this manner.
So far, the states state.apply, state.sls, state.highstate has been identified and a workaround has been implemented in executors/sudo.py (https://github.com/saltstack/salt/pull/42021).
I have run into a few other states that need the fix as well, and are unsure whether this is now a conclusive list of all states needing concurrent to run in a sudo setup:
state.show_highstate
state.show_sls
Setup and steps to Reproduce Issue
Setup and reproduction of errors has been fully described in https://github.com/saltstack/salt/issues/25842
Versions Report
Salt Version:
Salt: 2018.3.0
Dependency Versions:
cffi: 1.5.2
cherrypy: Not Installed
dateutil: Not Installed
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.8
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.6
mysql-python: Not Installed
pycparser: 2.10
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.13 (default, Jan 11 2017, 10:56:06) [GCC]
python-gnupg: Not Installed
PyYAML: 3.12
PyZMQ: 14.0.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.0.4
System Versions:
dist: SuSE 12 x86_64
locale: UTF-8
machine: x86_64
release: 4.4.143-94.47-default
system: Linux
version: SUSE Linux Enterprise Server 12 x86_64
I've also seen the issue on a 2018.3.2 on Debian 9.5, so not unique to the above version.
Suggested workaround/solution
Instead of making a list of states that need to have concurrent = True hardcoded in sudo.py, it might be an idea to check if the process found to be a concurrent instance is actually an ancestor of the state reporting the concurrency issue. This seem to be logically placed in modules/states.py, and seem to me to be a bit more dynamic in it's working. If a process is a direct decendant of the process being reported, then ignore that process.
The following patch seem to work in that way, but probably need someone to sanity check it before spreading it widely.
--- state.py.new 2018-11-26 14:53:26.306978300 +0100
+++ state.py.new 2018-11-26 14:56:43.554939700 +0100
@@ -21,6 +21,7 @@
import tarfile
import tempfile
import time
+import psutil
# Import salt libs
import salt.config
@@ -375,7 +376,23 @@
if concurrent:
return ret
active = __salt__['saltutil.is_running']('state.*')
+ ancestors = []
+ pid = os.getpid()
+ cur_pid = pid
+ init_found = False
+ # Build list of ancestor processors.
+ while not init_found:
+ cur_ppid = psutil.Process(cur_pid).ppid
+ ancestors.append(cur_ppid)
+ cur_pid = cur_ppid
+ if cur_pid in [0, 1]:
+ init_found = True
+
for data in active:
+ # If the pid that is concurrent is one of the ancestores,
+ # then ignore this process, as we are probably running a sudo setup.
+ if data['pid'] in ancestors:
+ continue
err = (
'The function "{0}" is running as PID {1} and was started at '
'{2} with jid {3}'
Of note, the psutil is overridden by psutil_compat, so it is my guess that it should work on most systems.
I'm unsure as to whether this module is executed on other than Unix/Linux like platforms and if it would work on those.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with executors/sudo.py and modules/states.py, then review the reproduction described in issue 25842. Check how state.apply, state.sls, state.highstate, state.show_highstate, and state.show_sls handle concurrent execution and how the proposed ancestor-process behavior works across supported platforms. Done means the affected states no longer report a false concurrent run in the sudo setup, with the complete affected list determined.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- devops, infrastructure
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 32/100