Merge ~mpontillo/maas:ha-dns-servers--bug-1753493--2.3 into maas:2.3

[Mike Pontillo]

LP: #1753493 - Make region IP addresses consistent for HA.

Backports: 5000bb58ea91af41b2465dc3d17aa2388a5edbaa

[Mike Pontillo (mpontillo)]

Self-approve backport.

review: Approve

diff --git a/src/maasserver/server_address.py b/src/maasserver/server_address.py
index b543556..e786068 100644
--- a/src/maasserver/server_address.py
+++ b/src/maasserver/server_address.py
@@ -110,17 +110,21 @@ def get_maas_facing_server_addresses(
         raise UnresolvableHost("No address found for host %s." % hostname)
     if not link_local:
         addresses = [ip for ip in addresses if not ip.is_link_local()]
+    # Addresses must be returned in a consistent order, with the set of IP
+    # addresses found from get_maas_facing_server_host() (i.e. maas_url)
+    # coming first.
+    addresses = sorted(addresses)
     if include_alternates:
         maas_id = get_maas_id()
         if maas_id is not None:
             # Circular imports
             from maasserver.models import Subnet
             from maasserver.models import StaticIPAddress
-            # Keep track of the regions already represented.
+            # Don't include more than one alternate IP address, per region,
+            # per address-family.
             regions = set()
             alternate_ips = []
             for ip in addresses:
-                regions.add(maas_id + str(ip.version))
                 if not ip.is_link_local():
                     # Since we only know that the IP address given in the MAAS
                     # URL is reachable, alternates must be pulled from the same
@@ -149,5 +153,10 @@ def get_maas_facing_server_addresses(
                             else:
                                 regions.add(id_plus_family)
                                 alternate_ips.append(ipa)
-            addresses.extend(alternate_ips)
+            # Append non-duplicate region IP addresses to the list of
+            # addresses to return. We don't want duplicates, but we
+            # also need to preserve the existing order.
+            for address in alternate_ips:
+                if address not in addresses:
+                    addresses.append(address)
     return addresses
diff --git a/src/maasserver/tests/test_server_address.py b/src/maasserver/tests/test_server_address.py
index ff7640c..57d8ade 100644
--- a/src/maasserver/tests/test_server_address.py
+++ b/src/maasserver/tests/test_server_address.py
@@ -214,12 +214,36 @@ class TestGetMAASFacingServerAddresses(MAASServerTestCase):
             region_ips,
             Equals([
                 IPAddress("192.168.0.254"),
+                IPAddress("192.168.0.1"),
                 IPAddress("192.168.0.2"),
                 IPAddress("192.168.0.4"),
             ])
         )
 
-    def test__alternates_include_one_ip_address_per_region(self):
+    def test__alternates_do_not_contain_duplicate_for_maas_url_ip(self):
+        # See bug #1753493. (This tests to ensure we don't provide the same
+        # IP address from maas_url twice.) Also ensures that the IP address
+        # from maas_url comes first.
+        factory.make_Subnet(cidr='192.168.0.0/24')
+        maas_url = 'http://192.168.0.2/MAAS'
+        rack = factory.make_RackController(url=maas_url)
+        r1 = factory.make_RegionController()
+        factory.make_Interface(node=r1, ip='192.168.0.1')
+        r2 = factory.make_RegionController()
+        factory.make_Interface(node=r2, ip='192.168.0.2')
+        # Make the "current" region controller r1.
+        self.patch(server_address, 'get_maas_id').return_value = r1.system_id
+        region_ips = get_maas_facing_server_addresses(
+            rack, include_alternates=True)
+        self.assertThat(
+            region_ips,
+            Equals([
+                IPAddress("192.168.0.2"),
+                IPAddress("192.168.0.1"),
+            ])
+        )
+
+    def test__alternates_include_one_ip_address_per_region_and_maas_url(self):
         factory.make_Subnet(cidr='192.168.0.0/24')
         maas_url = 'http://192.168.0.254/MAAS'
         rack = factory.make_RackController(url=maas_url)
@@ -240,6 +264,7 @@ class TestGetMAASFacingServerAddresses(MAASServerTestCase):
             region_ips,
             Equals([
                 IPAddress("192.168.0.254"),
+                IPAddress("192.168.0.1"),
                 IPAddress("192.168.0.2"),
                 IPAddress("192.168.0.4"),
             ])
@@ -296,12 +321,12 @@ class TestGetMAASFacingServerAddresses(MAASServerTestCase):
         self.assertThat(
             region_ips,
             Equals([
-                IPAddress("2001:db8::1"),
                 IPAddress("192.168.0.1"),
-                IPAddress("2001:db8::2"),
-                IPAddress("2001:db8::4"),
+                IPAddress("2001:db8::1"),
                 IPAddress("192.168.0.2"),
                 IPAddress("192.168.0.4"),
+                IPAddress("2001:db8::2"),
+                IPAddress("2001:db8::4"),
             ])
         )