Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis returner stacktrace in clean_old_jobs 2016.3.0 #33969

Closed
Inveracity opened this issue Jun 13, 2016 · 9 comments
Closed

Redis returner stacktrace in clean_old_jobs 2016.3.0 #33969

Inveracity opened this issue Jun 13, 2016 · 9 comments
Labels
Bug broken, incorrect, or confusing behavior P4 Priority 4 Returners RIoT Relates to integration with cloud providers, hypervisors, API-based services, etc. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@Inveracity
Copy link
Contributor

Inveracity commented Jun 13, 2016

description

[ERROR   ] An un-handled exception from the multiprocessing process 'Maintenance-11' was caught:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/utils/process.py", line 613, in _run
    return self._original_run()
  File "/usr/lib/python2.7/dist-packages/salt/master.py", line 236, in run
    salt.daemons.masterapi.clean_old_jobs(self.opts)
  File "/usr/lib/python2.7/dist-packages/salt/daemons/masterapi.py", line 187, in clean_old_jobs
    mminion.returners[fstr]()
  File "/usr/lib/python2.7/dist-packages/salt/returners/redis_return.py", line 215, in clean_old_jobs
    serv.delete(**to_remove)  # pylint: disable=E1134
TypeError: delete() argument after ** must be a mapping, not list

salt master versions report

Salt Version:
           Salt: 2016.3.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.3.0
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.6 (default, Jun 22 2015, 17:58:13)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.4

System Versions:
           dist: Ubuntu 14.04 trusty
        machine: x86_64
        release: 3.13.0-85-generic
         system: Linux
        version: Ubuntu 14.04 trusty
@Inveracity Inveracity changed the title Redis returner raises various exceptions 2016.3.0 Redis returner stacktrace in clean_old_jobs 2016.3.0 Jun 13, 2016
@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 13, 2016

@Inveracity thanks for the report. Just glanced at the code and it looks like its calling this function here in the client.py in redis. And then that calls this function. I'm guessing list to_remove just needs to be a dictionary and not a list since its a kwargs.

ping @jizhilong would you mind taking a look at this. Looks like you made some changes here for the redis returner with PR #30571

@Ch3LL Ch3LL added Bug broken, incorrect, or confusing behavior P4 Priority 4 Returners RIoT Relates to integration with cloud providers, hypervisors, API-based services, etc. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around labels Jun 13, 2016
@Ch3LL Ch3LL added this to the Approved milestone Jun 13, 2016
@jizhilong
Copy link
Contributor

@Ch3LL it's true I introduced this bug in with PR #30571, I'll fix this soon.
@Inveracity thanks for the report, I also found this issue in our deployment.And as each HSET 'ret:*' was saved with a TTL, this bug should not lead to a huge mount of junk data in normal case, so you can relax for a while if you are worried about that.

@Inveracity
Copy link
Contributor Author

Thank you! I wasn't too worried, but I am taking a "see something, say something" approach to all things saltstack ❤️

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 14, 2016

@jizhilong thanks for looking and fixing the issue! Very much appreciated. @Inveracity would you mind seeing if that fix works for you?

@Ch3LL Ch3LL added the fixed-pls-verify fix is linked, bug author to confirm fix label Jun 14, 2016
@rallytime
Copy link
Contributor

@jizhilong I think this fix is needed in the 2016.3 branch, correct? Should your fix be back-ported?

@Inveracity
Copy link
Contributor Author

@Ch3LL I'll be sure to test it and report back tomorrow morning (eu time) when I get into the office

@Inveracity
Copy link
Contributor Author

I brought up my test environment
did a fresh install of the salt-master
made no changes to the redis_return.py code to confirm that the error was happening still.
But instead I'm seeing this now:

[DEBUG   ] Could not LazyLoad redis.prep_jid
[ERROR   ] Returner 'redis' does not support function prep_jid
[ERROR   ] Error in function _return:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/master.py", line 1540, in run_func
    ret = getattr(self, func)(load)
  File "/usr/lib/python2.7/dist-packages/salt/master.py", line 1353, in _return
    self.opts, load, event=self.event, mminion=self.mminion)
  File "/usr/lib/python2.7/dist-packages/salt/utils/job.py", line 44, in store_job
    raise KeyError(emsg)
KeyError: "Returner 'redis' does not support function prep_jid"
[DEBUG   ] Could not LazyLoad redis.prep_jid

the redis server is up and listening and it responds on the port using telnet.

redis.db: '0'
redis.host: backend-database
redis.port: 6379
master_job_cache: redis

So now I can't confirm whether the fixed worked for me
Am I missing something?
Should I open a separate issue for this?

Thanks!

@jizhilong
Copy link
Contributor

@Inveracity it seems the KeyError is a new issue

@cachedout cachedout removed the fixed-pls-verify fix is linked, bug author to confirm fix label Jun 23, 2016
@Inveracity
Copy link
Contributor Author

The keyerror was a fluke, couldn't reproduce it again

The original problem is fixed, thank you!

rallytime pushed a commit to rallytime/salt that referenced this issue Sep 2, 2016
rallytime pushed a commit that referenced this issue Sep 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior P4 Priority 4 Returners RIoT Relates to integration with cloud providers, hypervisors, API-based services, etc. severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

5 participants