Merge lp:~lamont/maas/bug-1602482 into lp:~maas-committers/maas/trunk

[LaMont Jones]

If there are any assigned (not DISCOVERED) IP addresses on a host, then do not add any DISCOVERED addresses to the DNS.

If there are any assigned (not DISCOVERED) IP addresses on a host, then do not add any DISCOVERED addresses to the DNS.

[Blake Rouse (blake-rouse)]

Looks good. Makes since, surprised we where not doing this already.

review: Approve

=== modified file 'src/maasserver/models/staticipaddress.py'
--- src/maasserver/models/staticipaddress.py	2016-10-12 16:19:16 +0000
+++ src/maasserver/models/staticipaddress.py	2016-10-14 14:24:16 +0000
@@ -420,7 +420,8 @@
                 node.node_type,
                 """ + ttl_clause + """ AS ttl,
                 staticip.ip,
-                interface.name
+                interface.name,
+                alloc_type != 6 /* DISCOVERED */ AS assigned
             FROM
                 maasserver_interface AS interface
             JOIN maasserver_node AS node ON
@@ -457,6 +458,7 @@
                 host(staticip.ip) != ''
             ORDER BY
                 node.hostname,
+                assigned DESC, /* Return all assigned IPs for a node first. */
                 interface.id
             """
         # We get user reserved et al mappings first, so that we can overwrite
@@ -467,6 +469,7 @@
         iface_is_boot = defaultdict(bool, {
             hostname: True for hostname in mapping.keys()
         })
+        assigned_ips = defaultdict(bool)
         cursor.execute(sql_query, query_parms)
         # The records from the query provide, for each hostname (after
         # stripping domain), the boot and non-boot interface ip address in ipv4
@@ -485,16 +488,22 @@
             if is_boot == iface_is_boot[fqdn]:
                 mapping[fqdn].ips.add(ip)
         # Next, get all the addresses, on all the interfaces, and add the ones
-        # that are not already present on the FQDN as $IFACE.$FQDN.
+        # that are not already present on the FQDN as $IFACE.$FQDN.  Exclude
+        # any discovered addresses once there are any non-discovered addresses.
         cursor.execute(iface_sql_query, (domain_or_subnet.id,))
         for (fqdn, system_id, node_type, ttl,
-                ip, iface_name) in cursor.fetchall():
-            if ip not in mapping[fqdn].ips:
-                name = "%s.%s" % (iface_name, fqdn)
-                mapping[name].node_type = node_type
-                mapping[name].system_id = system_id
-                mapping[name].ttl = ttl
-                mapping[name].ips.add(ip)
+                ip, iface_name, assigned) in cursor.fetchall():
+            if assigned:
+                assigned_ips[fqdn] = True
+            # If this is an assigned IP, or there are NO assigned IPs on the
+            # node, then consider adding the IP.
+            if assigned or not assigned_ips[fqdn]:
+                if ip not in mapping[fqdn].ips:
+                    name = "%s.%s" % (iface_name, fqdn)
+                    mapping[name].node_type = node_type
+                    mapping[name].system_id = system_id
+                    mapping[name].ttl = ttl
+                    mapping[name].ips.add(ip)
         return mapping
 
     def filter_by_ip_family(self, family):

=== modified file 'src/maasserver/models/tests/test_staticipaddress.py'
--- src/maasserver/models/tests/test_staticipaddress.py	2016-10-12 17:34:30 +0000
+++ src/maasserver/models/tests/test_staticipaddress.py	2016-10-14 14:24:16 +0000
@@ -600,16 +600,14 @@
         staticip = factory.make_StaticIPAddress(
             alloc_type=IPADDRESS_TYPE.AUTO, interface=iface,
             subnet=subnet)
-        discovered = factory.make_StaticIPAddress(
+        factory.make_StaticIPAddress(
             alloc_type=IPADDRESS_TYPE.DISCOVERED, interface=iface,
             subnet=subnet)
         mapping = StaticIPAddress.objects.get_hostname_ip_mapping(
             node.domain)
         expected_mapping = {
             node.fqdn: HostnameIPMapping(
-                node.system_id, 30, {staticip.ip}, node.node_type),
-            "%s.%s" % (iface.name, node.fqdn): HostnameIPMapping(
-                node.system_id, 30, {discovered.ip}, node.node_type)}
+                node.system_id, 30, {staticip.ip}, node.node_type)}
         self.assertEqual(expected_mapping, mapping)
 
     def test_get_hostname_ip_mapping_prefers_bond_with_no_boot_interface(self):
@@ -828,9 +826,7 @@
         mapping = StaticIPAddress.objects.get_hostname_ip_mapping(domain)
         expected_mapping = {
             node.fqdn: HostnameIPMapping(
-                node.system_id, 30, {sip0.ip}, node.node_type),
-            "%s.%s" % (iface1.name, node.fqdn): HostnameIPMapping(
-                node.system_id, 30, {sip1.ip}, node.node_type)}
+                node.system_id, 30, {sip0.ip}, node.node_type)}
         self.assertEqual(expected_mapping, mapping)
 
     def test_get_hostname_ip_mapping_returns_correct_bond_ip(self):
@@ -877,8 +873,6 @@
         expected_mapping = {
             node.fqdn: HostnameIPMapping(
                 node.system_id, 30, {bond_sip.ip}, node.node_type),
-            "%s.%s" % (iface1.name, node.fqdn): HostnameIPMapping(
-                node.system_id, 30, {sip1.ip}, node.node_type),
             }
         self.assertEqual(expected_mapping, mapping)