Merge lp:~mpontillo/maas/auto-assigned-gateway-ip--1.9--bug-1690231 into lp:maas/1.9

Proposed by Mike Pontillo on 2017-05-15
Status: Merged
Approved by: Mike Pontillo on 2017-05-16
Approved revision: 4600
Merged at revision: 4600
Proposed branch: lp:~mpontillo/maas/auto-assigned-gateway-ip--1.9--bug-1690231
Merge into: lp:maas/1.9
Diff against target: 99 lines (+32/-3)
3 files modified
src/maasserver/models/interface.py (+14/-2)
src/maasserver/models/staticipaddress.py (+7/-1)
src/maasserver/models/tests/test_interface.py (+11/-0)
To merge this branch: bzr merge lp:~mpontillo/maas/auto-assigned-gateway-ip--1.9--bug-1690231
Reviewer Review Type Date Requested Status
Blake Rouse (community) 2017-05-15 Approve on 2017-05-16
Review via email: mp+324080@code.launchpad.net

Commit message

Ensure the default gateway for a subnet is excluded from automatic static IP address allocation.

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Looks good.

review: Approve
MAAS Lander (maas-lander) wrote :
Download full text (1.2 MiB)

The attempt to merge lp:~mpontillo/maas/auto-assigned-gateway-ip--1.9--bug-1690231 into lp:maas/1.9 failed. Below is the output from the failed tests.

Ign http://prodstack-zone-2.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates InRelease [65.9 kB]
Get:2 http://security.ubuntu.com trusty-security InRelease [65.9 kB]
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [131 kB]
Get:4 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main Sources [398 kB]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [52.5 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [614 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted Sources [6,327 B]
Get:8 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe Sources [178 kB]
Get:9 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [7,767 B]
Get:10 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [981 kB]
Get:11 http://security.ubuntu.com trusty-security/universe amd64 Packages [157 kB]
Get:12 http://security.ubuntu.com trusty-security/main Translation-en [334 kB]
Get:13 http://security.ubuntu.com trusty-security/universe Translation-en [91.4 kB]
Get:14 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [17.1 kB]
Get:15 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [405 kB]
Get:16 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [14.3 kB]
Get:17 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/main Translation-en [485 kB]
Get:18 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/multiverse Translation-en [7,430 B]
Get:19 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/restricted Translation-en [3,975 B]
Get:20 http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-updates/universe Translation-en [215 kB]
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backports/main Translation-en
Hit http://prodstack-zone-2.clouds.archive.ubuntu.com trusty-backp...

Mike Pontillo (mpontillo) wrote :

Not sure why this one failed. I don't see any errors in the output from the lander. Manually re-checking.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/interface.py'
2--- src/maasserver/models/interface.py 2016-04-05 15:38:30 +0000
3+++ src/maasserver/models/interface.py 2017-05-15 22:56:53 +0000
4@@ -1116,7 +1116,7 @@
5 alloc_type=IPADDRESS_TYPE.DISCOVERED):
6 self.unlink_ip_address(ip_address, clearing_config=clearing_config)
7
8- def claim_auto_ips(self, exclude_addresses=[]):
9+ def claim_auto_ips(self, exclude_addresses=None):
10 """Claim IP addresses for this interfaces AUTO IP addresses.
11
12 :param exclude_addresses: Exclude the following IP addresses in the
13@@ -1124,6 +1124,8 @@
14 runs to identify available IP address does not include the already
15 allocated IP addresses.
16 """
17+ if exclude_addresses is None:
18+ exclude_addresses = set()
19 exclude_addresses = set(exclude_addresses)
20 affected_nodegroups = set()
21 assigned_addresses = []
22@@ -1140,11 +1142,16 @@
23 self._update_dns_zones(affected_nodegroups)
24 return assigned_addresses
25
26- def _claim_auto_ip(self, auto_ip, exclude_addresses=[]):
27+ def _claim_auto_ip(self, auto_ip, exclude_addresses=None):
28 """Claim an IP address for the `auto_ip`.
29
30 :returns:NodeGroupInterface, new_ip_address
31 """
32+ if exclude_addresses is None:
33+ exclude_addresses = set()
34+ else:
35+ exclude_addresses = set(exclude_addresses)
36+
37 # Check if already has a hostmap allocated for this MAC address.
38 subnet = auto_ip.subnet
39 if subnet is None:
40@@ -1203,6 +1210,11 @@
41 static_ip_range_low, static_ip_range_high = (
42 get_first_and_last_usable_host_in_network(network))
43 in_use_ipset = subnet.get_ipranges_in_use()
44+
45+ # Make sure we don't step on the gateway IP on the subnet.
46+ if subnet is not None and subnet.gateway_ip is not None:
47+ exclude_addresses.add(unicode(subnet.gateway_ip))
48+
49 new_ip = StaticIPAddress.objects.allocate_new(
50 network, static_ip_range_low, static_ip_range_high,
51 None, None, alloc_type=IPADDRESS_TYPE.AUTO,
52
53=== modified file 'src/maasserver/models/staticipaddress.py'
54--- src/maasserver/models/staticipaddress.py 2016-05-26 17:51:44 +0000
55+++ src/maasserver/models/staticipaddress.py 2017-05-15 22:56:53 +0000
56@@ -166,7 +166,7 @@
57 dynamic_range_low, dynamic_range_high,
58 alloc_type=IPADDRESS_TYPE.AUTO, user=None,
59 requested_address=None, hostname=None, subnet=None,
60- exclude_addresses=[], in_use_ipset=set()):
61+ exclude_addresses=None, in_use_ipset=None):
62 """Return a new StaticIPAddress.
63
64 :param network: The network the address should be allocated in.
65@@ -194,6 +194,12 @@
66 Note that this method has been designed to work even when the database
67 is running with READ COMMITTED isolation. Try to keep it that way.
68 """
69+ # Set multable default parameters.
70+ if exclude_addresses is None:
71+ exclude_addresses = set()
72+ if in_use_ipset is None:
73+ in_use_ipset = set()
74+
75 # This check for `alloc_type` is important for later on. We rely on
76 # detecting IntegrityError as a sign than an IP address is already
77 # taken, and so we must first eliminate all other possible causes.
78
79=== modified file 'src/maasserver/models/tests/test_interface.py'
80--- src/maasserver/models/tests/test_interface.py 2016-04-05 15:38:30 +0000
81+++ src/maasserver/models/tests/test_interface.py 2017-05-15 22:56:53 +0000
82@@ -2469,6 +2469,17 @@
83 auto_ip = interface.ip_addresses.get(alloc_type=IPADDRESS_TYPE.AUTO)
84 self.assertEquals(IPAddress(exclude) + 1, IPAddress(auto_ip.ip))
85
86+ def test__excludes_default_gateway(self):
87+ interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
88+ subnet = factory.make_Subnet(
89+ vlan=interface.vlan, cidr='10.0.0.0/24', gateway_ip='10.0.0.1')
90+ factory.make_StaticIPAddress(
91+ alloc_type=IPADDRESS_TYPE.AUTO, ip="",
92+ subnet=subnet, interface=interface)
93+ interface.claim_auto_ips()
94+ auto_ip = interface.ip_addresses.get(alloc_type=IPADDRESS_TYPE.AUTO)
95+ self.assertEquals(IPAddress('10.0.0.1') + 1, IPAddress(auto_ip.ip))
96+
97 def test__can_acquire_multiple_address_from_the_same_subnet(self):
98 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
99 subnet = factory.make_Subnet(vlan=interface.vlan)

Subscribers

People subscribed via source and target branches