Merge lp:~mpontillo/maas/dhcp-leases-bug-1522294-trunk into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Generate DHCP lease names with identifiers that will parse when dhcpd re-reads the file.

[Blake Rouse (blake-rouse)]

Looks good. This needs to go in 1.9 as well but there will be a difference.

You have a conflict about unicode() and str(). In trunk it will now be str() and in 1.9 it will be unicode().

review: Approve

=== modified file 'src/provisioningserver/dhcp/omshell.py'
--- src/provisioningserver/dhcp/omshell.py	2015-12-01 18:12:59 +0000
+++ src/provisioningserver/dhcp/omshell.py	2015-12-04 21:58:18 +0000
@@ -160,6 +160,7 @@
         # the key) and new host mappings (using the MAC as the key).
         maaslog.debug(
             "Creating host mapping %s->%s" % (mac_address, ip_address))
+        name = mac_address.replace(':', '-')
         stdin = dedent("""\
             server {self.server_address}
             key omapi_key {self.shared_key}
@@ -168,11 +169,12 @@
             set ip-address = {ip_address}
             set hardware-address = {mac_address}
             set hardware-type = 1
-            set name = "{mac_address}"
+            set name = "{name}"
             create
             """)
         stdin = stdin.format(
-            self=self, ip_address=ip_address, mac_address=mac_address)
+            self=self, ip_address=ip_address, mac_address=mac_address,
+            name=name)
 
         returncode, output = self._run(stdin)
         # If the call to omshell doesn't result in output containing the
@@ -201,6 +203,7 @@
         # be the key for the mapping (it will be the IP if the record was
         # created with by an old version of MAAS and the MAC otherwise).
         maaslog.debug("Removing host mapping key=%s" % mac_address)
+        mac_address = mac_address.replace(':', '-')
         stdin = dedent("""\
             server {self.server_address}
             key omapi_key {self.shared_key}

=== modified file 'src/provisioningserver/dhcp/tests/test_omshell.py'
--- src/provisioningserver/dhcp/tests/test_omshell.py	2015-12-01 18:12:59 +0000
+++ src/provisioningserver/dhcp/tests/test_omshell.py	2015-12-04 21:58:18 +0000
@@ -111,12 +111,12 @@
             set ip-address = {ip}
             set hardware-address = {mac}
             set hardware-type = 1
-            set name = "{mac}"
+            set name = "{name}"
             create
             """)
         expected_script = expected_script.format(
             server=server_address, key=shared_key, ip=ip_address,
-            mac=mac_address)
+            mac=mac_address, name=mac_address.replace(':', '-'))
 
         # Check that the 'stdin' arg contains the correct set of
         # commands.

=== modified file 'src/provisioningserver/rpc/dhcp.py'
--- src/provisioningserver/rpc/dhcp.py	2015-12-01 18:12:59 +0000
+++ src/provisioningserver/rpc/dhcp.py	2015-12-04 21:58:18 +0000
@@ -194,7 +194,7 @@
 
 
 @synchronous
-def remove_host_maps(mac_addresses, shared_key):
+def remove_host_maps(identifiers, shared_key):
     """Remove DHCP host maps for the given IP addresses.
 
     Additionally, this will ensure that any lease present for the MAC
@@ -210,17 +210,21 @@
     _ensure_dhcpv4_is_accessible(CannotRemoveHostMap)
     # See bug 1039362 regarding server_address.
     omshell = Omshell(server_address='127.0.0.1', shared_key=shared_key)
-    for mac_address in mac_addresses:
+    for identifier in identifiers:
         try:
-            omshell.remove(mac_address)
-            omshell.nullify_lease(mac_address)
+            # Note: identifiers can be either MAC addresses or IP addresses;
+            # the region will send us a set of both, due to backward
+            # compatibility issues (MAAS 1.8 used the IP address as the
+            # identifier; now we use the MAC.)
+            omshell.remove(identifier)
+            omshell.nullify_lease(identifier)
         except ExternalProcessError as e:
             maaslog.error(
                 "Could not remove host map for %s: %s.",
-                mac_address, str(e))
+                identifier, str(e))
             if 'not connected.' in e.output_as_unicode:
                 raise CannotRemoveHostMap(
                     "The DHCP server could not be reached.")
             else:
                 raise CannotRemoveHostMap("%s: %s." % (
-                    mac_address, e.output_as_unicode))
+                    identifier, e.output_as_unicode))