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

Runner job `load` is never saved to job cache #43453

Closed
kunal-bajpai opened this issue Sep 12, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@kunal-bajpai
Copy link
Contributor

commented Sep 12, 2017

Description of Issue/Question

Runner code is broken. It never saves request load to the job cache.

Why does it usually work then? The returner local_cache is written such that it does not require the request load to be present to fetch job returns from the cache. It simply traverses the directory looking for hostnames. For other returners such as couchbase, load not being present in the job_cache will cause fetching job returns to fail.

ROOT CAUSE: Runner uses utils.job.store_job to store the job in the job cache. In this method, the condition that is checked before calling save_load is doomed to always return False.

salt/salt/utils/job.py

Lines 102 to 104 in b4ca024

if 'jid' in load \
and 'get_load' in mminion.returners \
and not mminion.returners[getfstr](load.get('jid', '')):

  1. get_load in returner loader. This check is incorrect as it does not conform to the loader interface. It should check for '{0}.get_load'.format(job_cache)
  2. not mminion.returners[getfstr](load.get('jid', '')). This check is redundant. It seems to be intended to make sure that no document/entry with the key jid is present in the job cache. It is not needed as it is the job of prep_jid to ensure JIDs do not collide. Should be removed.

Setup

master_job_cache: couchbase in master configs

Steps to Reproduce Issue

  1. Fire a runner test.stdout_print
  2. Lookup the return for the runner just fired. salt-run jobs.lookup_jid <runner_jid>
    Fails to find request load.

Might run in to issue #43452 while trying to reproduce using couchbase returner

Versions Report

Salt 2016.11.07

@kunal-bajpai

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2017

$ sudo salt-run test.stdout_print 2> /dev/null                                                                                                                                                                 
foo
bar
$ sudo salt-run jobs.lookup_jid 20170912114947701671 2> /dev/null
Exception occurred in runner jobs.lookup_jid: Traceback (most recent call last):
  File "/root/.pex/install/salt-2016.11.7-py2-none-any.whl.249b465ff64d91b2728e6ba08207c57bb133c22f/salt-2016.11.7-py2-none-any.whl/salt/client/mixins.py", line 395, in _low
    data['return'] = self.functions[fun](*args, **kwargs)
  File "/root/.pex/install/salt-2016.11.7-py2-none-any.whl.249b465ff64d91b2728e6ba08207c57bb133c22f/salt-2016.11.7-py2-none-any.whl/salt/runners/jobs.py", line 126, in lookup_jid
    display_progress=display_progress
  File "/root/.pex/install/salt-2016.11.7-py2-none-any.whl.249b465ff64d91b2728e6ba08207c57bb133c22f/salt-2016.11.7-py2-none-any.whl/salt/runners/jobs.py", line 195, in list_job
    job = mminion.returners['{0}.get_load'.format(returner)](jid)
  File "/root/.pex/install/salt-2016.11.7-py2-none-any.whl.249b465ff64d91b2728e6ba08207c57bb133c22f/salt-2016.11.7-py2-none-any.whl/salt/returners/couchbase_return.py", line 266, in get_load
    ret = jid_doc.value['load']
KeyError: 'load'

The above stacktrace is because load is not saved.

@garethgreenaway

This comment has been minimized.

Copy link
Member

commented Sep 12, 2017

@kunal-bajpai Thanks for the report and the PR!

@kunal-bajpai

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2017

On second thought, the check for getfstr is redundant as it is already done a couple lines before here

salt/salt/utils/job.py

Lines 93 to 100 in b4ca024

try:
savefstr_func = mminion.returners[savefstr]
getfstr_func = mminion.returners[getfstr]
fstr_func = mminion.returners[fstr]
except KeyError as error:
emsg = "Returner '{0}' does not support function {1}".format(job_cache, error)
log.error(emsg)
raise KeyError(emsg)

Replace the check for 'jid' key with a try/except block as it is more pythonic (follows EAFP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.