wbrawner/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Track notified handlers by object rather than simply their name

Browse source 
Due to the fact that roles may be instantiated with different sets of
params (multiple inclusions of the same role or via role dependencies),
simply tracking notified handlers by name does not work. This patch
changes the way we track handler notifications by using the handler
object itself instead of just the name, allowing for multiple internal
instances. Normally this would be bad, but we also modify the way we
search for handlers by first looking at the notifying tasks dependency
chain (ensuring that roles find their own handlers first) and then at
the main list of handlers, using the first match it finds.

This patch also modifies the way we setup the internal list of handlers,
which should allow us to correctly identify if a notified handler exists
more easily.

Fixes #15084
This commit is contained in:
James Cammarata 2016-06-12 18:03:18 -05:00
parent f6727aff39
commit 43d1ea0cfc
4 changed files with 77 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
lib/ansible/executor/task_queue_manager.py
View file

@ -114,17 +114,20 @@ class TaskQueueManager:
self._result_prc = ResultProcess(self._final_q, self._workers)
self._result_prc.start()
def _initialize_notified_handlers(self, handlers):
def _initialize_notified_handlers(self, play):
'''
Clears and initializes the shared notified handlers dict with entries
for each handler in the play, which is an empty array that will contain
inventory hostnames for those hosts triggering the handler.
'''
handlers = play.handlers
for role in play.roles:
handlers.extend(role._handler_blocks)
# Zero the dictionary first by removing any entries there.
# Proxied dicts don't support iteritems, so we have to use keys()
for key in self._notified_handlers.keys():
del self._notified_handlers[key]
self._notified_handlers.clear()
def _process_block(b):
temp_list = []
@ -139,9 +142,10 @@ class TaskQueueManager:
for handler_block in handlers:
handler_list.extend(_process_block(handler_block))
# then initialize it with the handler names from the handler list
# then initialize it with the given handler list
for handler in handler_list:
self._notified_handlers[handler.get_name()] = []
if handler not in self._notified_handlers:
self._notified_handlers[handler] = []
def load_callbacks(self):
'''
@ -226,7 +230,7 @@ class TaskQueueManager:
self.send_callback('v2_playbook_on_play_start', new_play)
# initialize the shared dictionary containing the notified handlers
self._initialize_notified_handlers(new_play.handlers)
self._initialize_notified_handlers(new_play)
# load the specified strategy (or the default linear one)
strategy = strategy_loader.get(new_play.strategy, self)

2
lib/ansible/playbook/task.py
View file

@ -105,7 +105,7 @@ class Task(Base, Conditional, Taggable, Become):
def get_name(self):
''' return the name of the task '''
if self._role and self.name:
if self._role and self.name and ("%s : " % self._role._role_name) not in self.name:
return "%s : %s" % (self._role.get_name(), self.name)
elif self.name:
return self.name

73
lib/ansible/plugins/strategy/__init__.py
View file

@ -318,11 +318,51 @@ class StrategyBase:
original_host = get_original_host(task_result._host)
original_task = iterator.get_original_task(original_host, task_result._task)
if handler_name not in self._notified_handlers:
self._notified_handlers[handler_name] = []
if original_host not in self._notified_handlers[handler_name]:
self._notified_handlers[handler_name].append(original_host)
def search_handler_blocks(handler_blocks):
for handler_block in handler_blocks:
for handler_task in handler_block.block:
handler_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, task=handler_task)
templar = Templar(loader=self._loader, variables=handler_vars)
try:
# first we check with the full result of get_name(), which may
# include the role name (if the handler is from a role). If that
# is not found, we resort to the simple name field, which doesn't
# have anything extra added to it.
target_handler_name = templar.template(handler_task.name)
if target_handler_name == handler_name:
return handler_task
else:
target_handler_name = templar.template(handler_task.get_name())
if target_handler_name == handler_name:
return handler_task
except (UndefinedError, AnsibleUndefinedVariable):
# We skip this handler due to the fact that it may be using
# a variable in the name that was conditionally included via
# set_fact or some other method, and we don't want to error
# out unnecessarily
continue
return None
# Find the handler using the above helper. First we look up the
# dependency chain of the current task (if it's from a role), otherwise
# we just look through the list of handlers in the current play/all
# roles and use the first one that matches the notify name
target_handler = None
if original_task._role:
target_handler = search_handler_blocks(original_task._role.get_handler_blocks())
if target_handler is None:
target_handler = search_handler_blocks(iterator._play.handlers)
if target_handler is None:
raise AnsibleError("The requested handler '%s' was not found in any of the known handlers" % handler_name)
# FIXME: this should be an error now in 2.1+
if target_handler not in self._notified_handlers:
self._notified_handlers[target_handler] = []
if original_host not in self._notified_handlers[target_handler]:
self._notified_handlers[target_handler].append(original_host)
# FIXME: should this be a callback?
display.vv("NOTIFIED HANDLER %s" % (handler_name,))
elif result[0] == 'register_host_var':
@ -572,25 +612,8 @@ class StrategyBase:
# but this may take some work in the iterator and gets tricky when
# we consider the ability of meta tasks to flush handlers
for handler in handler_block.block:
handler_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, task=handler)
templar = Templar(loader=self._loader, variables=handler_vars)
try:
# first we check with the full result of get_name(), which may
# include the role name (if the handler is from a role). If that
# is not found, we resort to the simple name field, which doesn't
# have anything extra added to it.
handler_name = templar.template(handler.name)
if handler_name not in self._notified_handlers:
handler_name = templar.template(handler.get_name())
except (UndefinedError, AnsibleUndefinedVariable):
# We skip this handler due to the fact that it may be using
# a variable in the name that was conditionally included via
# set_fact or some other method, and we don't want to error
# out unnecessarily
continue
if handler_name in self._notified_handlers and len(self._notified_handlers[handler_name]):
result = self._do_handler_run(handler, handler_name, iterator=iterator, play_context=play_context)
if handler in self._notified_handlers and len(self._notified_handlers[handler]):
result = self._do_handler_run(handler, handler.get_name(), iterator=iterator, play_context=play_context)
if not result:
break
return result
@ -608,7 +631,7 @@ class StrategyBase:
handler.name = saved_name
if notified_hosts is None:
notified_hosts = self._notified_handlers[handler_name]
notified_hosts = self._notified_handlers[handler]
run_once = False
try:
@ -671,7 +694,7 @@ class StrategyBase:
continue
# wipe the notification list
self._notified_handlers[handler_name] = []
self._notified_handlers[handler] = []
display.debug("done running handlers, result is: %s" % result)
return result

21
test/units/plugins/strategies/test_strategy_base.py
View file

@ -171,7 +171,10 @@ class TestStrategyBase(unittest.TestCase):
mock_tqm._stats = MagicMock()
mock_tqm._stats.increment.return_value = None
mock_play = MagicMock()
mock_iterator = MagicMock()
mock_iterator._play = mock_play
mock_iterator.mark_host_failed.return_value = None
mock_iterator.get_next_task_for_host.return_value = (None, None)
@ -184,6 +187,17 @@ class TestStrategyBase(unittest.TestCase):
mock_task._role = None
mock_task.ignore_errors = False
mock_handler_task = MagicMock(Handler)
mock_handler_task.action = 'foo'
mock_handler_task.get_name.return_value = "test handler"
mock_handler_task.has_triggered.return_value = False
mock_handler_block = MagicMock()
mock_handler_block.block = [mock_handler_task]
mock_play.handlers = [mock_handler_block]
mock_tqm._notified_handlers = {mock_handler_task: []}
mock_group = MagicMock()
mock_group.add_host.return_value = None
@ -281,8 +295,8 @@ class TestStrategyBase(unittest.TestCase):
self.assertEqual(len(results), 0)
self.assertEqual(strategy_base._pending_results, 1)
self.assertIn('test01', strategy_base._blocked_hosts)
self.assertIn('test handler', strategy_base._notified_handlers)
self.assertIn(mock_host, strategy_base._notified_handlers['test handler'])
self.assertIn(mock_handler_task, strategy_base._notified_handlers)
self.assertIn(mock_host, strategy_base._notified_handlers[mock_handler_task])
queue_items.append(('set_host_var', mock_host, mock_task, None, 'foo', 'bar'))
results = strategy_base._process_pending_results(iterator=mock_iterator)
@ -379,13 +393,14 @@ class TestStrategyBase(unittest.TestCase):
passwords=None,
)
tqm._initialize_processes(3)
tqm._initialize_notified_handlers(mock_play)
tqm.hostvars = dict()
try:
strategy_base = StrategyBase(tqm=tqm)
strategy_base._inventory = mock_inventory
strategy_base._notified_handlers = {"test handler": [mock_host]}
strategy_base._notified_handlers = {mock_handler_task: [mock_host]}
task_result = TaskResult(Host('host01'), Handler(), dict(changed=False))
tqm._final_q.put(('host_task_ok', task_result))