archive
/
ansible
mirror of https://github.com/ansible/ansible.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Reworking the way end of role detection is done

Browse Source 
Rather than trying to enumerate tasks or track an ever changing cur_role
flag in PlayIterator, this change simply sets a flag on the last block in
the list of blocks returned by Role.compile(). The PlayIterator then checks
for that flag when the cur_block number is incremented, and marks the role
as complete if the given host had any tasks run in that role.

Fixes #20224
pull/20430/head
James Cammarata 7 years ago
parent d7967aa3a1
commit cae682607c
4 changed files with 56 additions and 99 deletions
Show Stats Download Patch File Download Diff File

104
lib/ansible/executor/play_iterator.py
Unescape Escape View File

@ -48,8 +48,6 @@ class HostState:
self.cur_regular_task = 0
self.cur_rescue_task = 0
self.cur_always_task = 0
self.cur_role = None
self.cur_role_task = None
self.cur_dep_chain = None
self.run_state = PlayIterator.ITERATING_SETUP
self.fail_state = PlayIterator.FAILED_NONE
@ -82,12 +80,11 @@ class HostState:
ret.append(states[i])
return "|".join(ret)
return "HOST STATE: block=%d, task=%d, rescue=%d, always=%d, role=%s, run_state=%s, fail_state=%s, pending_setup=%s, tasks child state? (%s), rescue child state? (%s), always child state? (%s), did rescue? %s, did start at task? %s" % (
return "HOST STATE: block=%d, task=%d, rescue=%d, always=%d, run_state=%s, fail_state=%s, pending_setup=%s, tasks child state? (%s), rescue child state? (%s), always child state? (%s), did rescue? %s, did start at task? %s" % (
self.cur_block,
self.cur_regular_task,
self.cur_rescue_task,
self.cur_always_task,
self.cur_role,
_run_state_to_string(self.run_state),
_failed_state_to_string(self.fail_state),
self.pending_setup,
@ -104,7 +101,7 @@ class HostState:
for attr in (
'_blocks', 'cur_block', 'cur_regular_task', 'cur_rescue_task', 'cur_always_task',
'cur_role', 'run_state', 'fail_state', 'pending_setup', 'cur_dep_chain',
'run_state', 'fail_state', 'pending_setup', 'cur_dep_chain',
'tasks_child_state', 'rescue_child_state', 'always_child_state'
):
if getattr(self, attr) != getattr(other, attr):
@ -121,9 +118,6 @@ class HostState:
new_state.cur_regular_task = self.cur_regular_task
new_state.cur_rescue_task = self.cur_rescue_task
new_state.cur_always_task = self.cur_always_task
new_state.cur_role = self.cur_role
if self.cur_role_task:
new_state.cur_role_task = self.cur_role_task[:]
new_state.run_state = self.run_state
new_state.fail_state = self.fail_state
new_state.pending_setup = self.pending_setup
@ -272,92 +266,6 @@ class PlayIterator:
old_s = s
(s, task) = self._get_next_task_from_state(s, host=host, peek=peek)
def _roles_are_different(ra, rb):
if ra != rb:
return True
else:
return old_s.cur_dep_chain != task.get_dep_chain()
def _role_is_child(r):
parent = task._parent
while parent:
if hasattr(parent, '_role') and parent._role == r and isinstance(parent, IncludeRole):
return True
parent = parent._parent
return False
def _get_cur_task(s, depth=0):
res = [s.run_state, depth, s.cur_block, -1]
if s.run_state == self.ITERATING_TASKS:
if s.tasks_child_state:
res[-1] = [s.cur_regular_task, _get_cur_task(s.tasks_child_state, depth=depth+1)]
else:
res[-1] = s.cur_regular_task
elif s.run_state == self.ITERATING_RESCUE:
if s.rescue_child_state:
res[-1] = [s.cur_rescue_task, _get_cur_task(s.rescue_child_state, depth=depth+1)]
else:
res[-1] = s.cur_rescue_task
elif s.run_state == self.ITERATING_ALWAYS:
if s.always_child_state:
res[-1] = [s.cur_always_task, _get_cur_task(s.always_child_state, depth=depth+1)]
else:
res[-1] = s.cur_always_task
return res
def _do_task_cmp(a, b):
'''
Does the heavy lifting for _role_task_cmp() of comparing task state objects
returned by _get_cur_task() above.
'''
res = cmp(a[0], b[0])
if res == 0:
res = cmp(a[1], b[1])
if res == 0:
res = cmp(a[2], b[2])
if res == 0:
# if there were child states, the last value in the list may be
# a list itself representing the current task position plus a new
# list representing the child state. So here we normalize that so
# we can call this method recursively when all else is equal.
if isinstance(a[3], list):
a_i, a_il = a[3]
else:
a_i = a[3]
a_il = [-1, -1, -1, -1]
if isinstance(b[3], list):
b_i, b_il = b[3]
else:
b_i = b[3]
b_il = [-1, -1, -1, -1]
res = cmp(a_i, b_i)
if res == 0:
res = _do_task_cmp(a_il, b_il)
return res
def _role_task_cmp(s):
'''
Compares the given state against the stored state from the previous role task.
'''
if not s.cur_role_task:
return 1
cur_task = _get_cur_task(s)
return _do_task_cmp(cur_task, s.cur_role_task)
if task and task._role:
# if we had a current role, mark that role as completed
if s.cur_role:
role_diff = _roles_are_different(task._role, s.cur_role)
role_child = _role_is_child(s.cur_role)
tasks_cmp = _role_task_cmp(s)
host_done = host.name in s.cur_role._had_task_run
if (role_diff or (not role_diff and tasks_cmp <= 0)) and host_done and not role_child and not peek:
s.cur_role._completed[host.name] = True
s.cur_role = task._role
s.cur_role_task = _get_cur_task(s)
s.cur_dep_chain = task.get_dep_chain()
if not peek:
self._host_states[host.name] = s
@ -462,7 +370,6 @@ class PlayIterator:
if isinstance(task, Block) or state.tasks_child_state is not None:
state.tasks_child_state = HostState(blocks=[task])
state.tasks_child_state.run_state = self.ITERATING_TASKS
state.tasks_child_state.cur_role = state.cur_role
# since we've created the child state, clear the task
# so we can pick up the child state on the next pass
task = None
@ -493,7 +400,6 @@ class PlayIterator:
if isinstance(task, Block) or state.rescue_child_state is not None:
state.rescue_child_state = HostState(blocks=[task])
state.rescue_child_state.run_state = self.ITERATING_TASKS
state.rescue_child_state.cur_role = state.cur_role
task = None
state.cur_rescue_task += 1
@ -525,12 +431,16 @@ class PlayIterator:
state.rescue_child_state = None
state.always_child_state = None
state.did_rescue = False
# we're advancing blocks, so if this was an end-of-role block we
# mark the current role complete
if block._eor and host.name in block._role._had_task_run:
block._role._completed[host.name] = True
else:
task = block.always[state.cur_always_task]
if isinstance(task, Block) or state.always_child_state is not None:
state.always_child_state = HostState(blocks=[task])
state.always_child_state.run_state = self.ITERATING_TASKS
state.always_child_state.cur_role = state.cur_role
task = None
state.cur_always_task += 1

6
lib/ansible/playbook/block.py
Unescape Escape View File

@ -53,6 +53,9 @@ class Block(Base, Become, Conditional, Taggable):
self._use_handlers = use_handlers
self._implicit = implicit
# end of role flag
self._eor = False
if task_include:
self._parent = task_include
elif parent_block:
@ -182,6 +185,7 @@ class Block(Base, Become, Conditional, Taggable):
new_me = super(Block, self).copy()
new_me._play = self._play
new_me._use_handlers = self._use_handlers
new_me._eor = self._eor
if self._dep_chain is not None:
new_me._dep_chain = self._dep_chain[:]
@ -214,6 +218,7 @@ class Block(Base, Become, Conditional, Taggable):
data[attr] = getattr(self, attr)
data['dep_chain'] = self.get_dep_chain()
data['eor'] = self._eor
if self._role is not None:
data['role'] = self._role.serialize()
@ -241,6 +246,7 @@ class Block(Base, Become, Conditional, Taggable):
setattr(self, attr, data.get(attr))
self._dep_chain = data.get('dep_chain', None)
self._eor = data.get('eor', False)
# if there was a serialized role, unpack it too
role_data = data.get('role')

4
lib/ansible/playbook/role/__init__.py
Unescape Escape View File

@ -408,12 +408,14 @@ class Role(Base, Become, Conditional, Taggable):
dep_blocks = dep.compile(play=play, dep_chain=new_dep_chain)
block_list.extend(dep_blocks)
for task_block in self._task_blocks:
for idx, task_block in enumerate(self._task_blocks):
new_task_block = task_block.copy(exclude_parent=True)
if task_block._parent:
new_task_block._parent = task_block._parent.copy()
new_task_block._dep_chain = new_dep_chain
new_task_block._play = play
if idx == len(self._task_blocks) - 1:
new_task_block._eor = True
block_list.append(new_task_block)
return block_list

41
test/units/executor/test_play_iterator.py
Unescape Escape View File

@ -87,8 +87,22 @@ class TestPlayIterator(unittest.TestCase):
- debug: msg="this is a post_task"
""",
'/etc/ansible/roles/test_role/tasks/main.yml': """
- debug: msg="this is a role task"
- name: role task
debug: msg="this is a role task"
- block:
- name: role block task
debug: msg="inside block in role"
always:
- name: role always task
debug: msg="always task in block in role"
- include: foo.yml
- name: role task after include
debug: msg="after include in role"
""",
'/etc/ansible/roles/test_role/tasks/foo.yml': """
- name: role included task
debug: msg="this is task in an include from a role"
"""
})
mock_var_manager = MagicMock()
@ -141,6 +155,31 @@ class TestPlayIterator(unittest.TestCase):
(host_state, task) = itr.get_next_task_for_host(hosts[0])
self.assertIsNotNone(task)
self.assertEqual(task.action, 'debug')
self.assertEqual(task.name, "role task")
self.assertIsNotNone(task._role)
# role block task
(host_state, task) = itr.get_next_task_for_host(hosts[0])
self.assertIsNotNone(task)
self.assertEqual(task.action, 'debug')
self.assertEqual(task.name, "role block task")
self.assertIsNotNone(task._role)
# role block always task
(host_state, task) = itr.get_next_task_for_host(hosts[0])
self.assertIsNotNone(task)
self.assertEqual(task.action, 'debug')
self.assertEqual(task.name, "role always task")
self.assertIsNotNone(task._role)
# role include task
#(host_state, task) = itr.get_next_task_for_host(hosts[0])
#self.assertIsNotNone(task)
#self.assertEqual(task.action, 'debug')
#self.assertEqual(task.name, "role included task")
#self.assertIsNotNone(task._role)
# role task after include
(host_state, task) = itr.get_next_task_for_host(hosts[0])
self.assertIsNotNone(task)
self.assertEqual(task.action, 'debug')
self.assertEqual(task.name, "role task after include")
self.assertIsNotNone(task._role)
# regular play task
(host_state, task) = itr.get_next_task_for_host(hosts[0])

Write Preview
Loading…
Cancel
Save