launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/start_node_ignores_effective_powertype into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1004473 in MAAS: "NodeManager.start_nodes ignores the default power_type setting if a node has no specific power_type set."
  https://bugs.launchpad.net/maas/+bug/1004473

For more details, see:
https://code.launchpad.net/~rvb/maas/start_node_ignores_effective_powertype/+merge/107367

This branch fixes the way NodeManager.start_nodes accesses a node's power_type field.  It should use node.get_effective_power_type() to take into account the default power_type value (if it's defined and if node.power_type=POWER_TYPE.DEFAULT) instead of accessing node.power_type directly.
-- 
https://code.launchpad.net/~rvb/maas/start_node_ignores_effective_powertype/+merge/107367
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/start_node_ignores_effective_powertype into lp:maas.

=== modified file 'src/maasserver/models/__init__.py'
--- src/maasserver/models/__init__.py	2012-05-25 03:00:48 +0000
+++ src/maasserver/models/__init__.py	2012-05-25 12:42:19 +0000
@@ -56,7 +56,6 @@
 from django.db.models.signals import post_save
 from django.shortcuts import get_object_or_404
 from maasserver import DefaultMeta
-from maasserver.fields import JSONObjectField
 from maasserver.enum import (
     ARCHITECTURE,
     ARCHITECTURE_CHOICES,
@@ -71,7 +70,10 @@
     CannotDeleteUserException,
     NodeStateViolation,
     )
-from maasserver.fields import MACAddressField
+from maasserver.fields import (
+    JSONObjectField,
+    MACAddressField,
+    )
 from maasserver.models.cleansave import CleanSave
 from maasserver.models.config import Config
 from maasserver.models.filestorage import FileStorage
@@ -346,7 +348,8 @@
         for node in nodes:
             NodeUserData.objects.set_user_data(node, user_data)
             # Wake on LAN is a special case, deal with it first.
-            if node.power_type == POWER_TYPE.WAKE_ON_LAN:
+            node_power_type = node.get_effective_power_type()
+            if node_power_type == POWER_TYPE.WAKE_ON_LAN:
                 # If power_parameters is set, use it.  Otherwise, use the
                 # first registered MAC address.
                 mac = None
@@ -360,11 +363,11 @@
                     else:
                         mac = macaddress.mac_address
                 if mac is not None and mac != "":
-                    power_on.delay(node.power_type, mac=mac)
+                    power_on.delay(node_power_type, mac=mac)
                     processed_nodes.append(node)
             else:
                 if node.power_parameters:
-                    power_on.delay(node.power_type, **node.power_parameters)
+                    power_on.delay(node_power_type, **node.power_parameters)
                     processed_nodes.append(node)
         return processed_nodes
 

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-05-25 03:00:48 +0000
+++ src/maasserver/tests/test_models.py	2012-05-25 12:42:19 +0000
@@ -692,6 +692,23 @@
                 fixture.tasks[0]['kwargs']['mac'],
             ))
 
+    def test_start_nodes_uses_default_power_type_if_not_node_specific(self):
+        # If the node has a power_type set to POWER_TYPE.DEFAULT,
+        # NodeManager.start_node(this_node) should use the power_type set as
+        # the default (if defined).
+        fixture = self.useFixture(CeleryFixture())
+
+        Config.objects.set_config('node_power_type', POWER_TYPE.WAKE_ON_LAN)
+        user = factory.make_user()
+        node, unused = self.make_node_with_mac(
+            user, power_type=POWER_TYPE.DEFAULT)
+        output = Node.objects.start_nodes([node.system_id], user)
+
+        self.assertItemsEqual([node], output)
+        self.assertEqual(
+            (1, 'provisioningserver.tasks.power_on'),
+            (len(fixture.tasks), fixture.tasks[0]['task'].name))
+
     def test_start_nodes_wakeonlan_prefers_power_parameters(self):
         # If power_parameters is set we should prefer it to sifting
         # through related MAC addresses.