saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-17 10:10:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Support states returning a list for ret['comment']

Browse source 
Some states are complicated and multiple subparts,
or maybe cross-call into __states__ if they manage subresources.
In these cases, they will have multiple comments.
Make this more ergonomic by supporting a list of strings as the
value for ret['comment'] in state returns and documenting this.
By joining comments on newlines, it is possible to combine
single-line and multi-line comments cleanly, as opposed to e.g. commas.

The driving impetus for this is some of the boto modules.
An update to the boto_sqs module is included as an example.

Add a check that outgoing state return data has the right shape,
and add a testcase as well.
Fix the NPM state tests and the saltmod runner & wheel state functions
to comply with the prescribed format.
This commit is contained in:
Aneesh Agrawal 2017-08-19 00:06:15 +00:00
parent 0d1b715b97
commit ba63920874
9 changed files with 154 additions and 62 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
doc/ref/states/writing.rst
View file

@ -153,7 +153,12 @@ A State Module must return a dict containing the following keys/values:
However, if a state is going to fail and this can be determined
in test mode without applying the change, ``False`` can be returned.
- **comment:** A string containing a summary of the result.
- **comment:** A list of strings or a single string summarizing the result.
Note that support for lists of strings is available as of Salt Oxygen.
Lists of strings will be joined with newlines to form the final comment;
this is useful to allow multiple comments from subparts of a state.
Prefer to keep line lengths short (use multiple lines as needed),
and end with punctuation (e.g. a period) to delimit multiple comments.
The return data can also, include the **pchanges** key, this stands for
`predictive changes`. The **pchanges** key informs the State system what

6
doc/topics/releases/oxygen.rst
View file

@ -110,6 +110,12 @@ Support has been added to the ``virtual`` grain for detecting Solaris LDOMs
running on T-Series SPARC hardware. The ``virtual_subtype`` grain is
populated as a list of domain roles.
Lists of comments in state returns
----------------------------------
State functions can now return a list of strings for the ``comment`` field,
as opposed to only a single string.
This is meant to ease writing states with multiple or multi-part comments.
Beacon configuration changes
----------------------------------------

59
salt/state.py
View file

@ -969,22 +969,65 @@ class State(object):
elif data[u'state'] in (u'pkg', u'ports'):
self.module_refresh()
def verify_ret(self, ret):
@staticmethod
def verify_ret(ret):
'''
Verify the state return data
Perform basic verification of the raw state return data
'''
if not isinstance(ret, dict):
raise SaltException(
u'Malformed state return, return must be a dict'
)
u'Malformed state return, return must be a dict'
)
bad = []
for val in [u'name', u'result', u'changes', u'comment']:
if val not in ret:
bad.append(val)
if bad:
raise SaltException(
u'The following keys were not present in the state '
u'return: {0}'.format(u','.join(bad)))
m = u'The following keys were not present in the state return: {0}'
raise SaltException(m.format(u','.join(bad)))
@staticmethod
def munge_ret_for_export(ret):
'''
Process raw state return data to make it suitable for export,
to ensure consistency of the data format seen by external systems
'''
# We support lists of strings for ret['comment'] internal
# to the state system for improved ergonomics.
# However, to maintain backwards compatability with external tools,
# the list representation is not allowed to leave the state system,
# and should be converted like this at external boundaries.
if isinstance(ret[u'comment'], list):
ret[u'comment'] = u'\n'.join(ret[u'comment'])
@staticmethod
def verify_ret_for_export(ret):
'''
Verify the state return data for export outside the state system
'''
State.verify_ret(ret)
for key in [u'name', u'comment']:
if not isinstance(ret[key], six.string_types):
msg = (
u'The value for the {0} key in the state return '
u'must be a string, found {1}'
)
raise SaltException(msg.format(key, repr(ret[key])))
if ret[u'result'] not in [True, False, None]:
msg = (
u'The value for the result key in the state return '
u'must be True, False, or None, found {0}'
)
raise SaltException(msg.format(repr(ret[u'result'])))
if not isinstance(ret[u'changes'], dict):
msg = (
u'The value for the changes key in the state return '
u'must be a dict, found {0}'
)
raise SaltException(msg.format(repr(ret[u'changes'])))
def verify_data(self, data):
'''
@ -1847,6 +1890,7 @@ class State(object):
if u'check_cmd' in low and u'{0[state]}.mod_run_check_cmd'.format(low) not in self.states:
ret.update(self._run_check_cmd(low))
self.verify_ret(ret)
self.munge_ret_for_export(ret)
except Exception:
trb = traceback.format_exc()
# There are a number of possibilities to not have the cdata
@ -1874,6 +1918,7 @@ class State(object):
self.state_con.pop('runas')
self.state_con.pop('runas_password')
self.verify_ret_for_export(ret)
# If format_call got any warnings, let's show them to the user
if u'warnings' in cdata:

64
salt/states/boto_sqs.py
View file

@ -108,8 +108,12 @@ def present(
A dict with region, key and keyid, or a pillar key (string)
that contains a dict with region, key and keyid.
'''
comments = []
ret = {'name': name, 'result': True, 'changes': {}}
ret = {
'name': name,
'result': True,
'comment': [],
'changes': {},
}
r = __salt__['boto_sqs.exists'](
name,
@ -120,17 +124,18 @@ def present(
)
if 'error' in r:
ret['result'] = False
ret['comment'] = '\n'.join(comments + [str(r['error'])])
ret['comment'].append(r['error'])
return ret
if r['result']:
comments.append('SQS queue {0} present.'.format(name))
ret['comment'].append('SQS queue {0} present.'.format(name))
else:
if __opts__['test']:
ret['result'] = None
comments.append('SQS queue {0} is set to be created.'.format(name))
ret['comment'].append(
'SQS queue {0} is set to be created.'.format(name),
)
ret['pchanges'] = {'old': None, 'new': name}
ret['comment'] = '\n'.join(comments)
return ret
r = __salt__['boto_sqs.create'](
@ -143,22 +148,18 @@ def present(
)
if 'error' in r:
ret['result'] = False
comments.append('Failed to create SQS queue {0}: {1}'.format(
name,
str(r['error']),
))
ret['comment'] = '\n'.join(comments)
ret['comment'].append(
'Failed to create SQS queue {0}: {1}'.format(name, r['error']),
)
return ret
comments.append('SQS queue {0} created.'.format(name))
ret['comment'].append('SQS queue {0} created.'.format(name))
ret['changes']['old'] = None
ret['changes']['new'] = name
# Return immediately, as the create call also set all attributes
ret['comment'] = '\n'.join(comments)
return ret
if not attributes:
ret['comment'] = '\n'.join(comments)
return ret
r = __salt__['boto_sqs.get_attributes'](
@ -170,10 +171,9 @@ def present(
)
if 'error' in r:
ret['result'] = False
comments.append('Failed to get queue attributes: {0}'.format(
str(r['error']),
))
ret['comment'] = '\n'.join(comments)
ret['comment'].append(
'Failed to get queue attributes: {0}'.format(r['error']),
)
return ret
current_attributes = r['result']
@ -195,8 +195,7 @@ def present(
attr_names = ', '.join(attrs_to_set)
if not attrs_to_set:
comments.append('Queue attributes already set correctly.')
ret['comment'] = '\n'.join(comments)
ret['comment'].append('Queue attributes already set correctly.')
return ret
final_attributes = current_attributes.copy()
@ -218,12 +217,13 @@ def present(
if __opts__['test']:
ret['result'] = None
comments.append('Attribute(s) {0} set to be updated:'.format(
attr_names,
))
comments.append(attributes_diff)
ret['comment'].append(
'Attribute(s) {0} set to be updated:\n{1}'.format(
attr_names,
attributes_diff,
)
)
ret['pchanges'] = {'attributes': {'diff': attributes_diff}}
ret['comment'] = '\n'.join(comments)
return ret
r = __salt__['boto_sqs.set_attributes'](
@ -236,15 +236,15 @@ def present(
)
if 'error' in r:
ret['result'] = False
comments.append('Failed to set queue attributes: {0}'.format(
str(r['error']),
))
ret['comment'] = '\n'.join(comments)
ret['comment'].append(
'Failed to set queue attributes: {0}'.format(r['error']),
)
return ret
comments.append('Updated SQS queue attribute(s) {0}.'.format(attr_names))
ret['comment'].append(
'Updated SQS queue attribute(s) {0}.'.format(attr_names),
)
ret['changes']['attributes'] = {'diff': attributes_diff}
ret['comment'] = '\n'.join(comments)
return ret
@ -291,7 +291,7 @@ def absent(
if not r['result']:
ret['comment'] = 'SQS queue {0} does not exist in {1}.'.format(
name,
region
region,
)
return ret

22
salt/states/saltmod.py
View file

@ -742,18 +742,26 @@ def runner(name, **kwargs):
if isinstance(runner_return, dict) and 'Error' in runner_return:
out['success'] = False
if not out.get('success', True):
cmt = "Runner function '{0}' failed{1}.".format(
name,
' with return {0}'.format(runner_return) if runner_return else '',
)
ret = {
'name': name,
'result': False,
'changes': {},
'comment': runner_return if runner_return else "Runner function '{0}' failed without comment.".format(name)
'comment': cmt,
}
else:
cmt = "Runner function '{0}' executed{1}.".format(
name,
' with return {0}'.format(runner_return) if runner_return else '',
)
ret = {
'name': name,
'result': True,
'changes': runner_return if runner_return else {},
'comment': "Runner function '{0}' executed.".format(name)
'changes': {},
'comment': cmt,
}
ret['__orchestration__'] = True
@ -802,14 +810,14 @@ def wheel(name, **kwargs):
**kwargs)
ret['result'] = True
ret['comment'] = "Wheel function '{0}' executed.".format(name)
ret['__orchestration__'] = True
if 'jid' in out:
ret['__jid__'] = out['jid']
runner_return = out.get('return')
if runner_return:
ret['changes'] = runner_return
ret['comment'] = "Wheel function '{0}' executed{1}.".format(
name,
' with return {0}'.format(runner_return) if runner_return else '',
)
return ret

4
tests/integration/states/test_npm.py
View file

@ -54,7 +54,7 @@ class NpmStateTest(ModuleCase, SaltReturnAssertsMixin):
Basic test to determine if NPM module successfully installs multiple
packages.
'''
ret = self.run_state('npm.installed', name=None, pkgs=['pm2', 'grunt'])
ret = self.run_state('npm.installed', name='unused', pkgs=['pm2', 'grunt'])
self.assertSaltTrueReturn(ret)
@skipIf(salt.utils.path.which('npm') and LooseVersion(cmd.run('npm -v')) >= LooseVersion(MAX_NPM_VERSION),
@ -64,5 +64,5 @@ class NpmStateTest(ModuleCase, SaltReturnAssertsMixin):
'''
Basic test to determine if NPM successfully cleans its cached packages.
'''
ret = self.run_state('npm.cache_cleaned', name=None, force=True)
ret = self.run_state('npm.cache_cleaned', name='unused', force=True)
self.assertSaltTrueReturn(ret)

18
tests/unit/states/test_boto_sqs.py
View file

@ -62,15 +62,15 @@ class BotoSqsTestCase(TestCase, LoaderModuleMockMixin):
'boto_sqs.create': mock_bool,
'boto_sqs.get_attributes': mock_attr}):
with patch.dict(boto_sqs.__opts__, {'test': False}):
comt = 'Failed to create SQS queue {0}: create error'.format(
comt = ['Failed to create SQS queue {0}: create error'.format(
name,
)
)]
ret = base_ret.copy()
ret.update({'result': False, 'comment': comt})
self.assertDictEqual(boto_sqs.present(name), ret)
with patch.dict(boto_sqs.__opts__, {'test': True}):
comt = 'SQS queue {0} is set to be created.'.format(name)
comt = ['SQS queue {0} is set to be created.'.format(name)]
ret = base_ret.copy()
ret.update({
'result': None,
@ -85,17 +85,19 @@ class BotoSqsTestCase(TestCase, LoaderModuleMockMixin):
-{}
+DelaySeconds: 20
''')
comt = textwrap.dedent('''\
SQS queue mysqs present.
Attribute(s) DelaySeconds set to be updated:
''') + diff
comt = [
'SQS queue mysqs present.',
'Attribute(s) DelaySeconds set to be updated:\n{0}'.format(
diff,
),
]
ret.update({
'comment': comt,
'pchanges': {'attributes': {'diff': diff}},
})
self.assertDictEqual(boto_sqs.present(name, attributes), ret)
comt = ('SQS queue mysqs present.')
comt = ['SQS queue mysqs present.']
ret = base_ret.copy()
ret.update({'result': True, 'comment': comt})
self.assertDictEqual(boto_sqs.present(name), ret)

8
tests/unit/states/test_saltmod.py
View file

@ -251,8 +251,8 @@ class SaltmodTestCase(TestCase, LoaderModuleMockMixin):
'''
name = 'state'
ret = {'changes': True, 'name': 'state', 'result': True,
'comment': 'Runner function \'state\' executed.',
ret = {'changes': {}, 'name': 'state', 'result': True,
'comment': 'Runner function \'state\' executed with return True.',
'__orchestration__': True}
runner_mock = MagicMock(return_value={'return': True})
@ -267,8 +267,8 @@ class SaltmodTestCase(TestCase, LoaderModuleMockMixin):
'''
name = 'state'
ret = {'changes': True, 'name': 'state', 'result': True,
'comment': 'Wheel function \'state\' executed.',
ret = {'changes': {}, 'name': 'state', 'result': True,
'comment': 'Wheel function \'state\' executed with return True.',
'__orchestration__': True}
wheel_mock = MagicMock(return_value={'return': True})

28
tests/unit/test_state.py
View file

@ -16,8 +16,9 @@ from tests.support.mock import NO_MOCK, NO_MOCK_REASON, patch
from tests.support.mixins import AdaptedConfigurationTestCaseMixin
# Import Salt libs
import salt.state
import salt.exceptions
from salt.ext import six
import salt.state
from salt.utils.odict import OrderedDict, DefaultOrderedDict
@ -472,3 +473,28 @@ class TopFileMergeTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
expected_merge = DefaultOrderedDict(OrderedDict)
self.assertEqual(merged_tops, expected_merge)
class StateReturnsTestCase(TestCase):
'''
TestCase for code handling state returns.
'''
def test_comment_lists_are_converted_to_string(self):
'''
Tests that states returning a list of comments
have that converted to a single string
'''
ret = {
'name': 'myresource',
'result': True,
'comment': ['comment 1', 'comment 2'],
'changes': {},
}
salt.state.State.verify_ret(ret) # sanity check
with self.assertRaises(salt.exceptions.SaltException):
# Not suitable for export as is
salt.state.State.verify_ret_for_export(ret)
salt.state.State.munge_ret_for_export(ret)
self.assertIsInstance(ret[u'comment'], six.string_types)
salt.state.State.verify_ret_for_export(ret)