diff --git a/lib/ansible/playbook/__init__.py b/lib/ansible/playbook/__init__.py index da6357bf23a..e2e2aea3e5b 100644 --- a/lib/ansible/playbook/__init__.py +++ b/lib/ansible/playbook/__init__.py @@ -396,6 +396,7 @@ class PlayBook(object): remote_port=task.play.remote_port, module_vars=task.module_vars, default_vars=task.default_vars, + extra_vars=self.extra_vars, private_key_file=self.private_key_file, setup_cache=self.SETUP_CACHE, vars_cache=self.VARS_CACHE, diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 0e5ff7d3997..5187ef07ee4 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -89,15 +89,15 @@ class Play(object): self.vars_files = ds.get('vars_files', []) if not isinstance(self.vars_files, list): raise errors.AnsibleError('vars_files must be a list') - self._update_vars_files_for_host(None) + processed_vars_files = self._update_vars_files_for_host(None) # now we load the roles into the datastructure self.included_roles = [] ds = self._load_roles(self.roles, ds) - # and finally re-process the vars files as they may have - # been updated by the included roles - self.vars_files = ds.get('vars_files', []) + # and finally re-process the vars files as they may have been updated + # by the included roles, but exclude any which have been processed + self.vars_files = utils.list_difference(ds.get('vars_files', []), processed_vars_files) if not isinstance(self.vars_files, list): raise errors.AnsibleError('vars_files must be a list') @@ -765,27 +765,37 @@ class Play(object): """ Render the raw filename into 3 forms """ + # filename2 is the templated version of the filename, which will + # be fully rendered if any variables contained within it are + # non-inventory related filename2 = template(self.basedir, filename, self.vars) + + # filename3 is the same as filename2, but when the host object is + # available, inventory variables will be expanded as well since the + # name is templated with the injected variables filename3 = filename2 if host is not None: filename3 = template(self.basedir, filename2, inject) + + # filename4 is the dwim'd path, but may also be mixed-scope, so we use + # both play scoped vars and host scoped vars to template the filepath if self._has_vars_in(filename3) and host is not None: - # allow play scoped vars and host scoped vars to template the filepath inject.update(self.vars) filename4 = template(self.basedir, filename3, inject) filename4 = utils.path_dwim(self.basedir, filename4) else: filename4 = utils.path_dwim(self.basedir, filename3) + return filename2, filename3, filename4 - def update_vars_cache(host, inject, data, filename): + def update_vars_cache(host, data, target_filename=None): """ update a host's varscache with new var data """ - data = utils.combine_vars(inject, data) self.playbook.VARS_CACHE[host] = utils.combine_vars(self.playbook.VARS_CACHE.get(host, {}), data) - self.playbook.callbacks.on_import_for_host(host, filename4) + if target_filename: + self.playbook.callbacks.on_import_for_host(host, target_filename) def process_files(filename, filename2, filename3, filename4, host=None): @@ -796,21 +806,19 @@ class Play(object): if type(data) != dict: raise errors.AnsibleError("%s must be stored as a dictionary/hash" % filename4) if host is not None: - if self._has_vars_in(filename2) and not self._has_vars_in(filename3): - # running a host specific pass and has host specific variables - # load into setup cache - update_vars_cache(host, inject, data, filename4) - elif self._has_vars_in(filename3) and not self._has_vars_in(filename4): - # handle mixed scope variables in filepath - update_vars_cache(host, inject, data, filename4) - - elif not self._has_vars_in(filename4): - # found a non-host specific variable, load into vars and NOT - # the setup cache - if host is not None: - self.vars.update(data) - else: - self.vars = utils.combine_vars(self.vars, data) + target_filename = None + if self._has_vars_in(filename2): + if not self._has_vars_in(filename3): + target_filename = filename3 + else: + target_filename = filename4 + update_vars_cache(host, data, target_filename=target_filename) + else: + self.vars = utils.combine_vars(self.vars, data) + # we did process this file + return True + # we did not process this file + return False # Enforce that vars_files is always a list if type(self.vars_files) != list: @@ -825,6 +833,7 @@ class Play(object): else: inject = None + processed = [] for filename in self.vars_files: if type(filename) == list: # loop over all filenames, loading the first one, and failing if none found @@ -835,7 +844,8 @@ class Play(object): sequence.append(filename4) if os.path.exists(filename4): found = True - process_files(filename, filename2, filename3, filename4, host=host) + if process_files(filename, filename2, filename3, filename4, host=host): + processed.append(filename) elif host is not None: self.playbook.callbacks.on_not_import_for_host(host, filename4) if found: @@ -844,14 +854,12 @@ class Play(object): raise errors.AnsibleError( "%s: FATAL, no files matched for vars_files import sequence: %s" % (host, sequence) ) - else: # just one filename supplied, load it! filename2, filename3, filename4 = generate_filenames(host, inject, filename) if self._has_vars_in(filename4): continue - process_files(filename, filename2, filename3, filename4, host=host) + if process_files(filename, filename2, filename3, filename4, host=host): + processed.append(filename) - # finally, update the VARS_CACHE for the host, if it is set - if host is not None: - self.playbook.VARS_CACHE.setdefault(host, {}).update(self.playbook.extra_vars) + return processed diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 76a5d76d472..a63bf426533 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -130,6 +130,7 @@ class Runner(object): sudo_user=C.DEFAULT_SUDO_USER, # ex: 'root' module_vars=None, # a playbooks internals thing default_vars=None, # ditto + extra_vars=None, # extra vars specified with he playbook(s) is_playbook=False, # running from playbook or not? inventory=None, # reference to Inventory object subset=None, # subset pattern @@ -170,6 +171,8 @@ class Runner(object): self.module_vars = utils.default(module_vars, lambda: {}) self.default_vars = utils.default(default_vars, lambda: {}) + self.extra_vars = utils.default(extra_vars, lambda: {}) + self.always_run = None self.connector = connection.Connector(self) self.conditional = conditional @@ -605,10 +608,13 @@ class Runner(object): module_vars = template.template(self.basedir, self.module_vars, module_vars_inject) inject = {} + inject = utils.combine_vars(inject, self.default_vars) inject = utils.combine_vars(inject, host_variables) + inject = utils.combine_vars(inject, self.setup_cache.get(host, {})) inject = utils.combine_vars(inject, module_vars) - inject = utils.combine_vars(inject, combined_cache.get(host, {})) + inject = utils.combine_vars(inject, self.vars_cache.get(host, {})) + inject = utils.combine_vars(inject, self.extra_vars) inject.setdefault('ansible_ssh_user', self.remote_user) inject['hostvars'] = hostvars inject['group_names'] = host_variables.get('group_names', []) diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index fbceb937409..c18174b00d2 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -1163,6 +1163,16 @@ def list_intersection(a, b): result.append(x) return result +def list_difference(a, b): + result = [] + for x in a: + if x not in b and x not in result: + result.append(x) + for x in b: + if x not in a and x not in result: + result.append(x) + return result + def safe_eval(expr, locals={}, include_exceptions=False): ''' This is intended for allowing things like: diff --git a/test/integration/roles/test_var_precedence/tasks/main.yml b/test/integration/roles/test_var_precedence/tasks/main.yml index 27972675052..1915ebdb916 100644 --- a/test/integration/roles/test_var_precedence/tasks/main.yml +++ b/test/integration/roles/test_var_precedence/tasks/main.yml @@ -3,3 +3,4 @@ - 'extra_var == "extra_var"' - 'vars_var == "vars_var"' - 'vars_files_var == "vars_files_var"' + - 'vars_files_var_role == "vars_files_var_role3"' diff --git a/test/integration/roles/test_var_precedence_role1/tasks/main.yml b/test/integration/roles/test_var_precedence_role1/tasks/main.yml index a0db785a871..410e01b5704 100644 --- a/test/integration/roles/test_var_precedence_role1/tasks/main.yml +++ b/test/integration/roles/test_var_precedence_role1/tasks/main.yml @@ -2,6 +2,7 @@ - debug: var=param_var - debug: var=vars_var - debug: var=vars_files_var +- debug: var=vars_files_var_role - debug: var=defaults_file_var_role1 - assert: that: @@ -9,4 +10,5 @@ - 'param_var == "param_var_role1"' - 'vars_var == "vars_var"' - 'vars_files_var == "vars_files_var"' + - 'vars_files_var_role == "vars_files_var_role3"' - 'defaults_file_var_role1 == "defaults_file_var_role1"' diff --git a/test/integration/roles/test_var_precedence_role1/vars/main.yml b/test/integration/roles/test_var_precedence_role1/vars/main.yml index 0adb2faed4c..2f7613d30a6 100644 --- a/test/integration/roles/test_var_precedence_role1/vars/main.yml +++ b/test/integration/roles/test_var_precedence_role1/vars/main.yml @@ -1,3 +1,4 @@ --- # should override the global vars_files_var since it's local to the role -vars_files_var: "vars_files_var_role1" +# but will be set to the value in the last role included which defines it +vars_files_var_role: "vars_files_var_role1" diff --git a/test/integration/roles/test_var_precedence_role2/tasks/main.yml b/test/integration/roles/test_var_precedence_role2/tasks/main.yml index f38d63b3ee9..96551a8e9c7 100644 --- a/test/integration/roles/test_var_precedence_role2/tasks/main.yml +++ b/test/integration/roles/test_var_precedence_role2/tasks/main.yml @@ -2,6 +2,7 @@ - debug: var=param_var - debug: var=vars_var - debug: var=vars_files_var +- debug: var=vars_files_var_role - debug: var=defaults_file_var_role1 - assert: that: @@ -9,4 +10,5 @@ - 'param_var == "param_var_role2"' - 'vars_var == "vars_var"' - 'vars_files_var == "vars_files_var"' + - 'vars_files_var_role == "vars_files_var_role3"' - 'defaults_file_var_role2 == "overridden by role vars"' diff --git a/test/integration/roles/test_var_precedence_role2/vars/main.yml b/test/integration/roles/test_var_precedence_role2/vars/main.yml index f079e4d01d0..483c5ea2453 100644 --- a/test/integration/roles/test_var_precedence_role2/vars/main.yml +++ b/test/integration/roles/test_var_precedence_role2/vars/main.yml @@ -1,5 +1,5 @@ --- # should override the global vars_files_var since it's local to the role -vars_files_var: "vars_files_var_role1" +vars_files_var_role: "vars_files_var_role2" # should override the value in defaults/main.yml for role 2 defaults_file_var_role2: "overridden by role vars" diff --git a/test/integration/roles/test_var_precedence_role3/tasks/main.yml b/test/integration/roles/test_var_precedence_role3/tasks/main.yml index f07f329fd02..12346ecdc8f 100644 --- a/test/integration/roles/test_var_precedence_role3/tasks/main.yml +++ b/test/integration/roles/test_var_precedence_role3/tasks/main.yml @@ -2,6 +2,7 @@ - debug: var=param_var - debug: var=vars_var - debug: var=vars_files_var +- debug: var=vars_files_var_role - debug: var=defaults_file_var_role1 - assert: that: @@ -9,4 +10,5 @@ - 'param_var == "param_var_role3"' - 'vars_var == "vars_var"' - 'vars_files_var == "vars_files_var"' + - 'vars_files_var_role == "vars_files_var_role3"' - 'defaults_file_var_role3 == "overridden from inventory"' diff --git a/test/integration/roles/test_var_precedence_role3/vars/main.yml b/test/integration/roles/test_var_precedence_role3/vars/main.yml index 0adb2faed4c..3cfb1b1c686 100644 --- a/test/integration/roles/test_var_precedence_role3/vars/main.yml +++ b/test/integration/roles/test_var_precedence_role3/vars/main.yml @@ -1,3 +1,3 @@ --- # should override the global vars_files_var since it's local to the role -vars_files_var: "vars_files_var_role1" +vars_files_var_role: "vars_files_var_role3" diff --git a/test/integration/test_var_precedence.yml b/test/integration/test_var_precedence.yml index cacbe71c90c..301f6cdb311 100644 --- a/test/integration/test_var_precedence.yml +++ b/test/integration/test_var_precedence.yml @@ -12,8 +12,10 @@ - debug: var=extra_var - debug: var=vars_var - debug: var=vars_files_var + - debug: var=vars_files_var_role - assert: that: - 'extra_var == "extra_var"' - 'vars_var == "vars_var"' - 'vars_files_var == "vars_files_var"' + - 'vars_files_var_role == "vars_files_var_role3"' diff --git a/test/integration/vars/test_var_precedence.yml b/test/integration/vars/test_var_precedence.yml index 3f43f1031de..19d65cba3e9 100644 --- a/test/integration/vars/test_var_precedence.yml +++ b/test/integration/vars/test_var_precedence.yml @@ -2,3 +2,4 @@ extra_var: "BAD!" role_var: "BAD!" vars_files_var: "vars_files_var" +vars_files_var_role: "should be overridden by roles" diff --git a/test/units/TestPlayVarsFiles.py b/test/units/TestPlayVarsFiles.py index 94fada6f7ed..d1b1f9dfa22 100644 --- a/test/units/TestPlayVarsFiles.py +++ b/test/units/TestPlayVarsFiles.py @@ -266,37 +266,6 @@ class TestMe(unittest.TestCase): assert 'foo' in play.playbook.VARS_CACHE['localhost'], "vars_file vars were not loaded into vars_cache" assert play.playbook.VARS_CACHE['localhost']['foo'] == 'bar', "foo does not equal bar" - def test_vars_files_for_host_with_extra_vars(self): - - # host != None - # vars in filename2 - # no vars in filename3 - - # make a vars file - fd, temp_path = mkstemp() - f = open(temp_path, "wb") - f.write("foo: bar\n") - f.close() - - # build play attributes - playbook = FakePlayBook() - ds = { "hosts": "localhost", - "vars_files": ["{{ temp_path }}"]} - basedir = "." - playbook.VARS_CACHE['localhost']['temp_path'] = temp_path - playbook.extra_vars = {"foo": "extra"} - - # create play and do first run - play = Play(playbook, ds, basedir) - - # the second run is started by calling update_vars_files - play.update_vars_files(['localhost']) - os.remove(temp_path) - - assert 'foo' in play.vars, "extra vars were not set in play.vars" - assert 'foo' in play.playbook.VARS_CACHE['localhost'], "vars_file vars were not loaded into vars_cache" - assert play.playbook.VARS_CACHE['localhost']['foo'] == 'extra', "extra vars did not overwrite vars_files vars" - ######################################## # COMPLEX FILENAME TEMPLATING TESTS