Merge lp:~mpontillo/maas/fix-random-failure--bug-1683503 into lp:~maas-committers/maas/trunk

Proposed by Mike Pontillo
Status: Merged
Approved by: Mike Pontillo
Approved revision: no longer in the source branch.
Merged at revision: 5991
Proposed branch: lp:~mpontillo/maas/fix-random-failure--bug-1683503
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 85 lines (+36/-6)
2 files modified
src/provisioningserver/utils/network.py (+11/-6)
src/provisioningserver/utils/tests/test_network.py (+25/-0)
To merge this branch: bzr merge lp:~mpontillo/maas/fix-random-failure--bug-1683503
Reviewer Review Type Date Requested Status
Lee Trager (community) Approve
Review via email: mp+322651@code.launchpad.net

Commit message

Fix dynamic range suggestion code to prevent suggesting a very small dynamic range.

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/provisioningserver/utils/network.py'
--- src/provisioningserver/utils/network.py 2016-12-07 12:46:14 +0000
+++ src/provisioningserver/utils/network.py 2017-04-17 23:20:00 +0000
@@ -267,6 +267,8 @@
267 suggested_gateway = None267 suggested_gateway = None
268 first_address = self.first_address_value268 first_address = self.first_address_value
269 last_address = self.last_address_value269 last_address = self.last_address_value
270 if self.ip_version == 6 and self.total_addresses <= 2:
271 return None
270 if self.ip_version == 6:272 if self.ip_version == 6:
271 # For IPv6 addresses, always return the subnet-router anycast273 # For IPv6 addresses, always return the subnet-router anycast
272 # address. (See RFC 4291 section 2.6.1 for more information.)274 # address. (See RFC 4291 section 2.6.1 for more information.)
@@ -315,20 +317,23 @@
315 candidate.first, candidate.last - 1,317 candidate.first, candidate.last - 1,
316 purpose=IPRANGE_TYPE.PROPOSED_DYNAMIC)318 purpose=IPRANGE_TYPE.PROPOSED_DYNAMIC)
317 if candidate is not None:319 if candidate is not None:
320 first = candidate.first
318 one_fourth_range = self.total_addresses >> 2321 one_fourth_range = self.total_addresses >> 2
319 half_remaining_space = self.num_available >> 1322 half_remaining_space = self.num_available >> 1
320 if candidate.size > one_fourth_range:323 if candidate.size > one_fourth_range:
321 # Prevent the proposed range from taking up too much available324 # Prevent the proposed range from taking up too much available
322 # space in the subnet.325 # space in the subnet.
323 candidate = MAASIPRange(326 first = candidate.last - one_fourth_range
324 candidate.last - one_fourth_range, candidate.last,
325 purpose=IPRANGE_TYPE.PROPOSED_DYNAMIC)
326 elif candidate.size >= half_remaining_space:327 elif candidate.size >= half_remaining_space:
327 # Prevent the proposed range from taking up the remainder of328 # Prevent the proposed range from taking up the remainder of
328 # the available IP addresses. (take at most half.)329 # the available IP addresses. (take at most half.)
329 candidate = MAASIPRange(330 first = candidate.last - half_remaining_space + 1
330 candidate.last - half_remaining_space + 1, candidate.last,331 if first >= candidate.last:
331 purpose=IPRANGE_TYPE.PROPOSED_DYNAMIC)332 # Calculated an impossible range.
333 return None
334 candidate = MAASIPRange(
335 first, candidate.last,
336 purpose=IPRANGE_TYPE.PROPOSED_DYNAMIC)
332 return candidate337 return candidate
333338
334 @property339 @property
335340
=== modified file 'src/provisioningserver/utils/tests/test_network.py'
--- src/provisioningserver/utils/tests/test_network.py 2017-04-04 07:42:31 +0000
+++ src/provisioningserver/utils/tests/test_network.py 2017-04-17 23:20:00 +0000
@@ -825,6 +825,12 @@
825 self.assertThat(u['10.0.0.2'].purpose, Not(Contains('unused')))825 self.assertThat(u['10.0.0.2'].purpose, Not(Contains('unused')))
826 self.assertThat(u['10.0.0.254'].purpose, Contains('unused'))826 self.assertThat(u['10.0.0.254'].purpose, Contains('unused'))
827827
828 def test__calculates_full_range_for_small_ipv6(self):
829 s = MAASIPSet([])
830 u = s.get_full_range('2001:db8::/127')
831 self.assertThat(u['2001:db8::'].purpose, Contains('unused'))
832 self.assertThat(u['2001:db8::1'].purpose, Contains('unused'))
833
828 def test__supports_ior(self):834 def test__supports_ior(self):
829 s1 = MAASIPSet(['10.0.0.2', '10.0.0.4', '10.0.0.6', '10.0.0.8'])835 s1 = MAASIPSet(['10.0.0.2', '10.0.0.4', '10.0.0.6', '10.0.0.8'])
830 s2 = MAASIPSet(['10.0.0.1', '10.0.0.3', '10.0.0.5', '10.0.0.7'])836 s2 = MAASIPSet(['10.0.0.1', '10.0.0.3', '10.0.0.5', '10.0.0.7'])
@@ -1082,6 +1088,25 @@
1082 stats.suggested_dynamic_range, Not(Contains(1088 stats.suggested_dynamic_range, Not(Contains(
1083 "2001:db8::bfff:ffff:ffff:ffff")))1089 "2001:db8::bfff:ffff:ffff:ffff")))
10841090
1091 def test__no_suggestion_for_small_ipv6_slash_126(self):
1092 s = MAASIPSet([])
1093 u = s.get_full_range('2001:db8::/126')
1094 stats = IPRangeStatistics(u)
1095 self.assertThat(stats.suggested_gateway, Equals("2001:db8::"))
1096 self.assertThat(stats.suggested_dynamic_range, Is(None))
1097
1098 def test__no_suggestion_for_small_ipv6_slash_127(self):
1099 s = MAASIPSet([])
1100 u = s.get_full_range('2001:db8::/127')
1101 stats = IPRangeStatistics(u)
1102 self.assertThat(stats.suggested_gateway, Equals(None))
1103
1104 def test__no_suggestion_and_no_gateway_for_small_ipv6_slash_128(self):
1105 s = MAASIPSet([])
1106 u = s.get_full_range('2001:db8::/128')
1107 stats = IPRangeStatistics(u)
1108 self.assertThat(stats.suggested_gateway, Is(None))
1109
1085 def test__suggests_half_available_for_ipv6(self):1110 def test__suggests_half_available_for_ipv6(self):
1086 s = MAASIPSet([MAASIPRange(1111 s = MAASIPSet([MAASIPRange(
1087 "2001:db8::1", "2001:db8::ffff:ffff:ffff:ff00")])1112 "2001:db8::1", "2001:db8::ffff:ffff:ffff:ff00")])