From 44d4dede92353e060a43a73b8e8bc08b2e1e9f68 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Fri, 23 Mar 2012 21:03:25 -0400 Subject: [PATCH] Split conditional imports in playbook into subfunction, fix small bug in event reporting on playbook actions. --- lib/ansible/playbook.py | 90 ++++++++++++++++++++++------------------- test/playbook1.events | 11 +---- 2 files changed, 49 insertions(+), 52 deletions(-) diff --git a/lib/ansible/playbook.py b/lib/ansible/playbook.py index d52ef76b3b4..5b994d7ac82 100755 --- a/lib/ansible/playbook.py +++ b/lib/ansible/playbook.py @@ -220,9 +220,9 @@ class PlayBook(object): elif 'skipped' in host_result: self.skipped[host] = self.skipped.get(host, 0) + 1 self.callbacks.on_skipped(host) - if poll: + elif poll: continue - if not setup and ('changed' in host_result): + elif not setup and ('changed' in host_result): self.invocations[host] = self.invocations.get(host, 0) + 1 self.changed[host] = self.changed.get(host, 0) + 1 self.callbacks.on_ok(host, host_result) @@ -402,55 +402,61 @@ class PlayBook(object): # ***************************************************** + def _do_conditional_imports(self, vars_files, host_list): + ''' handle the vars_files section, which can contain variables ''' + + # FIXME: save parsed variable results in memory to avoid excessive re-reading/parsing + # FIXME: currently parses imports for hosts not in the pattern, that is not wrong, but it's + # not super optimized yet either, because we wouldn't have hit them, ergo + # it will raise false errors if there is no defaults variable file without any $vars + # in it, which could happen on uncontacted hosts. + + if type(vars_files) != list: + raise errors.AnsibleError("vars_files must be a list") + for host in host_list: + cache_vars = SETUP_CACHE.get(host,{}) + SETUP_CACHE[host] = {} + for filename in vars_files: + if type(filename) == list: + # loop over all filenames, loading the first one, and failing if # none found + found = False + sequence = [] + for real_filename in filename: + filename2 = utils.path_dwim(self.basedir, utils.template(real_filename, cache_vars)) + sequence.append(filename2) + if os.path.exists(filename2): + found = True + data = utils.parse_yaml_from_file(filename2) + SETUP_CACHE[host].update(data) + self.callbacks.on_import_for_host(host, filename2) + break + else: + self.callbacks.on_not_import_for_host(host, filename2) + if not found: + raise errors.AnsibleError( + "%s: FATAL, no files matched for vars_files import sequence: %s" % (host, sequence) + ) + + else: + filename2 = utils.path_dwim(self.basedir, utils.template(filename, cache_vars)) + if not os.path.exists(filename2): + raise errors.AnsibleError("no file matched for vars_file import: %s" % filename2) + data = utils.parse_yaml_from_file(filename2) + SETUP_CACHE[host].update(data) + self.callbacks.on_import_for_host(host, filename2) + + # ***************************************************** + def _do_setup_step(self, pattern, vars, user, host_list, vars_files=None): ''' push variables down to the systems and get variables+facts back up ''' # this enables conditional includes like $facter_os.yml and is only done # after the original pass when we have that data. # - # FIXME: refactor into subfunction - # FIXME: save parsed variable results in memory to avoid excessive re-reading/parsing - # FIXME: currently parses imports for hosts not in the pattern, that is not wrong, but it's - # not super optimized yet either, because we wouldn't have hit them, ergo - # it will raise false errors if there is no defaults variable file without any $vars - # in it, which could happen on uncontacted hosts. if vars_files is not None: - if type(vars_files) != list: - raise errors.AnsibleError("vars_files must be a list") self.callbacks.on_setup_secondary() - for host in host_list: - cache_vars = SETUP_CACHE.get(host,{}) - SETUP_CACHE[host] = {} - for filename in vars_files: - if type(filename) == list: - # loop over all filenames, loading the first one, and failing if - # none found - found = False - sequence = [] - for real_filename in filename: - filename2 = utils.path_dwim(self.basedir, utils.template(real_filename, cache_vars)) - sequence.append(filename2) - if os.path.exists(filename2): - found = True - data = utils.parse_yaml_from_file(filename2) - SETUP_CACHE[host].update(data) - self.callbacks.on_import_for_host(host, filename2) - break - else: - self.callbacks.on_not_import_for_host(host, filename2) - if not found: - raise errors.AnsibleError( - "%s: FATAL, no files matched for vars_files import sequence: %s" % (host, sequence) - ) - - else: - filename2 = utils.path_dwim(self.basedir, utils.template(filename, cache_vars)) - if not os.path.exists(filename2): - raise errors.AnsibleError("no file matched for vars_file import: %s" % filename2) - data = utils.parse_yaml_from_file(filename2) - SETUP_CACHE[host].update(data) - self.callbacks.on_import_for_host(host, filename2) + self._do_conditional_imports(vars_files, host_list) else: self.callbacks.on_setup_primary() diff --git a/test/playbook1.events b/test/playbook1.events index 05074d77a39..d4dd49dfbd8 100644 --- a/test/playbook1.events +++ b/test/playbook1.events @@ -223,15 +223,6 @@ "127.0.0.1" ] ], - [ - "ok", - [ - "127.0.0.1", - { - "skipped": true - } - ] - ], [ "task start", [ @@ -276,7 +267,7 @@ "changed": 2, "dark": 0, "failed": 0, - "resources": 12, + "resources": 11, "skipped": 1 } }