saltstack / saltstack/salt

[BUG] Redis external_job_cache store jobs with wrong data

Open
#58,612 7 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug pending-discussion
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
My setup using redis returner on master. When I try a simple command it stored the job data but they are not accurate.
The Arguments, Target, Target-Type and User fields are all wrong.
It is good with the default job cache (local_cache)

Setup

# Setup external job cache on master
master_job_cache: redis
redis.cluster_mode: False
redis.host: 'redis'
redis.port: 6379
redis.db: '0'

Steps to Reproduce the behavior
Run command:

salt minion cmd.run "echo Test argument"

Get job data:

salt-run jobs.print_job 20201002035730861408

# Return
20201002035730861408:
    ----------
    Arguments:
        |_
          ----------
          __kwarg__:
              True
          cmd:
              echo Testing args
    Function:
        cmd.run
    Minions:
        - SID-45ecf38b-11b1-5264-2448-b8813e672b9d-99502
    Result:
        ----------
        SID-45ecf38b-11b1-5264-2448-b8813e672b9d-99502:
            ----------
            retcode:
                0
            return:
                Testing args
            success:
                True
    StartTime:
        2020, Oct 02 03:57:30.861408
    Target:
        Unknow-target
    Target-type:
        list
    User:
        root

I also tried to send the command as another user but only the default job cache stored it properly

Expected behavior
It should be the same as default job cache

20201002035730861408:
    ----------
    Arguments:
        |_
          ----------
          __kwarg__:
              True
          cmd:
              echo Testing args
    Function:
        cmd.run
    Minions:
        - minion
    Result:
        ----------
        minion:
            ----------
            retcode:
                0
            return:
                Testing args
            success:
                True
    StartTime:
        2020, Oct 02 03:57:30.861408
    Target:
        minion
    Target-type:
        glob
    User:
        root

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) 
Salt Version:
           Salt: 3000.3
 
Dependency Versions:
           cffi: 1.14.0
       cherrypy: unknown
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.20
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.7.3 (default, Dec 20 2019, 18:57:59)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.1.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1
 
System Versions:
           dist: debian 10.4 
         locale: utf-8
        machine: x86_64
        release: 4.15.0-96-generic
         system: Linux
        version: debian 10.4

Additional info
It could be the same for all external returner.
As I check it might related to #43454 and #48734

https://github.com/saltstack/salt/blob/062739a0b633da2dad14181426456a11d02e4053/salt/utils/job.py#L130-L132
If we use an external job cache, save_load is called here. The data of the jobs are overwritten in this code block.

With the following edit, we can see how it is overwritten.

# redis_return.py
...
def save_load(jid, load, minions=None):
    '''
    Save the load to the specified jid
    '''
+   log.info('Save load for {}:\n{}'.format(jid,load))
+   import inspect
+   log.info(inspect.getouterfranes(inspect.currentframe(), 2)[1])
    serv = _get_serv(ret=None)
    serv.setex('load:{0}'.format(jid), _get_ttl(), salt.utils.json.dumps(load))

# salt/utils/job.py
...
+   log.info('Load in store_job {} - {}'.format(job_cache, load))
    if job_cache != "local_cache":
        try:
            mminion.returners[savefstr](load["jid"], load)
logs 
[INFO    ] Save load for 20201002085528297819:
{'cmd': 'publish', 'tgt': 'minion', 'fun': 'cmd.run', 'arg': [{'__kwarg__': True, 'cmd': 'echo Testing args'}], 'tgt_type': 'glob', 'ret': '', 'jid': '20201002085528297819', 'kwargs': {'show_timeout': True, 'show_jid': True, 'delimiter': ':'}, 'user': 'root', 'to': 20}
[INFO    ] inspect FrameInfo(frame=<frame at 0x236f778, file '/usr/lib/python3/dist-packages/salt/master.py', line 2321, code _prep_pub>, filename='/usr/lib/python3/dist-packages/salt/master.py', lineno=2321, function='_prep_pub', code_context=["            fstr = '{0}.save_load'.format(self.opts['master_job_cache'])\n", "            self.mminion.returners[fstr](clear_load['jid'], clear_load, minions)\n"], index=1)
[INFO    ] User root Published command cmd.run with jid 20201002085528297819
[INFO    ] Save load for 20201002085528297819:
{'cmd': 'publish', 'tgt': 'minion', 'fun': 'cmd.run', 'arg': [{'__kwarg__': True, 'cmd': 'echo Testing args'}], 'tgt_type': 'glob', 'ret': '', 'jid': '20201002085528297819', 'kwargs': {'master_id': 'salt-master', 'user': 'root'}, 'user': 'root', 'Minions': ['minion']}
[INFO    ] inspect FrameInfo(frame=<frame at 0x2427178, file '/usr/lib/python3/dist-packages/salt/master.py', line 1692, code _syndic_return>, filename='/usr/lib/python3/dist-packages/salt/master.py', lineno=1692, function='_syndic_return', code_context=["                fstr = '{0}.save_load'.format(self.opts['master_job_cache'])\n", "                self.mminion.returners[fstr](load['jid'], load['load'])\n"], index=1)
[INFO    ] Got return from minion for job 20201002085528297819
[INFO    ] Load in stored_job: redis - {'jid': '20201002085528297819', 'id': 'minion', 'return': 'Testing args', 'retcode': 0, 'success': True, 'master_id': 'salt-master', 'fun': 'cmd.run', 'fun_args': None, '_stamp': '2020-10-02T08:55:28.745403'}
[INFO    ] Save load for 20201002085528297819:
{'jid': '20201002085528297819', 'id': 'minion', 'return': 'Testing args', 'retcode': 0, 'success': True, 'master_id': 'salt-master', 'fun': 'cmd.run', 'fun_args': None, '_stamp': '2020-10-02T08:55:28.745403'}
[INFO    ] FrameInfo(frame=<frame at 0x22e1c08, file '/usr/lib/python3/dist-packages/salt/utils/job.py', line 109, code store_job>, filename='/usr/lib/python3/dist-packages/salt/utils/job.py', lineno=109, function='store_job', code_context=['        try:\n', "            mminion.returners[savefstr](load['jid'], load)\n"], index=1)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with salt/utils/job.py around store_job and the redis_return.py save_load implementation referenced in the issue. Trace both the master publish path and the later stored job path, then verify that Redis external job-cache output preserves the same Arguments, Target, Target-type, and User values as local_cache for the provided command.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, redis
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.