[BUG] Race-condition on SaltMinion leads to job being reported as succeeded when in reality it failed
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description
SaltMinion receives 2 jobs from the master. The first one includes a state.apply and the other one can be any job. When the second job executes, it will reload all the modules and recreate the context as well. The failed result of the state.apply is set to the old context so when checking for the actual result we will get a false positive because the context has been reset. The important thing to mention is that multiprocessing is set to False in the minion config. Setting it to True seems to fix this issue because the jobs are running in different processes. However, I guess this should work even if multiprocessing is set to False (clients without enough RAM cannot enable multiprocessing).
Setup
- on-prem machine
- VM (VMWare)
- used bootstrap to install
Steps to Reproduce the behavior
Use this simple function as a module(I've added it in the test.py salt module)
def test_fail():
time.sleep(6)
raise CommandExecutionError(
'Problem encountered'
)
Then add a simple test.sls under /srv/salt:
test.test_fail:
module.run
Then just send a job from the master to the minion:
salt osboxes state.apply,test.ping test, --out json
The result will be a success, validated with salt-run state.event as well. If we change the time.sleep(6) to time.sleep(3) it will be reported as failed. Why? Because 6 is greater than 5 which I guess is the default timeout for the master running a saltutil.find_job job.
So we have a state.apply that takes 6 seconds to fail. And a find_job that runs and completes before the state.apply to finish. I've attached some logs that could help. The place to look for the issue is line 14 vs line 35, the state.apply will report the failure but in the old context. This can also be seen by checking the value for ContextObjectId between the 2 use cases.
SaltRaceConditionLogs.txt
Expected behavior
Since running the same job when no other jobs are running reports a failure, I expect to return the same thing even if other jobs are running in the same time. Think about the first one being a state.apply that installs software (could take some time) and the other one being just an internal job that gets the current state or the saltutil.find_job that is running on this setup. This is causing false-positive results.
Screenshots
If applicable, add screenshots to help explain your problem.
Versions Report
Salt Version:
Salt: 3005.1
Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 2.7.3
docker-py: Not Installed
gitdb: 2.0.6
gitpython: 3.0.7
Jinja2: 2.10.1
libgit2: Not Installed
M2Crypto: Not Installed
Mako: 1.1.0
msgpack: 0.6.2
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: Not Installed
pycryptodome: 3.6.1
pygit2: Not Installed
Python: 3.8.10 (default, Jun 22 2022, 20:18:18)
python-gnupg: 0.4.5
PyYAML: 5.3.1
PyZMQ: 20.0.0
smmap: 2.0.5
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.2
System Versions:
dist: ubuntu 20.04 focal
locale: utf-8
machine: x86_64
release: 5.15.0-53-generic
system: Linux
version: Ubuntu 20.04 focal
I've reproduced this initially on a Windows10 client having salt 3000.2 installed and then tried with the latest version.
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 by reproducing the race with the test_fail function in the test.py Salt module, the test.sls state, and the provided salt command using multiprocessing=False. Compare the state.apply and saltutil.find_job paths, including ContextObjectId and SaltRaceConditionLogs.txt. Done means a concurrent internal job no longer causes a long-running state.apply failure to be reported as success.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- distributed-systems, infrastructure
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100