launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/power-off-at-release into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1026460 in MAAS: "Releasing a node doesn't shut it down"
  https://bugs.launchpad.net/maas/+bug/1026460

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/power-off-at-release/+merge/116188

Discussed with a few people and everyone is in agreement that when a node is released from ownership it needs to be shut down, otherwise it's not re-usable by anyone as it cannot be booted to a fresh OS install.
-- 
https://code.launchpad.net/~julian-edwards/maas/power-off-at-release/+merge/116188
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/power-off-at-release into lp:maas.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-07-19 18:43:28 +0000
+++ src/maasserver/models/node.py	2012-07-23 05:12:25 +0000
@@ -590,7 +590,8 @@
         self.save()
 
     def release(self):
-        """Mark allocated or reserved node as available again."""
+        """Mark allocated or reserved node as available again and power off."""
+        Node.objects.stop_nodes([self.system_id], self.owner)
         self.status = NODE_STATUS.READY
         self.owner = None
         self.token = None

=== modified file 'src/maasserver/tests/test_node.py'
--- src/maasserver/tests/test_node.py	2012-07-19 18:43:28 +0000
+++ src/maasserver/tests/test_node.py	2012-07-23 05:12:25 +0000
@@ -44,6 +44,7 @@
     )
 from provisioningserver.enum import POWER_TYPE
 from provisioningserver.power.poweraction import PowerAction
+from provisioningserver import tasks
 from testtools.matchers import FileContains
 
 
@@ -258,6 +259,18 @@
         node.release()
         self.assertEqual((NODE_STATUS.READY, None), (node.status, node.owner))
 
+    def test_release_powers_off_node(self):
+        # Test that releasing a node causes a 'power_off' celery job.
+        node = factory.make_node(
+            status=NODE_STATUS.ALLOCATED, owner=factory.make_user(),
+            power_type=POWER_TYPE.VIRSH)
+        # Prevent actual job script from running.
+        self.patch(PowerAction, 'run_shell', lambda *args, **kwargs: ('', ''))
+        node.release()
+        self.assertEqual(
+            (1, 'provisioningserver.tasks.power_off'),
+            (len(self.celery.tasks), self.celery.tasks[0]['task'].name))
+
     def test_accept_enlistment_gets_node_out_of_declared_state(self):
         # If called on a node in Declared state, accept_enlistment()
         # changes the node's status, and returns the node.