From 6826aa7360ed76d84a9c60eb8f2fb1d46e6ac964 Mon Sep 17 00:00:00 2001 From: George Miroshnykov Date: Sun, 10 Mar 2013 01:30:18 +0200 Subject: [PATCH] Tweaked merge_hash to also affect Runner behavior --- .../inventory/vars_plugins/group_vars.py | 12 +--- lib/ansible/playbook/play.py | 5 +- lib/ansible/runner/__init__.py | 18 ++--- lib/ansible/utils/__init__.py | 9 ++- test/TestPlayBook.py | 69 +++++++++++++++++++ test/test_hash_behavior/goodbye.yml | 3 + test/test_hash_behavior/hello.yml | 3 + test/test_hash_behavior/message.j2 | 3 + test/test_hash_behavior/playbook.yml | 11 +++ 9 files changed, 110 insertions(+), 23 deletions(-) create mode 100644 test/test_hash_behavior/goodbye.yml create mode 100644 test/test_hash_behavior/hello.yml create mode 100644 test/test_hash_behavior/message.j2 create mode 100644 test/test_hash_behavior/playbook.yml diff --git a/lib/ansible/inventory/vars_plugins/group_vars.py b/lib/ansible/inventory/vars_plugins/group_vars.py index 49d59ba6705..6801c2da7fb 100644 --- a/lib/ansible/inventory/vars_plugins/group_vars.py +++ b/lib/ansible/inventory/vars_plugins/group_vars.py @@ -49,11 +49,7 @@ class VarsModule(object): data = utils.parse_yaml_from_file(path) if type(data) != dict: raise errors.AnsibleError("%s must be stored as a dictionary/hash" % path) - if C.DEFAULT_HASH_BEHAVIOUR == "merge": - # let data content override results if needed - results = utils.merge_hash(results, data) - else: - results.update(data) + results = utils.combine_vars(results, data); # load vars in inventory_dir/hosts_vars/name_of_host path = os.path.join(basedir, "host_vars/%s" % host.name) @@ -61,10 +57,6 @@ class VarsModule(object): data = utils.parse_yaml_from_file(path) if type(data) != dict: raise errors.AnsibleError("%s must be stored as a dictionary/hash" % path) - if C.DEFAULT_HASH_BEHAVIOUR == "merge": - # let data content override results if needed - results = utils.merge_hash(results, data) - else: - results.update(data) + results = utils.combine_vars(results, data); return results diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 3a8270d382e..60b90869f30 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -330,8 +330,9 @@ class Play(object): if host is not None and 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 - self.playbook.SETUP_CACHE[host].update(new_vars) + self.playbook.SETUP_CACHE[host] = utils.combine_vars( + self.playbook.SETUP_CACHE[host], new_vars) self.playbook.callbacks.on_import_for_host(host, filename4) elif host is None: # running a non-host specific pass and we can update the global vars instead - self.vars.update(new_vars) + self.vars = utils.combine_vars(self.vars, new_vars) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index 25b8982da98..99735356af6 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -175,7 +175,7 @@ class Runner(object): # ensure we are using unique tmp paths random.seed() - + # ***************************************************** def _complex_args_hack(self, complex_args, module_args): @@ -333,9 +333,9 @@ class Runner(object): port = self.remote_port inject = {} - inject.update(host_variables) - inject.update(self.module_vars) - inject.update(self.setup_cache[host]) + inject = utils.combine_vars(inject, host_variables) + inject = utils.combine_vars(inject, self.module_vars) + inject = utils.combine_vars(inject, self.setup_cache[host]) inject['hostvars'] = HostVars(self.setup_cache, self.inventory) inject['group_names'] = host_variables.get('group_names', []) inject['groups'] = self.inventory.groups_list() @@ -492,7 +492,7 @@ class Runner(object): # all modules get a tempdir, action plugins get one unless they have NEEDS_TMPPATH set to False if getattr(handler, 'NEEDS_TMPPATH', True): tmp = self._make_tmp_path(conn) - + result = handler.run(conn, tmp, module_name, module_args, inject, complex_args) conn.close() @@ -625,8 +625,8 @@ class Runner(object): module_data = f.read() if module_common.REPLACER in module_data: is_new_style=True - - complex_args_json = utils.jsonify(complex_args) + + complex_args_json = utils.jsonify(complex_args) encoded_args = "\"\"\"%s\"\"\"" % module_args.replace("\"","\\\"") encoded_lang = "\"\"\"%s\"\"\"" % C.DEFAULT_MODULE_LANG encoded_complex = "\"\"\"%s\"\"\"" % complex_args_json.replace("\\", "\\\\") @@ -635,7 +635,7 @@ class Runner(object): module_data = module_data.replace(module_common.REPLACER_ARGS, encoded_args) module_data = module_data.replace(module_common.REPLACER_LANG, encoded_lang) module_data = module_data.replace(module_common.REPLACER_COMPLEX, encoded_complex) - + if is_new_style: facility = C.DEFAULT_SYSLOG_FACILITY if 'ansible_syslog_facility' in inject: @@ -737,7 +737,7 @@ class Runner(object): # run once per hostgroup, rather than pausing once per each # host. p = utils.plugins.action_loader.get(self.module_name, self) - + if p and getattr(p, 'BYPASS_HOST_LOOP', None): # Expose the current hostgroup to the bypassing plugins diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index 9f66b3f5383..a13f9a1ab7c 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -306,7 +306,7 @@ def merge_hash(a, b): for k, v in b.iteritems(): if k in a and isinstance(a[k], dict): # if this key is a hash and exists in a - # we recursively call ourselves with + # we recursively call ourselves with # the key value of b a[k] = merge_hash(a[k], v) else: @@ -663,8 +663,13 @@ def get_diff(diff): return ">> the files are different, but the diff library cannot compare unicode strings" def is_list_of_strings(items): - for x in items: + for x in items: if not isinstance(x, basestring): return False return True +def combine_vars(a, b): + if C.DEFAULT_HASH_BEHAVIOUR == "merge": + return merge_hash(a, b) + else: + return dict(a.items() + b.items()) diff --git a/test/TestPlayBook.py b/test/TestPlayBook.py index 0445158b167..c0a88957839 100644 --- a/test/TestPlayBook.py +++ b/test/TestPlayBook.py @@ -10,6 +10,7 @@ import ansible.utils as utils import ansible.callbacks as ans_callbacks import os import shutil +import ansible.constants as C EVENTS = [] @@ -93,6 +94,8 @@ class TestPlaybook(unittest.TestCase): os.unlink('/tmp/ansible_test_data_copy.out') if os.path.exists('/tmp/ansible_test_data_template.out'): os.unlink('/tmp/ansible_test_data_template.out') + if os.path.exists('/tmp/ansible_test_messages.out'): + os.unlink('/tmp/ansible_test_messages.out') def _prepare_stage_dir(self): stage_path = os.path.join(self.test_dir, 'test_data') @@ -236,3 +239,69 @@ class TestPlaybook(unittest.TestCase): play = ansible.playbook.Play(playbook, playbook.playbook[0], os.getcwd()) assert play.hosts == ';'.join(('host1', 'host2', 'host3')) + def test_playbook_hash_replace(self): + # save default hash behavior so we can restore it in the end of the test + saved_hash_behavior = C.DEFAULT_HASH_BEHAVIOUR + C.DEFAULT_HASH_BEHAVIOUR = "replace" + + test_callbacks = TestCallbacks() + playbook = ansible.playbook.PlayBook( + playbook=os.path.join(self.test_dir, 'test_hash_behavior', 'playbook.yml'), + host_list='test/ansible_hosts', + stats=ans_callbacks.AggregateStats(), + callbacks=test_callbacks, + runner_callbacks=test_callbacks + ) + playbook.run() + + with open('/tmp/ansible_test_messages.out') as f: + actual = [l.strip() for l in f.readlines()] + + print "**ACTUAL**" + print actual + + expected = [ + "goodbye: Goodbye World!" + ] + + print "**EXPECTED**" + print expected + + assert actual == expected + + # restore default hash behavior + C.DEFAULT_HASH_BEHAVIOUR = saved_hash_behavior + + def test_playbook_hash_merge(self): + # save default hash behavior so we can restore it in the end of the test + saved_hash_behavior = C.DEFAULT_HASH_BEHAVIOUR + C.DEFAULT_HASH_BEHAVIOUR = "merge" + + test_callbacks = TestCallbacks() + playbook = ansible.playbook.PlayBook( + playbook=os.path.join(self.test_dir, 'test_hash_behavior', 'playbook.yml'), + host_list='test/ansible_hosts', + stats=ans_callbacks.AggregateStats(), + callbacks=test_callbacks, + runner_callbacks=test_callbacks + ) + playbook.run() + + with open('/tmp/ansible_test_messages.out') as f: + actual = [l.strip() for l in f.readlines()] + + print "**ACTUAL**" + print actual + + expected = [ + "hello: Hello World!", + "goodbye: Goodbye World!" + ] + + print "**EXPECTED**" + print expected + + assert actual == expected + + # restore default hash behavior + C.DEFAULT_HASH_BEHAVIOUR = saved_hash_behavior diff --git a/test/test_hash_behavior/goodbye.yml b/test/test_hash_behavior/goodbye.yml new file mode 100644 index 00000000000..d04fd368e82 --- /dev/null +++ b/test/test_hash_behavior/goodbye.yml @@ -0,0 +1,3 @@ +--- +messages: + goodbye: "Goodbye World!" diff --git a/test/test_hash_behavior/hello.yml b/test/test_hash_behavior/hello.yml new file mode 100644 index 00000000000..a50c0883530 --- /dev/null +++ b/test/test_hash_behavior/hello.yml @@ -0,0 +1,3 @@ +--- +messages: + hello: "Hello World!" diff --git a/test/test_hash_behavior/message.j2 b/test/test_hash_behavior/message.j2 new file mode 100644 index 00000000000..c2da7b9e715 --- /dev/null +++ b/test/test_hash_behavior/message.j2 @@ -0,0 +1,3 @@ +{% for k, v in messages.iteritems() %} +{{ k }}: {{ v }} +{% endfor %} diff --git a/test/test_hash_behavior/playbook.yml b/test/test_hash_behavior/playbook.yml new file mode 100644 index 00000000000..29c7736e970 --- /dev/null +++ b/test/test_hash_behavior/playbook.yml @@ -0,0 +1,11 @@ +--- +- hosts: all + connection: local + + vars_files: + - hello.yml + - goodbye.yml + + tasks: + - name: generate messages + action: template src=message.j2 dest=/tmp/ansible_test_messages.out