summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Cammarata <jimi@sngx.net>2016-11-11 02:34:01 -0600
committerJames Cammarata <jimi@sngx.net>2016-11-11 02:34:01 -0600
commita35f6560f3e06807c9b367c851cd6f1b139c2e0d (patch)
treebeb5aee488246a179ad3214bcffea6ec4876896d
parent3f785ee1733003d79a74decfabdeb903b99a7c3a (diff)
downloadansible-issue_18206_pb_include+includes+conditionals.tar.gz
Don't copy the parent block of TaskIncludes when loading staticallyissue_18206_pb_include+includes+conditionals
When loading an include statically, we previously were simply doing a copy() of the TaskInclude object, which recurses up the parents creating a new lineage of objects. This caused problems when used inside load_list_of_blocks as the new parent Block of the new TaskInclude was not actually in the list of blocks being operated on. In most circumstances, this did not cause a problem as the new parent block was a proper copy, however when used in combination with PlaybookInclude (which copies conditionals to the list of blocks loaded) this untracked parent was not being properly updated, leading to tasks being run improperly. Fixes #18206
-rw-r--r--lib/ansible/playbook/base.py4
-rw-r--r--lib/ansible/playbook/block.py14
-rw-r--r--lib/ansible/playbook/conditional.py11
-rw-r--r--lib/ansible/playbook/helpers.py12
-rw-r--r--lib/ansible/playbook/playbook_include.py15
-rw-r--r--lib/ansible/playbook/task.py6
-rw-r--r--lib/ansible/plugins/strategy/__init__.py1
7 files changed, 27 insertions, 36 deletions
diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py
index 331ee53bd6..fe4c0fe9e7 100644
--- a/lib/ansible/playbook/base.py
+++ b/lib/ansible/playbook/base.py
@@ -197,8 +197,8 @@ class Base(with_metaclass(BaseMeta, object)):
def dump_me(self, depth=0):
if depth == 0:
- display.debug("DUMPING OBJECT ------------------------------------------------------")
- display.debug("%s- %s (%s, id=%s)" % (" " * depth, self.__class__.__name__, self, id(self)))
+ print("DUMPING OBJECT ------------------------------------------------------")
+ print("%s- %s (%s, id=%s)" % (" " * depth, self.__class__.__name__, self, id(self)))
if hasattr(self, '_parent') and self._parent:
self._parent.dump_me(depth+2)
dep_chain = self._parent.get_dep_chain()
diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py
index 875dc45755..7d6dab32f6 100644
--- a/lib/ansible/playbook/block.py
+++ b/lib/ansible/playbook/block.py
@@ -56,6 +56,9 @@ class Block(Base, Become, Conditional, Taggable):
super(Block, self).__init__()
+ def __repr__(self):
+ return "BLOCK(uuid=%s)(id=%s)(parent=%s)" % (self._uuid, id(self), self._parent)
+
def get_vars(self):
'''
Blocks do not store variables directly, however they may be a member
@@ -255,17 +258,6 @@ class Block(Base, Become, Conditional, Taggable):
self._parent = p
self._dep_chain = self._parent.get_dep_chain()
- def evaluate_conditional(self, templar, all_vars):
- dep_chain = self.get_dep_chain()
- if dep_chain:
- for dep in dep_chain:
- if not dep.evaluate_conditional(templar, all_vars):
- return False
- if self._parent is not None:
- if not self._parent.evaluate_conditional(templar, all_vars):
- return False
- return super(Block, self).evaluate_conditional(templar, all_vars)
-
def set_loader(self, loader):
self._loader = loader
if self._parent:
diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py
index 1fb54df998..0fada8258c 100644
--- a/lib/ansible/playbook/conditional.py
+++ b/lib/ansible/playbook/conditional.py
@@ -51,6 +51,17 @@ class Conditional:
if not isinstance(value, list):
setattr(self, name, [ value ])
+ def _get_attr_when(self):
+ '''
+ Override for the 'tags' getattr fetcher, used from Base.
+ '''
+ when = self._attributes['when']
+ if when is None:
+ when = []
+ if hasattr(self, '_get_parent_attribute'):
+ when = self._get_parent_attribute('when', extend=True)
+ return when
+
def evaluate_conditional(self, templar, all_vars):
'''
Loops through the conditionals set on this object, returning
diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py
index 5b12c0fc7a..9157bde540 100644
--- a/lib/ansible/playbook/helpers.py
+++ b/lib/ansible/playbook/helpers.py
@@ -56,7 +56,7 @@ def load_list_of_blocks(ds, play, parent_block=None, role=None, task_include=Non
task_include=task_include,
use_handlers=use_handlers,
variable_manager=variable_manager,
- loader=loader
+ loader=loader,
)
# Implicit blocks are created by bare tasks listed in a play without
# an explicit block statement. If we have two implicit blocks in a row,
@@ -219,11 +219,13 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
task_list.append(t)
continue
+ ti_copy = t.copy(exclude_parent=True)
+ ti_copy._parent = block
included_blocks = load_list_of_blocks(
data,
play=play,
parent_block=None,
- task_include=t.copy(),
+ task_include=ti_copy,
role=role,
use_handlers=use_handlers,
loader=loader,
@@ -233,12 +235,12 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
# pop tags out of the include args, if they were specified there, and assign
# them to the include. If the include already had tags specified, we raise an
# error so that users know not to specify them both ways
- tags = t.vars.pop('tags', [])
+ tags = ti_copy.vars.pop('tags', [])
if isinstance(tags, string_types):
tags = tags.split(',')
if len(tags) > 0:
- if len(t.tags) > 0:
+ if len(ti_copy.tags) > 0:
raise AnsibleParserError(
"Include tasks should not specify tags in more than one way (both via args and directly on the task). " \
"Mixing styles in which tags are specified is prohibited for whole import hierarchy, not only for single import statement",
@@ -247,7 +249,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
)
display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option")
else:
- tags = t.tags[:]
+ tags = ti_copy.tags[:]
# now we extend the tags on each of the included blocks
for b in included_blocks:
diff --git a/lib/ansible/playbook/playbook_include.py b/lib/ansible/playbook/playbook_include.py
index 9cac3317c2..139ea2048e 100644
--- a/lib/ansible/playbook/playbook_include.py
+++ b/lib/ansible/playbook/playbook_include.py
@@ -61,15 +61,6 @@ class PlaybookInclude(Base, Conditional, Taggable):
templar = Templar(loader=loader, variables=all_vars)
- try:
- forward_conditional = False
- if not new_obj.evaluate_conditional(templar=templar, all_vars=all_vars):
- return None
- except AnsibleError:
- # conditional evaluation raised an error, so we set a flag to indicate
- # we need to forward the conditionals on to the included play(s)
- forward_conditional = True
-
# then we use the object to load a Playbook
pb = Playbook(loader=loader)
@@ -95,9 +86,9 @@ class PlaybookInclude(Base, Conditional, Taggable):
# Check to see if we need to forward the conditionals on to the included
# plays. If so, we can take a shortcut here and simply prepend them to
# those attached to each block (if any)
- if forward_conditional:
- for task_block in entry.pre_tasks + entry.roles + entry.tasks + entry.post_tasks:
- task_block.when = self.when[:] + task_block.when
+ if new_obj.when:
+ for task_block in (entry.pre_tasks + entry.roles + entry.tasks + entry.post_tasks):
+ task_block._attributes['when'] = new_obj.when[:] + task_block.when[:]
return pb
diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py
index 2d98f75046..275a59f9e8 100644
--- a/lib/ansible/playbook/task.py
+++ b/lib/ansible/playbook/task.py
@@ -376,12 +376,6 @@ class Task(Base, Conditional, Taggable, Become):
super(Task, self).deserialize(data)
- def evaluate_conditional(self, templar, all_vars):
- if self._parent is not None:
- if not self._parent.evaluate_conditional(templar, all_vars):
- return False
- return super(Task, self).evaluate_conditional(templar, all_vars)
-
def set_loader(self, loader):
'''
Sets the loader on this object and recursively on parent, child objects.
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index 53d90a0263..381e60af78 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -654,6 +654,7 @@ class StrategyBase:
obj=included_file._task._ds)
display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option")
included_file._task.tags = tags
+
ti_copy.vars = temp_vars
block_list = load_list_of_blocks(