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

Proposed by Mike Pontillo on 2018-04-11
Status: Merged
Approved by: Mike Pontillo on 2018-04-11
Approved revision: 6ea58d795340d11d163e67aeb190bb0144218d83
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:ha-dns-servers--bug-1753493--2.3
Merge into: maas:2.3
Diff against target: 105 lines (+41/-7)
2 files modified
src/maasserver/server_address.py (+12/-3)
src/maasserver/tests/test_server_address.py (+29/-4)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve on 2018-04-11
Review via email: mp+343055@code.launchpad.net

Commit message

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

Backports: 5000bb58ea91af41b2465dc3d17aa2388a5edbaa

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

Self-approve backport.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/server_address.py b/src/maasserver/server_address.py
2index b543556..e786068 100644
3--- a/src/maasserver/server_address.py
4+++ b/src/maasserver/server_address.py
5@@ -110,17 +110,21 @@ def get_maas_facing_server_addresses(
6 raise UnresolvableHost("No address found for host %s." % hostname)
7 if not link_local:
8 addresses = [ip for ip in addresses if not ip.is_link_local()]
9+ # Addresses must be returned in a consistent order, with the set of IP
10+ # addresses found from get_maas_facing_server_host() (i.e. maas_url)
11+ # coming first.
12+ addresses = sorted(addresses)
13 if include_alternates:
14 maas_id = get_maas_id()
15 if maas_id is not None:
16 # Circular imports
17 from maasserver.models import Subnet
18 from maasserver.models import StaticIPAddress
19- # Keep track of the regions already represented.
20+ # Don't include more than one alternate IP address, per region,
21+ # per address-family.
22 regions = set()
23 alternate_ips = []
24 for ip in addresses:
25- regions.add(maas_id + str(ip.version))
26 if not ip.is_link_local():
27 # Since we only know that the IP address given in the MAAS
28 # URL is reachable, alternates must be pulled from the same
29@@ -149,5 +153,10 @@ def get_maas_facing_server_addresses(
30 else:
31 regions.add(id_plus_family)
32 alternate_ips.append(ipa)
33- addresses.extend(alternate_ips)
34+ # Append non-duplicate region IP addresses to the list of
35+ # addresses to return. We don't want duplicates, but we
36+ # also need to preserve the existing order.
37+ for address in alternate_ips:
38+ if address not in addresses:
39+ addresses.append(address)
40 return addresses
41diff --git a/src/maasserver/tests/test_server_address.py b/src/maasserver/tests/test_server_address.py
42index ff7640c..57d8ade 100644
43--- a/src/maasserver/tests/test_server_address.py
44+++ b/src/maasserver/tests/test_server_address.py
45@@ -214,12 +214,36 @@ class TestGetMAASFacingServerAddresses(MAASServerTestCase):
46 region_ips,
47 Equals([
48 IPAddress("192.168.0.254"),
49+ IPAddress("192.168.0.1"),
50 IPAddress("192.168.0.2"),
51 IPAddress("192.168.0.4"),
52 ])
53 )
54
55- def test__alternates_include_one_ip_address_per_region(self):
56+ def test__alternates_do_not_contain_duplicate_for_maas_url_ip(self):
57+ # See bug #1753493. (This tests to ensure we don't provide the same
58+ # IP address from maas_url twice.) Also ensures that the IP address
59+ # from maas_url comes first.
60+ factory.make_Subnet(cidr='192.168.0.0/24')
61+ maas_url = 'http://192.168.0.2/MAAS'
62+ rack = factory.make_RackController(url=maas_url)
63+ r1 = factory.make_RegionController()
64+ factory.make_Interface(node=r1, ip='192.168.0.1')
65+ r2 = factory.make_RegionController()
66+ factory.make_Interface(node=r2, ip='192.168.0.2')
67+ # Make the "current" region controller r1.
68+ self.patch(server_address, 'get_maas_id').return_value = r1.system_id
69+ region_ips = get_maas_facing_server_addresses(
70+ rack, include_alternates=True)
71+ self.assertThat(
72+ region_ips,
73+ Equals([
74+ IPAddress("192.168.0.2"),
75+ IPAddress("192.168.0.1"),
76+ ])
77+ )
78+
79+ def test__alternates_include_one_ip_address_per_region_and_maas_url(self):
80 factory.make_Subnet(cidr='192.168.0.0/24')
81 maas_url = 'http://192.168.0.254/MAAS'
82 rack = factory.make_RackController(url=maas_url)
83@@ -240,6 +264,7 @@ class TestGetMAASFacingServerAddresses(MAASServerTestCase):
84 region_ips,
85 Equals([
86 IPAddress("192.168.0.254"),
87+ IPAddress("192.168.0.1"),
88 IPAddress("192.168.0.2"),
89 IPAddress("192.168.0.4"),
90 ])
91@@ -296,12 +321,12 @@ class TestGetMAASFacingServerAddresses(MAASServerTestCase):
92 self.assertThat(
93 region_ips,
94 Equals([
95- IPAddress("2001:db8::1"),
96 IPAddress("192.168.0.1"),
97- IPAddress("2001:db8::2"),
98- IPAddress("2001:db8::4"),
99+ IPAddress("2001:db8::1"),
100 IPAddress("192.168.0.2"),
101 IPAddress("192.168.0.4"),
102+ IPAddress("2001:db8::2"),
103+ IPAddress("2001:db8::4"),
104 ])
105 )
106

Subscribers

People subscribed via source and target branches