Merge ~mpontillo/maas:default-dns-ip--bug-1776604--2.3 into maas:2.3

[Mike Pontillo]

LP: #1776604 - Improve hostname mapping heuristic for bonded bridges

This commit changes the default hostname to IP address mapping to
account for situations where an interface parent's parent is a
machine's boot interface.

Backports: 72aba3a90faa053905c20967604195e80fee6b05

[MAAS Lander (maas-lander)]

UNIT TESTS
-b default-dns-ip--bug-1776604--2.3 lp:~mpontillo/maas/+git/maas into -b 2.3 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3395/console
COMMIT: 9700765435ddfc532aa62a6dc41c239c7a1cd418

review: Needs Fixing

e74e6dc... by Mike Pontillo on 2018-07-13

Fix errors resulting from merge conflict.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b default-dns-ip--bug-1776604--2.3 lp:~mpontillo/maas/+git/maas into -b 2.3 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3398/console
COMMIT: e74e6dc2150c8c25dc516381dead740c1d6cccd8

review: Needs Fixing

b3662e7... by Mike Pontillo on 2018-07-13

Update test cases to remove portions that don't work on MAAS 2.3.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b default-dns-ip--bug-1776604--2.3 lp:~mpontillo/maas/+git/maas into -b 2.3 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: b3662e720ee79d6f87b199dc362890b5b0468b6c

review: Approve

[Mike Pontillo (mpontillo)]

Self-approve backport.

review: Approve

diff --git a/src/maasserver/models/interface.py b/src/maasserver/models/interface.py
index 5c425b5..9a1867a 100644
--- a/src/maasserver/models/interface.py
+++ b/src/maasserver/models/interface.py
@@ -1116,6 +1116,8 @@ class Interface(CleanSave, TimestampedModel):
         """Remove all the `IPAddress` link on the interface."""
         for ip_address in self.ip_addresses.exclude(
                 alloc_type=IPADDRESS_TYPE.DISCOVERED):
+            maaslog.info("%s: IP address automatically unlinked: %s" % (
+                self.get_log_string(), ip_address))
             self.unlink_ip_address(ip_address, clearing_config=clearing_config)
 
     def claim_auto_ips(self, exclude_addresses=[]):
diff --git a/src/maasserver/models/staticipaddress.py b/src/maasserver/models/staticipaddress.py
index 26ccd1b..40cdc1b 100644
--- a/src/maasserver/models/staticipaddress.py
+++ b/src/maasserver/models/staticipaddress.py
@@ -455,7 +455,7 @@ class StaticIPAddressManager(Manager):
                     domain.ttl,
                     %s)""" % default_ttl
         sql_query = """
-            SELECT DISTINCT ON (node.hostname, is_boot, family(staticip.ip))
+            SELECT DISTINCT ON (fqdn, is_boot, family)
                 CONCAT(node.hostname, '.', domain.name) AS fqdn,
                 node.system_id,
                 node.node_type,
@@ -465,16 +465,38 @@ class StaticIPAddressManager(Manager):
                     node.boot_interface_id IS NOT NULL AND
                     (
                         node.boot_interface_id = interface.id OR
-                        node.boot_interface_id = parent.id
+                        node.boot_interface_id = parent.id OR
+                        node.boot_interface_id = parent_parent.id
                     ),
                     False
-                ) AS is_boot
+                ) AS is_boot,
+                CASE
+                    WHEN interface.type = 'bridge' AND
+                        parent_parent.id = node.boot_interface_id THEN 1
+                    WHEN interface.type = 'bridge' AND
+                        parent.id = node.boot_interface_id THEN 2
+                    WHEN interface.type = 'bond' AND
+                        parent.id = node.boot_interface_id THEN 3
+                    WHEN interface.type = 'physical' AND
+                        interface.id = node.boot_interface_id THEN 4
+                    WHEN interface.type = 'bond' THEN 5
+                    WHEN interface.type = 'physical' THEN 6
+                    WHEN interface.type = 'vlan' THEN 7
+                    WHEN interface.type = 'alias' THEN 8
+                    WHEN interface.type = 'unknown' THEN 9
+                    ELSE 10
+                END AS preference,
+                family(staticip.ip) AS family
             FROM
                 maasserver_interface AS interface
             LEFT OUTER JOIN maasserver_interfacerelationship AS rel ON
                 interface.id = rel.child_id
             LEFT OUTER JOIN maasserver_interface AS parent ON
                 rel.parent_id = parent.id
+            LEFT OUTER JOIN maasserver_interfacerelationship AS parent_rel ON
+                parent.id = parent_rel.child_id
+            LEFT OUTER JOIN maasserver_interface AS parent_parent ON
+                parent_rel.parent_id = parent_parent.id
             JOIN maasserver_node AS node ON
                 node.id = interface.node_id
             JOIN maasserver_domain AS domain ON
@@ -512,21 +534,10 @@ class StaticIPAddressManager(Manager):
                 staticip.ip IS NOT NULL AND
                 host(staticip.ip) != ''
             ORDER BY
-                node.hostname,
+                fqdn,
                 is_boot DESC,
-                family(staticip.ip),
-                CASE
-                    WHEN interface.type = 'bond' AND
-                        parent.id = node.boot_interface_id THEN 1
-                    WHEN interface.type = 'physical' AND
-                        interface.id = node.boot_interface_id THEN 2
-                    WHEN interface.type = 'bond' THEN 3
-                    WHEN interface.type = 'physical' THEN 4
-                    WHEN interface.type = 'vlan' THEN 5
-                    WHEN interface.type = 'alias' THEN 6
-                    WHEN interface.type = 'unknown' THEN 7
-                    ELSE 8
-                END,
+                family,
+                preference,
                 /*
                  * We want STICKY and USER_RESERVED addresses to be preferred,
                  * followed by AUTO, DHCP, and finally DISCOVERED.
@@ -612,7 +623,7 @@ class StaticIPAddressManager(Manager):
         # we will see all of the boot interfaces before we see any non-boot
         # interface IPs.  See Bug#1584850
         for (fqdn, system_id, node_type, ttl,
-                ip, is_boot) in cursor.fetchall():
+                ip, is_boot, preference, family) in cursor.fetchall():
             mapping[fqdn].node_type = node_type
             mapping[fqdn].system_id = system_id
             mapping[fqdn].ttl = ttl
diff --git a/src/maasserver/models/tests/test_staticipaddress.py b/src/maasserver/models/tests/test_staticipaddress.py
index 995bb7a..d9a79a3 100644
--- a/src/maasserver/models/tests/test_staticipaddress.py
+++ b/src/maasserver/models/tests/test_staticipaddress.py
@@ -821,6 +821,84 @@ class TestStaticIPAddressManagerMapping(MAASServerTestCase):
                 node.system_id, 30, {vlanip.ip}, node.node_type)}
         self.assertEqual(expected_mapping, mapping)
 
+    def test_get_hostname_ip_mapping_prefers_bridged_bond_pxe_interface(self):
+        subnet = factory.make_Subnet(
+            cidr='10.0.0.0/24', dns_servers=[], gateway_ip='10.0.0.1')
+        node = factory.make_Node_with_Interface_on_Subnet(
+            hostname='host', subnet=subnet)
+        eth0 = node.get_boot_interface()
+        eth0.name = 'eth0'
+        eth0.save()
+        eth1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, name='eth1', vlan=subnet.vlan)
+        eth2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, name='eth2', vlan=subnet.vlan)
+        node.boot_interface = eth1
+        node.save()
+        bond0 = factory.make_Interface(
+            INTERFACE_TYPE.BOND, node=node, parents=[eth1, eth2], name='bond0')
+        br_bond0 = factory.make_Interface(
+            INTERFACE_TYPE.BRIDGE, parents=[bond0], name='br-bond0')
+        phy_staticip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, interface=eth0,
+            subnet=subnet, ip='10.0.0.2')
+        bridge_ip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, interface=br_bond0,
+            subnet=subnet, ip='10.0.0.3')
+        mapping = StaticIPAddress.objects.get_hostname_ip_mapping(
+            node.domain)
+        expected_mapping = {
+            node.fqdn: HostnameIPMapping(
+                node.system_id, 30, {bridge_ip.ip}, node.node_type),
+            "%s.%s" % (eth0.name, node.fqdn): HostnameIPMapping(
+                node.system_id, 30, {phy_staticip.ip}, node.node_type),
+        }
+        self.assertThat(mapping, Equals(expected_mapping))
+
+    def test_get_hostname_ip_mapping_with_v4_and_v6_and_bridged_bonds(self):
+        subnet_v4 = factory.make_Subnet(
+            cidr=str(factory.make_ipv4_network().cidr))
+        subnet_v6 = factory.make_Subnet(
+            cidr='2001:db8::/64')
+        node = factory.make_Node_with_Interface_on_Subnet(
+            hostname='host', subnet=subnet_v4)
+        eth0 = node.get_boot_interface()
+        eth0.name = 'eth0'
+        eth0.save()
+        eth1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, name='eth1')
+        eth2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, name='eth2')
+        node.boot_interface = eth1
+        node.save()
+        bond0 = factory.make_Interface(
+            INTERFACE_TYPE.BOND, node=node, parents=[eth1, eth2], name='bond0')
+        br_bond0 = factory.make_Interface(
+            INTERFACE_TYPE.BRIDGE, parents=[bond0], name='br-bond0')
+        phy_staticip_v4 = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, interface=eth0,
+            subnet=subnet_v4)
+        bridge_ip_v4 = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, interface=br_bond0,
+            subnet=subnet_v4)
+        phy_staticip_v6 = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, interface=eth0,
+            subnet=subnet_v6)
+        bridge_ip_v6 = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.STICKY, interface=br_bond0,
+            subnet=subnet_v6)
+        mapping = StaticIPAddress.objects.get_hostname_ip_mapping(
+            node.domain)
+        expected_mapping = {
+            node.fqdn: HostnameIPMapping(
+                node.system_id, 30, {bridge_ip_v4.ip, bridge_ip_v6.ip},
+                node.node_type),
+            "%s.%s" % (eth0.name, node.fqdn): HostnameIPMapping(
+                node.system_id, 30, {phy_staticip_v4.ip, phy_staticip_v6.ip},
+                node.node_type),
+        }
+        self.assertThat(mapping, Equals(expected_mapping))
+
     def test_get_hostname_ip_mapping_returns_domain_head_ips(self):
         parent = factory.make_Domain()
         name = factory.make_name()