Much streamlining around setup steps in playbooks, now only run setup once per play.

CHANGELOG.md

@@ -20,6 +20,7 @@ Ansible Changes By Release
 * modules now consistently all take yes/no for boolean parameters (some accepted true/false)
 * in YAML inventory, hosts can list their groups in inverted order now also (see tests/yaml_hosts)
 * setup module no longer saves to disk, template module now only used in playbooks
+* setup module no longer needs to run twice per playbook
 
 0.5 "Amsterdam" ------- July 04, 2012
 

examples/playbooks/templates/foo.j2

@@ -1,4 +1,10 @@
 # This is a very simple Jinja2 template representing an imaginary configuration file
 # for an imaginary app.
 
+# this is an example of loading a fact from the setup module
+system={{ ansible_system }}
+
+# here is a variable that could be set in a playbook or inventory file
 http_port={{ http_port }}
+
+

lib/ansible/callbacks.py

@@ -247,11 +247,8 @@ class PlaybookCallbacks(object):
             return getpass.getpass(msg)
         return raw_input(msg)
         
-    def on_setup_primary(self):
-        print banner("SETUP PHASE")
-    
-    def on_setup_secondary(self):
-        print banner("VARIABLE IMPORT PHASE")
+    def on_setup(self):
+        print banner("GATHERING FACTS")
     
     def on_import_for_host(self, host, imported_file):
         print "%s: importing %s" % (host, imported_file)

lib/ansible/playbook/__init__.py

@@ -270,19 +270,13 @@ class PlayBook(object):
 
     # *****************************************************
 
-    def _do_setup_step(self, play, vars_files=None):
+    def _do_setup_step(self, play):
 
-        ''' push variables down to the systems and get variables+facts back up '''
+        ''' get facts from the remote system '''
 
-        # this enables conditional includes like $facter_os.yml and is only done
-        # after the original pass when we have that data.
-        #
+        setup_args = {}
 
-        if vars_files is not None:
-            self.callbacks.on_setup_secondary()
-            play.update_vars_files(self.inventory.list_hosts(play.hosts))
-        else:
-            self.callbacks.on_setup_primary()
+        self.callbacks.on_setup()
 
         host_list = [ h for h in self.inventory.list_hosts(play.hosts) 
             if not (h in self.stats.failures or h in self.stats.dark) ]
@@ -291,7 +285,7 @@ class PlayBook(object):
 
         # push any variables down to the system
         setup_results = ansible.runner.Runner(
-            pattern=play.hosts, module_name='setup', module_args=play.vars, inventory=self.inventory,
+            pattern=play.hosts, module_name='setup', module_args=setup_args, inventory=self.inventory,
             forks=self.forks, module_path=self.module_path, timeout=self.timeout, remote_user=play.remote_user,
             remote_pass=self.remote_pass, remote_port=play.remote_port, private_key_file=self.private_key_file,
             setup_cache=self.SETUP_CACHE, callbacks=self.runner_callbacks, sudo=play.sudo, sudo_user=play.sudo_user, 
@@ -304,11 +298,9 @@ class PlayBook(object):
         # now for each result, load into the setup cache so we can
         # let runner template out future commands
         setup_ok = setup_results.get('contacted', {})
-        if vars_files is None:
-            # first pass only or we'll erase good work
-            for (host, result) in setup_ok.iteritems():
-                if 'ansible_facts' in result:
-                    self.SETUP_CACHE[host] = result['ansible_facts']
+        for (host, result) in setup_ok.iteritems():
+            if 'ansible_facts' in result:
+                self.SETUP_CACHE[host] = result['ansible_facts']
         return setup_results
 
     # *****************************************************
@@ -321,12 +313,12 @@ class PlayBook(object):
 
         self.callbacks.on_play_start(play.name)
 
-        # push any variables down to the system # and get facts/ohai/other data back up
-        rc = self._do_setup_step(play) # pattern, vars, user, port, sudo, sudo_user, transport, None)
+        # get facts from system
+        rc = self._do_setup_step(play) 
 
         # now with that data, handle contentional variable file imports!
         if play.vars_files and len(play.vars_files) > 0:
-            rc = self._do_setup_step(play, play.vars_files)
+            play.update_vars_files(self.inventory.list_hosts(play.hosts))
 
         for task in play.tasks():
             

test/TestPlayBook.py

@@ -30,12 +30,9 @@ class TestCallbacks(object):
     def on_start(self):
         EVENTS.append('start')
 
-    def on_setup_primary(self):
+    def on_setup(self):
         EVENTS.append([ 'primary_setup' ])
  
-    def on_setup_secondary(self):
-        EVENTS.append([ 'secondary_setup' ])
-
     def on_skipped(self, host):
         EVENTS.append([ 'skipped', [ host ]])
 
@@ -86,10 +83,7 @@ class TestCallbacks(object):
     def on_unreachable(self, host, msg):
         EVENTS.append([ 'failed/dark', [ host, msg ]])
 
-    def on_setup_primary(self):
-        pass
-    
-    def on_setup_secondary(self):
+    def on_setup(self):
         pass
     
     def on_no_hosts(self):
@@ -158,7 +152,7 @@ class TestPlaybook(unittest.TestCase):
             "127.0.0.2": {
                 "changed": 9, 
                 "failures": 0, 
-                "ok": 12, 
+                "ok": 11, 
                 "skipped": 1, 
                 "unreachable": 0
             }

test/TestRunner.py

@@ -194,14 +194,13 @@ class TestRunner(unittest.TestCase):
     def test_service(self):
         # TODO: tests for the service module
         pass
+
     def test_assemble(self):
         input = self._get_test_file('assemble.d')
-        metadata = self._get_test_file('metadata.json')
         output = self._get_stage_file('sample.out')
         result = self._run('assemble', [
             "src=%s" % input,
             "dest=%s" % output,
-            "metadata=%s" % metadata
         ])
         assert os.path.exists(output)
         out = file(output).read()
@@ -214,7 +213,6 @@ class TestRunner(unittest.TestCase):
         result = self._run('assemble', [
             "src=%s" % input,
             "dest=%s" % output,
-            "metadata=%s" % metadata
         ])
         assert result['changed'] == False