Move absent node pruning to cleanup section

Add a cleanup_state.yml playbook that is run after every deployment and
teardown. It will remove any nodes marked as 'absent' from the state
file, by calling schedule.py with `prune_only=True`.

ansible/action_plugins/tenks_update_state.py

@@ -47,6 +47,9 @@ class ActionModule(ActionBase):
             :state: A dict of existing Tenks state (as produced by a previous
                     run of this module), to be taken into account in this run.
                     Optional.
+            :prune_only: A boolean which, if set, will instruct the plugin to
+                         only remove any nodes with state='absent' from
+                         `state`.
         :returns: A dict of Tenks state for each hypervisor, keyed by the
                   hostname of the hypervisor to which the state refers.
         """
@@ -59,14 +62,25 @@ class ActionModule(ActionBase):
         self.localhost_vars = task_vars['hostvars']['localhost']
         self._validate_args()
 
-        # Modify the state as necessary.
-        self._set_physnet_idxs()
-        self._process_specs()
+        if self.args['prune_only']:
+            self._prune_absent_nodes()
+        else:
+            # Modify the state as necessary.
+            self._set_physnet_idxs()
+            self._process_specs()
 
         # Return the modified state.
         result['result'] = self.args['state']
         return result
 
+    def _prune_absent_nodes(self):
+        """
+        Remove any nodes with state='absent' from the state dict.
+        """
+        for hyp in six.itervalues(self.args['state']):
+            hyp['nodes'] = [n for n in hyp['nodes']
+                            if n.get('state') != 'absent']
+
     def _set_physnet_idxs(self):
         """
         Set the index of each physnet for each host.
@@ -112,10 +126,9 @@ class ActionModule(ActionBase):
         """
         # Iterate through existing nodes, marking for deletion where necessary.
         for hyp in six.itervalues(self.args['state']):
-            # Anything already marked as 'absent' should no longer exist.
-            hyp['nodes'] = [n for n in hyp['nodes']
-                            if n.get('state') != 'absent']
-            for node in hyp['nodes']:
+            # Absent nodes cannot fulfil a spec.
+            for node in [n for n in hyp.get('nodes', [])
+                         if n.get('state') != 'absent']:
                 if ((self.localhost_vars['cmd'] == 'teardown' or
                      not self._tick_off_node(self.args['specs'], node))):
                     # We need to delete this node, since it exists but does not
@@ -204,27 +217,30 @@ class ActionModule(ActionBase):
             # any yet.
             ('state', {}),
             ('vol_name_prefix', 'vol'),
+            ('prune_only', False),
         ]
-        for arg in REQUIRED_ARGS:
-            if arg not in self.args:
-                e = "The parameter '%s' must be specified." % arg
-                raise AnsibleActionFail(to_text(e))
-
         for arg in OPTIONAL_ARGS:
             if arg[0] not in self.args:
                 self.args[arg[0]] = arg[1]
 
-        if not self.args['hypervisor_vars']:
-            e = ("There are no hosts in the 'hypervisors' group to which we "
-                 "can schedule.")
-            raise AnsibleActionFail(to_text(e))
+        # No arguments are required in prune_only mode.
+        if not self.args['prune_only']:
+            for arg in REQUIRED_ARGS:
+                if arg not in self.args:
+                    e = "The parameter '%s' must be specified." % arg
+                    raise AnsibleActionFail(to_text(e))
 
-        for spec in self.args['specs']:
-            if 'type' not in spec or 'count' not in spec:
-                e = ("All specs must contain a `type` and a `count`. "
-                     "Offending spec: %s" % spec)
+            if not self.args['hypervisor_vars']:
+                e = ("There are no hosts in the 'hypervisors' group to which "
+                     "we can schedule.")
                 raise AnsibleActionFail(to_text(e))
 
+            for spec in self.args['specs']:
+                if 'type' not in spec or 'count' not in spec:
+                    e = ("All specs must contain a `type` and a `count`. "
+                         "Offending spec: %s" % spec)
+                    raise AnsibleActionFail(to_text(e))
+
 
 @six.add_metaclass(abc.ABCMeta)
 class Scheduler():

ansible/cleanup_state.yml

@@ -0,0 +1,20 @@
+---
+- hosts: localhost
+  tasks:
+    - name: Load state from file
+      include_vars:
+        file: "{{ state_file_path }}"
+        name: tenks_state
+
+    - name: Prune absent nodes from state
+      tenks_update_state:
+        prune_only: true
+        state: "{{ tenks_state }}"
+      register: new_state
+
+    - name: Write new state to file
+      copy:
+        # tenks_schedule lookup plugin outputs a dict. Pretty-print this to
+        # persist it in a YAML file.
+        content: "{{ new_state.result | to_nice_yaml }}"
+        dest: "{{ state_file_path }}"

ansible/deploy.yml

@@ -25,3 +25,6 @@
 
 - name: Register flavors in Nova
   import_playbook: flavor_registration.yml
+
+- name: Clean up Tenks state
+  import_playbook: cleanup_state.yml

ansible/teardown.yml

@@ -25,3 +25,6 @@
 
 - name: Perform deployment host deconfiguration
   import_playbook: host_setup.yml
+
+- name: Clean up Tenks state
+  import_playbook: cleanup_state.yml

tests/test_tenks_update_state.py

@@ -201,7 +201,7 @@ class TestTenksUpdateState(unittest.TestCase):
         self.assertIn(new_node, self.args['state']['foo']['nodes'])
 
     def test__process_specs_teardown(self):
-        # Create some nodes definitions.
+        # Create some node definitions.
         self.mod._process_specs()
 
         # After teardown, we expected all created definitions to now have an
@@ -210,13 +210,13 @@ class TestTenksUpdateState(unittest.TestCase):
         for node in expected_state['foo']['nodes']:
             node['state'] = 'absent'
         self.mod.localhost_vars['cmd'] = 'teardown'
-        self.mod._process_specs()
-        self.assertEqual(expected_state, self.args['state'])
 
-        # After yet another run, the 'absent' state nodes should be deleted
-        # from state altogether.
-        self.mod._process_specs()
-        self.assertEqual(self.args['state']['foo']['nodes'], [])
+        # After one or more runs, the 'absent' state nodes should still exist,
+        # since they're only removed after completion of deployment in a
+        # playbook.
+        for _ in six.moves.range(3):
+            self.mod._process_specs()
+            self.assertEqual(expected_state, self.args['state'])
 
     def test__process_specs_no_hypervisors(self):
         self.args['hypervisor_vars'] = {}
@@ -243,3 +243,13 @@ class TestTenksUpdateState(unittest.TestCase):
         self.hypervisor_vars['foo']['ipmi_port_range_start'] = 123
         self.hypervisor_vars['foo']['ipmi_port_range_end'] = 123
         self.assertRaises(AnsibleActionFail, self.mod._process_specs)
+
+    def test__prune_absent_nodes(self):
+        # Create some node definitions.
+        self.mod._process_specs()
+        # Set them to be 'absent'.
+        for node in self.args['state']['foo']['nodes']:
+            node['state'] = 'absent'
+        self.mod._prune_absent_nodes()
+        # Ensure they were removed.
+        self.assertEqual(self.args['state']['foo']['nodes'], [])