launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/keep-cobblers-toys-in-the-pram into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/keep-cobblers-toys-in-the-pram/+merge/100852
-- 
https://code.launchpad.net/~allenap/maas/keep-cobblers-toys-in-the-pram/+merge/100852
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/keep-cobblers-toys-in-the-pram into lp:maas.

=== modified file 'src/provisioningserver/api.py'
--- src/provisioningserver/api.py	2012-03-30 13:36:46 +0000
+++ src/provisioningserver/api.py	2012-04-04 18:40:37 +0000
@@ -58,7 +58,7 @@
         "mac_addresses": [
             mac_address.strip()
             for mac_address in mac_addresses
-            if not mac_address.isspace()
+            if mac_address and not mac_address.isspace()
             ],
         "power_type": data["power_type"],
         }
@@ -134,6 +134,15 @@
         in izip(eth_names, mac_addresses, dns_names)
         }
 
+    # If we're removing all MAC addresses, we need to leave one unconfigured
+    # interface behind to satisfy Cobbler's data model constraints.
+    if len(mac_addresses) == 0:
+        interfaces_to["eth0"] = {
+            "interface": "eth0",
+            "mac_address": "",
+            "dns_name": "",
+            }
+
     # Go through interfaces, generating deltas from `interfaces_from` to
     # `interfaces_to`. This is done in sorted order to make testing easier.
     interface_names = set().union(interfaces_from, interfaces_to)

=== modified file 'src/provisioningserver/tests/test_api.py'
--- src/provisioningserver/tests/test_api.py	2012-04-04 13:40:05 +0000
+++ src/provisioningserver/tests/test_api.py	2012-04-04 18:40:37 +0000
@@ -69,6 +69,9 @@
             "hostname": "dystopia",
             "interfaces": {
                 "eth0": {"mac_address": "12:34:56:78:9a:bc"},
+                "eth1": {"mac_address": "  "},
+                "eth2": {"mac_address": ""},
+                "eth3": {"mac_address": None},
                 },
             "power_type": "virsh",
             "ju": "nk",
@@ -257,6 +260,31 @@
             current_interfaces, hostname, mac_addresses)
         self.assertItemsEqual(expected, observed)
 
+    def test_gen_cobbler_interface_deltas_remove_all_macs(self):
+        # Removing all MAC addresses results in a delta to remove all but the
+        # first interface. The first interface is instead deconfigured; this
+        # is necessary to satisfy the Cobbler data model.
+        current_interfaces = {
+            "eth0": {
+                "mac_address": "11:11:11:11:11:11",
+                },
+            "eth1": {
+                "mac_address": "22:22:22:22:22:22",
+                },
+            }
+        hostname = "empiricism"
+        mac_addresses = []
+        expected = [
+            {"interface": "eth0",
+             "mac_address": "",
+             "dns_name": ""},
+            {"interface": "eth1",
+             "delete_interface": True},
+            ]
+        observed = gen_cobbler_interface_deltas(
+            current_interfaces, hostname, mac_addresses)
+        self.assertItemsEqual(expected, observed)
+
 
 class ProvisioningAPITests(ProvisioningFakeFactory):
     """Tests for `provisioningserver.api.ProvisioningAPI`.
@@ -394,6 +422,19 @@
             [mac_address2], values[node_name]["mac_addresses"])
 
     @inlineCallbacks
+    def test_modify_nodes_remove_all_mac_addresses(self):
+        papi = self.get_provisioning_api()
+        node_name = yield self.add_node(papi)
+        mac_address = factory.getRandomMACAddress()
+        yield papi.modify_nodes(
+            {node_name: {"mac_addresses": [mac_address]}})
+        yield papi.modify_nodes(
+            {node_name: {"mac_addresses": []}})
+        values = yield papi.get_nodes_by_name([node_name])
+        self.assertEqual(
+            [], values[node_name]["mac_addresses"])
+
+    @inlineCallbacks
     def test_delete_distros_by_name(self):
         # Create a distro via the Provisioning API.
         papi = self.get_provisioning_api()