If we can't squash for any reason, then simply do not optimize the items loop.

Also add more squashing testcases

Fixes #15649
This commit is contained in:
Toshio Kuratomi 2016-05-11 18:26:39 -07:00
parent 8cd0c432e7
commit 90fb1fb3fa
2 changed files with 137 additions and 56 deletions

View file

@ -269,59 +269,64 @@ class TaskExecutor:
Squash items down to a comma-separated list for certain modules which support it Squash items down to a comma-separated list for certain modules which support it
(typically package management modules). (typically package management modules).
''' '''
# _task.action could contain templatable strings (via action: and try:
# local_action:) Template it before comparing. If we don't end up # _task.action could contain templatable strings (via action: and
# optimizing it here, the templatable string might use template vars # local_action:) Template it before comparing. If we don't end up
# that aren't available until later (it could even use vars from the # optimizing it here, the templatable string might use template vars
# with_items loop) so don't make the templated string permanent yet. # that aren't available until later (it could even use vars from the
templar = Templar(loader=self._loader, shared_loader_obj=self._shared_loader_obj, variables=variables) # with_items loop) so don't make the templated string permanent yet.
task_action = self._task.action templar = Templar(loader=self._loader, shared_loader_obj=self._shared_loader_obj, variables=variables)
if templar._contains_vars(task_action): task_action = self._task.action
task_action = templar.template(task_action, fail_on_undefined=False) if templar._contains_vars(task_action):
task_action = templar.template(task_action, fail_on_undefined=False)
if len(items) > 0 and task_action in self.SQUASH_ACTIONS: if len(items) > 0 and task_action in self.SQUASH_ACTIONS:
if all(isinstance(o, string_types) for o in items): if all(isinstance(o, string_types) for o in items):
final_items = [] final_items = []
name = None name = None
for allowed in ['name', 'pkg', 'package']: for allowed in ['name', 'pkg', 'package']:
name = self._task.args.pop(allowed, None) name = self._task.args.pop(allowed, None)
if name is not None: if name is not None:
break break
# This gets the information to check whether the name field # This gets the information to check whether the name field
# contains a template that we can squash for # contains a template that we can squash for
template_no_item = template_with_item = None template_no_item = template_with_item = None
if name: if name:
if templar._contains_vars(name): if templar._contains_vars(name):
variables[loop_var] = '\0$' variables[loop_var] = '\0$'
template_no_item = templar.template(name, variables, cache=False) template_no_item = templar.template(name, variables, cache=False)
variables[loop_var] = '\0@' variables[loop_var] = '\0@'
template_with_item = templar.template(name, variables, cache=False) template_with_item = templar.template(name, variables, cache=False)
del variables[loop_var] del variables[loop_var]
# Check if the user is doing some operation that doesn't take # Check if the user is doing some operation that doesn't take
# name/pkg or the name/pkg field doesn't have any variables # name/pkg or the name/pkg field doesn't have any variables
# and thus the items can't be squashed # and thus the items can't be squashed
if template_no_item != template_with_item: if template_no_item != template_with_item:
for item in items: for item in items:
variables[loop_var] = item variables[loop_var] = item
if self._task.evaluate_conditional(templar, variables): if self._task.evaluate_conditional(templar, variables):
new_item = templar.template(name, cache=False) new_item = templar.template(name, cache=False)
final_items.append(new_item) final_items.append(new_item)
self._task.args['name'] = final_items self._task.args['name'] = final_items
# Wrap this in a list so that the calling function loop # Wrap this in a list so that the calling function loop
# executes exactly once # executes exactly once
return [final_items] return [final_items]
else: else:
# Restore the name parameter # Restore the name parameter
self._task.args['name'] = name self._task.args['name'] = name
#elif: #elif:
# Right now we only optimize single entries. In the future we # Right now we only optimize single entries. In the future we
# could optimize more types: # could optimize more types:
# * lists can be squashed together # * lists can be squashed together
# * dicts could squash entries that match in all cases except the # * dicts could squash entries that match in all cases except the
# name or pkg field. # name or pkg field.
except:
# Squashing is an optimization. If it fails for any reason,
# simply use the unoptimized list of items.
pass
return items return items
def _execute(self, variables=None): def _execute(self, variables=None):

View file

@ -180,8 +180,10 @@ class TestTaskExecutor(unittest.TestCase):
mock_host = MagicMock() mock_host = MagicMock()
loop_var = 'item'
def _evaluate_conditional(templar, variables): def _evaluate_conditional(templar, variables):
item = variables.get('item') item = variables.get(loop_var)
if item == 'b': if item == 'b':
return False return False
return True return True
@ -230,9 +232,31 @@ class TestTaskExecutor(unittest.TestCase):
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars) new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, ['a', 'b', 'c']) self.assertEqual(new_items, ['a', 'b', 'c'])
mock_task.action = '{{unknown}}'
mock_task.args={'name': '{{item}}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, ['a', 'b', 'c'])
# Maybe should raise an error in this case. The user would have to specify:
# - yum: name="{{ packages[item] }}"
# with_items:
# - ['a', 'b']
# - ['foo', 'bar']
# you can't use a list as a dict key so that would probably throw
# an error later. If so, we can throw it now instead.
# Squashing in this case would not be intuitive as the user is being
# explicit in using each list entry as a key.
job_vars = dict(pkg_mgr='yum', packages={ "a": "foo", "b": "bar", "foo": "baz", "bar": "quux" })
items = [['a', 'b'], ['foo', 'bar']]
mock_task.action = 'yum'
mock_task.args = {'name': '{{ packages[item] }}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, items)
# #
# Replaces # Replaces
# #
items = ['a', 'b', 'c']
mock_task.action = 'yum' mock_task.action = 'yum'
mock_task.args={'name': '{{item}}'} mock_task.args={'name': '{{item}}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars) new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
@ -243,22 +267,74 @@ class TestTaskExecutor(unittest.TestCase):
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars) new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, [['a', 'c']]) self.assertEqual(new_items, [['a', 'c']])
# New loop_var
mock_task.action = 'yum'
mock_task.args = {'name': '{{a_loop_var_item}}'}
mock_task.loop_control = {'loop_var': 'a_loop_var_item'}
loop_var = 'a_loop_var_item'
new_items = te._squash_items(items=items, loop_var='a_loop_var_item', variables=job_vars)
self.assertEqual(new_items, [['a', 'c']])
loop_var = 'item'
# #
# Smoketests -- these won't optimize but make sure that they don't # These are presently not optimized but could be in the future.
# traceback either # Expected output if they were optimized is given as a comment
# Please move these to a different section if they are optimized
# #
mock_task.action = '{{unknown}}'
mock_task.args={'name': '{{item}}'} # Squashing lists
job_vars = dict(pkg_mgr='yum')
items = [['a', 'b'], ['foo', 'bar']]
mock_task.action = 'yum'
mock_task.args = {'name': '{{ item }}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars) new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, ['a', 'b', 'c']) #self.assertEqual(new_items, [['a', 'b', 'foo', 'bar']])
self.assertEqual(new_items, items)
# Retrieving from a dict
items = ['a', 'b', 'foo']
mock_task.action = 'yum'
mock_task.args = {'name': '{{ packages[item] }}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
#self.assertEqual(new_items, [['foo', 'baz']])
self.assertEqual(new_items, items)
# Another way to retrieve from a dict
job_vars = dict(pkg_mgr='yum')
items = [{'package': 'foo'}, {'package': 'bar'}]
mock_task.action = 'yum'
mock_task.args = {'name': '{{ item["package"] }}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
#self.assertEqual(new_items, [['foo', 'bar']])
self.assertEqual(new_items, items)
items = [dict(name='a', state='present'), items = [dict(name='a', state='present'),
dict(name='b', state='present'), dict(name='b', state='present'),
dict(name='c', state='present')] dict(name='c', state='present')]
mock_task.action = 'yum' mock_task.action = 'yum'
mock_task.args={'name': '{{item}}'} mock_task.args={'name': '{{item.name}}', 'state': '{{item.state}}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars) new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, items) self.assertEqual(new_items, items)
#self.assertEqual(new_items, [dict(name=['a', 'b', 'c'], state='present')])
items = [dict(name='a', state='present'),
dict(name='b', state='present'),
dict(name='c', state='absent')]
mock_task.action = 'yum'
mock_task.args={'name': '{{item.name}}', 'state': '{{item.state}}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, items)
#self.assertEqual(new_items, [dict(name=['a', 'b'], state='present'),
# dict(name='c', state='absent')])
# Could do something like this to recover from bad deps in a package
job_vars = dict(pkg_mgr='yum', packages=['a', 'b'])
items = [ 'absent', 'latest' ]
mock_task.action = 'yum'
mock_task.args = {'name': '{{ packages }}', 'state': '{{ item }}'}
new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
self.assertEqual(new_items, items)
def test_task_executor_execute(self): def test_task_executor_execute(self):
fake_loader = DictDataLoader({}) fake_loader = DictDataLoader({})