launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/control-netboot-in-cobbler into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/control-netboot-in-cobbler/+merge/100972

This also removes a superfluous change in start_nodes() that was introduced in r400.3.3.
-- 
https://code.launchpad.net/~allenap/maas/control-netboot-in-cobbler/+merge/100972
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/control-netboot-in-cobbler into lp:maas.

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-04-05 12:24:50 +0000
+++ src/maasserver/models.py	2012-04-05 13:55:42 +0000
@@ -429,16 +429,10 @@
         # TODO: File structure needs sorting out to avoid this circular
         # import dance.
         from metadataserver.models import NodeUserData
-        from maasserver.provisioning import select_profile_for_node
         nodes = self.get_nodes(by_user, NODE_PERMISSION.EDIT, ids=ids)
-        profiles = {}
         for node in nodes:
             NodeUserData.objects.set_user_data(node, user_data)
-            profiles[node.system_id] = {
-                'profile': select_profile_for_node(node)}
-        papi = get_papi()
-        papi.modify_nodes(profiles)
-        papi.start_nodes([node.system_id for node in nodes])
+        get_papi().start_nodes([node.system_id for node in nodes])
         return nodes
 
 

=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	2012-04-04 07:01:35 +0000
+++ src/maasserver/provisioning.py	2012-04-05 13:55:42 +0000
@@ -249,6 +249,14 @@
     papi.add_node(
         instance.system_id, instance.hostname,
         profile, power_type, metadata)
+    if instance.status != NODE_STATUS.ALLOCATED:
+        deltas = {
+            instance.system_id: {
+                "netboot_enabled":
+                    instance.status != NODE_STATUS.RETIRED,
+                }
+            }
+        papi.modify_nodes(deltas)
 
 
 def set_node_mac_addresses(node):

=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	2012-04-05 07:50:52 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-04-05 13:55:42 +0000
@@ -24,6 +24,7 @@
     Node,
     NODE_AFTER_COMMISSIONING_ACTION,
     NODE_STATUS,
+    NODE_STATUS_CHOICES,
     )
 from maasserver.provisioning import (
     compose_metadata,
@@ -335,3 +336,32 @@
     def setUp(self):
         super(TestProvisioningWithFake, self).setUp()
         self.papi = provisioning.get_provisioning_api_proxy()
+
+    def test_provision_post_save_Node_set_netboot_enabled(self):
+        # When a node is under MAAS's control - i.e. not allocated and not
+        # retired - it is always configured for netbooting. When the node is
+        # allocated, netbooting is left alone. When the node is retired
+        # netbooting is disabled.
+        expected = {
+            NODE_STATUS.DECLARED: True,
+            NODE_STATUS.COMMISSIONING: True,
+            NODE_STATUS.FAILED_TESTS: True,
+            NODE_STATUS.MISSING: True,
+            NODE_STATUS.READY: True,
+            NODE_STATUS.RESERVED: True,
+            NODE_STATUS.ALLOCATED: None,  # No setting.
+            NODE_STATUS.RETIRED: False,
+            }
+        nodes = {
+            status: factory.make_node(status=status)
+            for status, title in NODE_STATUS_CHOICES
+            }
+        pserv_nodes = {
+            status: node.system_id
+            for status, node in nodes.items()
+            }
+        observed = {
+            status: self.papi.nodes[pserv_node].get("netboot_enabled")
+            for status, pserv_node in pserv_nodes.items()
+            }
+        self.assertEqual(expected, observed)

=== modified file 'src/provisioningserver/api.py'
--- src/provisioningserver/api.py	2012-04-04 18:34:15 +0000
+++ src/provisioningserver/api.py	2012-04-05 13:55:42 +0000
@@ -60,6 +60,7 @@
             for mac_address in mac_addresses
             if mac_address and not mac_address.isspace()
             ],
+        "netboot_enabled": data.get("netboot_enabled"),
         "power_type": data["power_type"],
         }
 

=== modified file 'src/provisioningserver/cobblerclient.py'
--- src/provisioningserver/cobblerclient.py	2012-03-29 12:57:02 +0000
+++ src/provisioningserver/cobblerclient.py	2012-04-05 13:55:42 +0000
@@ -726,6 +726,7 @@
         'mac_address',
         'profile',
         'dns_name',
+        'netboot_enabled',
         ]
 
     @classmethod

=== modified file 'src/provisioningserver/testing/fakeapi.py'
--- src/provisioningserver/testing/fakeapi.py	2012-03-29 16:31:32 +0000
+++ src/provisioningserver/testing/fakeapi.py	2012-04-05 13:55:42 +0000
@@ -92,6 +92,7 @@
         self.nodes[name]["hostname"] = hostname
         self.nodes[name]["profile"] = profile
         self.nodes[name]["mac_addresses"] = []
+        self.nodes[name]["netboot_enabled"] = True
         self.nodes[name]["metadata"] = metadata
         self.nodes[name]["power_type"] = power_type
         return name

=== modified file 'src/provisioningserver/tests/test_api.py'
--- src/provisioningserver/tests/test_api.py	2012-04-04 18:34:51 +0000
+++ src/provisioningserver/tests/test_api.py	2012-04-05 13:55:42 +0000
@@ -74,6 +74,7 @@
                 "eth3": {"mac_address": None},
                 },
             "power_type": "virsh",
+            "netboot_enabled": False,
             "ju": "nk",
             }
         expected = {
@@ -81,6 +82,7 @@
             "profile": "earth",
             "hostname": "dystopia",
             "mac_addresses": ["12:34:56:78:9a:bc"],
+            "netboot_enabled": False,
             "power_type": "virsh",
             }
         observed = cobbler_to_papi_node(data)
@@ -92,6 +94,7 @@
             "profile": "earth",
             "hostname": "darksaga",
             "power_type": "ether_wake",
+            "netboot_enabled": True,
             "ju": "nk",
             }
         expected = {
@@ -99,6 +102,7 @@
             "profile": "earth",
             "hostname": "darksaga",
             "mac_addresses": [],
+            "netboot_enabled": True,
             "power_type": "ether_wake",
             }
         observed = cobbler_to_papi_node(data)
@@ -422,6 +426,7 @@
             [mac_address2], values[node_name]["mac_addresses"])
 
     @inlineCallbacks
+<<<<<<< TREE
     def test_modify_nodes_remove_all_mac_addresses(self):
         papi = self.get_provisioning_api()
         node_name = yield self.add_node(papi)
@@ -435,6 +440,19 @@
             [], values[node_name]["mac_addresses"])
 
     @inlineCallbacks
+=======
+    def test_modify_nodes_set_netboot_enabled(self):
+        papi = self.get_provisioning_api()
+        node_name = yield self.add_node(papi)
+        yield papi.modify_nodes({node_name: {"netboot_enabled": False}})
+        values = yield papi.get_nodes_by_name([node_name])
+        self.assertFalse(values[node_name]["netboot_enabled"])
+        yield papi.modify_nodes({node_name: {"netboot_enabled": True}})
+        values = yield papi.get_nodes_by_name([node_name])
+        self.assertTrue(values[node_name]["netboot_enabled"])
+
+    @inlineCallbacks
+>>>>>>> MERGE-SOURCE
     def test_delete_distros_by_name(self):
         # Create a distro via the Provisioning API.
         papi = self.get_provisioning_api()