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

Fix issue with salt-run jobs.list_jobs where Target: unknown-target #50328

Merged
merged 5 commits into from Nov 13, 2018

Conversation

Projects
None yet
2 participants
@rallytime
Copy link
Contributor

commented Oct 31, 2018

What does this PR do?

This PR fixes a regression in the local_cache where when running salt-run jobs.list_jobs, the data would contain variables such as Target: unknown-target and Target-type: list instead of the name of the minion for the target, and the correct target type.

A fix in #43454 changed the logic for when mminion.returners[savefstr](load['jid'], load) would be called in salt.utils.job.store_job in order to fix a bug with external job caches. However, this caused a regression when using this with the local_cache.

My PR restores the previous behavior of the local_cache, but keeps the fix for external job caches.

What issues does this PR fix or reference?

Fixes #48734

Previous Behavior

➜  ~ salt rallytime test.ping
rallytime:
    True
➜  ~ salt-run jobs.list_jobs search_function="test.ping"
20181031144832045674:
    ----------
    Arguments:
    Function:
        test.ping
    StartTime:
        2018, Oct 31 14:48:32.045674
    Target:
        unknown-target
    Target-type:
        list
    User:
        root

The Target and Target-type data above are incorrect ^

New Behavior

➜  ~ salt rallytime test.ping
rallytime:
    True
➜  ~ salt-run jobs.list_jobs search_function="test.ping"
20181031155610176789:
    ----------
    Arguments:
    Function:
        test.ping
    StartTime:
        2018, Oct 31 15:56:10.176789
    Target:
        rallytime
    Target-type:
        glob
    User:
        root

The Target and Target-type data above are restored ^

Tests written?

Yes

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

rallytime added some commits Oct 31, 2018

Fix issue #48734
A regression was caused in the local_cache returner with PR #43454.

PR #43454 fixes an issue where `jobs.lookup_jid` was not working correctly
with external job caches.

However, when this issue was fixed, there was a slight logic error that
was overlooked for the local_cache. This fixes the issue with the local_cache
so that the "Target" and "Target Type" data is restored to their previous
values.
@rallytime

This comment has been minimized.

Copy link
Contributor Author

commented Oct 31, 2018

I will fix those py3 test failures tomorrow.

mminion.returners[savefstr](load['jid'], load)
except KeyError as e:
log.error("Load does not contain 'jid': %s", e)
if job_cache != 'local_cache':

This comment has been minimized.

Copy link
@cachedout

cachedout Nov 1, 2018

Collaborator

Why is the local cache different? Trying to solve this at this level doesn't feel like the right approach.

This comment has been minimized.

Copy link
@rallytime

rallytime Nov 1, 2018

Author Contributor

I would normally agree with you, but this was how this worked in the local cache before the logic change in #43454. This was the least invasive way to fix this without going down many local_cache rabbit holes which could potentially break many other things.

This comment has been minimized.

Copy link
@cachedout

cachedout Nov 6, 2018

Collaborator

OK. Thanks for the explanation.

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Nov 6, 2018

Hrm. Still a test failure here though.

@rallytime

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

Ah, I missed that. I will fix!

@rallytime

This comment has been minimized.

Copy link
Contributor Author

commented Nov 8, 2018

Hrm, this is still being flaky. I'm investigating.

@rallytime rallytime requested a review from cachedout Nov 12, 2018

@cachedout cachedout merged commit c1dde7e into saltstack:2018.3 Nov 13, 2018

9 of 10 checks passed

jenkins/pr/py2-windows-2016 The py2-windows-2016 job has failed
Details
WIP Ready for review
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details

@rallytime rallytime deleted the rallytime:fix-48734 branch Nov 13, 2018

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.