Include failed state/function orchestration results in changes dict

Before this commit they would be in the comment, making it difficult to
programatically analyze the failures.
This commit is contained in:
Erik Johnson 2017-11-29 14:16:29 -06:00
parent 8e2e1f957e
commit ee3486a671
No known key found for this signature in database
GPG key ID: 5E5583C437808F3F
5 changed files with 91 additions and 28 deletions

View file

@ -351,7 +351,6 @@ def state(name,
changes = {}
fail = set()
failures = {}
no_change = set()
if fail_minions is None:
@ -393,7 +392,7 @@ def state(name,
if not m_state:
if minion not in fail_minions:
fail.add(minion)
failures[minion] = m_ret or 'Minion did not respond'
changes[minion] = m_ret
continue
try:
for state_item in six.itervalues(m_ret):
@ -418,18 +417,6 @@ def state(name,
state_ret['comment'] += ' Updating {0}.'.format(', '.join(changes))
if no_change:
state_ret['comment'] += ' No changes made to {0}.'.format(', '.join(no_change))
if failures:
state_ret['comment'] += '\nFailures:\n'
for minion, failure in six.iteritems(failures):
state_ret['comment'] += '\n'.join(
(' ' * 4 + l)
for l in salt.output.out_format(
{minion: failure},
'highstate',
__opts__,
).splitlines()
)
state_ret['comment'] += '\n'
if test or __opts__.get('test'):
if state_ret['changes'] and state_ret['result'] is True:
# Test mode with changes is the only case where result should ever be none
@ -570,7 +557,6 @@ def function(
changes = {}
fail = set()
failures = {}
if fail_minions is None:
fail_minions = ()
@ -598,7 +584,7 @@ def function(
if not m_func:
if minion not in fail_minions:
fail.add(minion)
failures[minion] = m_ret and m_ret or 'Minion did not respond'
changes[minion] = m_ret
continue
changes[minion] = m_ret
if not cmd_ret:
@ -614,18 +600,6 @@ def function(
func_ret['comment'] = 'Function ran successfully.'
if changes:
func_ret['comment'] += ' Function {0} ran on {1}.'.format(name, ', '.join(changes))
if failures:
func_ret['comment'] += '\nFailures:\n'
for minion, failure in six.iteritems(failures):
func_ret['comment'] += '\n'.join(
(' ' * 4 + l)
for l in salt.output.out_format(
{minion: failure},
'highstate',
__opts__,
).splitlines()
)
func_ret['comment'] += '\n'
return func_ret

View file

@ -125,3 +125,26 @@ def modules_available(*names):
if not fnmatch.filter(list(__salt__), name):
not_found.append(name)
return not_found
def nonzero_retcode_return_true():
'''
Sets a nonzero retcode before returning. Designed to test orchestration.
'''
__context__['retcode'] = 1
return True
def nonzero_retcode_return_false():
'''
Sets a nonzero retcode before returning. Designed to test orchestration.
'''
__context__['retcode'] = 1
return False
def fail_function(*args, **kwargs): # pylint: disable=unused-argument
'''
Return False no matter what is passed to it
'''
return False

View file

@ -0,0 +1,2 @@
test fail with changes:
test.fail_with_changes

View file

@ -0,0 +1,11 @@
Step01:
salt.state:
- tgt: 'minion'
- sls:
- orch.issue43204.fail_with_changes
Step02:
salt.function:
- name: runtests_helpers.nonzero_retcode_return_false
- tgt: 'minion'
- fail_function: runtests_helpers.fail_function

View file

@ -6,6 +6,7 @@ Tests for the state runner
# Import Python Libs
from __future__ import absolute_import
import errno
import json
import os
import shutil
import signal
@ -81,6 +82,58 @@ class StateRunnerTest(ShellCase):
self.assertFalse(os.path.exists('/tmp/ewu-2016-12-13'))
self.assertNotEqual(code, 0)
def test_orchestrate_state_and_function_failure(self):
'''
Ensure that returns from failed minions are in the changes dict where
they belong, so they can be programatically analyzed.
See https://github.com/saltstack/salt/issues/43204
'''
self.run_run('saltutil.sync_modules')
ret = json.loads(
'\n'.join(
self.run_run(u'state.orchestrate orch.issue43204 --out=json')
)
)
# Drill down to the changes dict
state_ret = ret[u'data'][u'master'][u'salt_|-Step01_|-Step01_|-state'][u'changes']
func_ret = ret[u'data'][u'master'][u'salt_|-Step02_|-runtests_helpers.nonzero_retcode_return_false_|-function'][u'changes']
# Remove duration and start time from the results, since they would
# vary with each run and that would make it impossible to test.
for item in ('duration', 'start_time'):
state_ret['ret']['minion']['test_|-test fail with changes_|-test fail with changes_|-fail_with_changes'].pop(item)
self.assertEqual(
state_ret,
{
u'out': u'highstate',
u'ret': {
u'minion': {
u'test_|-test fail with changes_|-test fail with changes_|-fail_with_changes': {
u'__id__': u'test fail with changes',
u'__run_num__': 0,
u'__sls__': u'orch.issue43204.fail_with_changes',
u'changes': {
u'testing': {
u'new': u'Something pretended to change',
u'old': u'Unchanged'
}
},
u'comment': u'Failure!',
u'name': u'test fail with changes',
u'result': False,
}
}
}
}
)
self.assertEqual(
func_ret,
{u'out': u'highstate', u'ret': {u'minion': False}}
)
def test_orchestrate_target_exists(self):
'''
test orchestration when target exists