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

2017.7 fix for list compound targeting #49897

Closed

Conversation

Projects
None yet
3 participants
@brejoc
Copy link
Member

commented Oct 4, 2018

What does this PR do?

This is a follow up of #49435, but for 2017.7 instead of develop.

What issues does this PR fix or reference?

Issue: #49205
PR to develop. #49435

Previous Behavior & New Behavior

Please see the comments in #49435.

Tests written?

No. Only very small adjustments to existing test.

Commits signed with GPG?

Yes

@brejoc brejoc requested a review from saltstack/team-core as a code owner Oct 4, 2018

@salt-jenkins salt-jenkins requested a review from saltstack/team-suse Oct 4, 2018

@brejoc

This comment has been minimized.

Copy link
Member Author

commented Oct 4, 2018

@rallytime Should I also create an PR for 2018.3?

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2018

@brejoc No, I don't think so. This will merge forward to 2018.3. Thanks!

brejoc and others added some commits Aug 30, 2018

No longer passes missing in list compound engine
Missing targets cause a very odd behaviour in combination with a
"not" statement and don't seem to be used in every other case.

Having a compound list target like "not L@foominion" would cause
a return for "foominon" even though it doesn't exist. Returning
an empty list for "missing" fixes that.
Ignore missing minions only when excluding them with 'not'
Co-authored-by: Mihai Dinca <Mihai.Dinca@suse.com>
Minor fix for def test_batch_run_grains_targeting
item was never "minion" without a colon, but os_grain was overwritten
anyway. So this fix does not actually affect the test outcome.
Adds check for engine before adding the additional engine argument
The additional engine argument is only intended to be used with the
_check_list_minons engine and shouldn't be added for every other engine.

@brejoc brejoc force-pushed the brejoc:2017.7-fix-for-list-compound-targeting branch from 756bdf8 to 0de44f3 Oct 4, 2018

'''
Return the minions found by looking via a list
'''
if isinstance(expr, six.string_types):
expr = [m for m in expr.split(',') if m]
minions = self._pki_minions()
return [x for x in expr if x in minions]

This comment has been minimized.

Copy link
@brejoc

brejoc Oct 4, 2018

Author Member

Oh, I think I mixed something up here. Maybe 2017.7 is too early and we only need that for 2018.3. Seems like the missing key was not yet introduced. I'm re-checking this. Sorry!

@brejoc

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2018

I'm closing this because of temporary mental derangement on my side. Opening up an other one for 2018.3. Sorry for the confusion!

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.