Merge lp:~mpontillo/maas/range-broadcast-overlap-error--bug-1678236 into lp:~maas-committers/maas/trunk
- range-broadcast-overlap-error--bug-1678236
- Merge into trunk
| Status: | Merged |
|---|---|
| Approved by: | Mike Pontillo |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 5910 |
| Proposed branch: | lp:~mpontillo/maas/range-broadcast-overlap-error--bug-1678236 |
| Merge into: | lp:~maas-committers/maas/trunk |
| Diff against target: |
549 lines (+179/-84) 6 files modified
src/maasserver/models/iprange.py (+41/-27) src/maasserver/models/tests/test_iprange.py (+95/-26) src/maasserver/rpc/tests/test_nodes.py (+2/-2) src/maastesting/factory.py (+31/-13) src/maastesting/tests/test_factory.py (+10/-14) src/provisioningserver/rpc/tests/test_clusterservice.py (+0/-2) |
| To merge this branch: | bzr merge lp:~mpontillo/maas/range-broadcast-overlap-error--bug-1678236 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Andres Rodriguez (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Fix error message when broadcast IP or reserved network IP is inside a proposed IP range.
* Remove skipped test; replace with less-random tests.
* Fix factory to no longer pick reserved subnet or broadcast addresses when choosing a random IP address.
* Fix random test failures made more obvious by fixing said factory bug.
* Fix factory to remove unused and buggy but_not feature for creating random IP ranges. (but_not still works for picking an IP in a subnet.)
* Make errors more consistent when a proposed range is not unused.
* Clean up field highlighting when raising errors.
Description of the change
| MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~mpontillo/maas/range-broadcast-overlap-error--bug-1678236 into lp:maas failed. Below is the output from the failed tests.
Hit:1 http://
Get:2 http://
Get:3 http://
Get:4 http://
Fetched 306 kB in 0s (636 kB/s)
Reading package lists...
sudo DEBIAN_
--no-
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubun
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (...
| MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~mpontillo/maas/range-broadcast-overlap-error--bug-1678236 into lp:maas failed. Below is the output from the failed tests.
Hit:1 http://
Get:2 http://
Get:3 http://
Get:4 http://
Fetched 306 kB in 0s (592 kB/s)
Reading package lists...
sudo DEBIAN_
--no-
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubun
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (...
| MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~mpontillo/maas/range-broadcast-overlap-error--bug-1678236 into lp:maas failed. Below is the output from the failed tests.
Hit:1 http://
Get:2 http://
Get:3 http://
Get:4 http://
Get:5 http://
Get:6 http://
Get:7 http://
Fetched 1,078 kB in 0s (2,135 kB/s)
Reading package lists...
sudo DEBIAN_
--no-
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubun
distro-info is already the newest version (0.14build1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is...
Preview Diff
| 1 | === modified file 'src/maasserver/models/iprange.py' |
| 2 | --- src/maasserver/models/iprange.py 2016-10-19 19:20:24 +0000 |
| 3 | +++ src/maasserver/models/iprange.py 2017-03-31 23:23:16 +0000 |
| 4 | @@ -129,6 +129,15 @@ |
| 5 | def __contains__(self, item): |
| 6 | return item in self.netaddr_iprange |
| 7 | |
| 8 | + def _raise_validation_error(self, message, fields=None): |
| 9 | + if fields is None: |
| 10 | + # By default, highlight the start_ip and the end_ip. |
| 11 | + fields = ['start_ip', 'end_ip'] |
| 12 | + validation_errors = {} |
| 13 | + for field in fields: |
| 14 | + validation_errors[field] = [message] |
| 15 | + raise ValidationError(validation_errors) |
| 16 | + |
| 17 | def clean(self): |
| 18 | super().clean() |
| 19 | try: |
| 20 | @@ -141,29 +150,40 @@ |
| 21 | # This validation will be called even if the start_ip or end_ip |
| 22 | # field is missing. So we need to check them again here, before |
| 23 | # proceeding with the validation (and potentially crashing). |
| 24 | - raise ValidationError( |
| 25 | + self._raise_validation_error( |
| 26 | "Start IP address and end IP address are both required.") |
| 27 | if end_ip.version != start_ip.version: |
| 28 | - raise ValidationError( |
| 29 | + self._raise_validation_error( |
| 30 | "Start IP address and end IP address must be in the same " |
| 31 | "address family.") |
| 32 | if end_ip < start_ip: |
| 33 | - raise ValidationError( |
| 34 | - "End IP address must not be less than Start IP address.") |
| 35 | + self._raise_validation_error( |
| 36 | + "End IP address must not be less than Start IP address.", |
| 37 | + fields=['end_ip']) |
| 38 | if self.subnet_id is not None: |
| 39 | cidr = IPNetwork(self.subnet.cidr) |
| 40 | if start_ip not in cidr and end_ip not in cidr: |
| 41 | - raise ValidationError( |
| 42 | + self._raise_validation_error( |
| 43 | "IP addresses must be within subnet: %s." % cidr) |
| 44 | if start_ip not in cidr: |
| 45 | - raise ValidationError( |
| 46 | - "Start IP address must be within subnet: %s." % cidr) |
| 47 | + self._raise_validation_error( |
| 48 | + "Start IP address must be within subnet: %s." % cidr, |
| 49 | + fields=['start_ip']) |
| 50 | if end_ip not in cidr: |
| 51 | - raise ValidationError( |
| 52 | - "End IP address must be within subnet: %s." % cidr) |
| 53 | + self._raise_validation_error( |
| 54 | + "End IP address must be within subnet: %s." % cidr, |
| 55 | + fields=['end_ip']) |
| 56 | + if cidr.network == start_ip: |
| 57 | + self._raise_validation_error( |
| 58 | + "Reserved network address cannot be included in IP range.", |
| 59 | + fields=['start_ip']) |
| 60 | + if cidr.version == 4 and cidr.broadcast == end_ip: |
| 61 | + self._raise_validation_error( |
| 62 | + "Broadcast address cannot be included in IP range.", |
| 63 | + fields=['end_ip']) |
| 64 | if (start_ip.version == 6 and self.type == IPRANGE_TYPE.DYNAMIC and |
| 65 | netaddr.IPRange(start_ip, end_ip).size < 256): |
| 66 | - raise ValidationError( |
| 67 | + self._raise_validation_error( |
| 68 | "IPv6 dynamic range must be at least 256 addresses in size.") |
| 69 | self.clean_prevent_dupes_and_overlaps() |
| 70 | |
| 71 | @@ -184,12 +204,6 @@ |
| 72 | If it will, raise ValidationError. |
| 73 | """ |
| 74 | |
| 75 | - # A range overlap/conflict could be due to any of these fields. |
| 76 | - def fail(message, fields=['start_ip', 'end_ip', 'type']): |
| 77 | - for field in fields: |
| 78 | - validation_errors[field] = [message] |
| 79 | - raise ValidationError(validation_errors) |
| 80 | - |
| 81 | # Check against the valid types before going further, since whether |
| 82 | # or not the range overlaps anything that could cause an error heavily |
| 83 | # depends on its type. |
| 84 | @@ -233,10 +247,16 @@ |
| 85 | else: |
| 86 | unused = self.subnet.get_ipranges_available_for_dynamic_range() |
| 87 | |
| 88 | - validation_errors = {} |
| 89 | if len(unused) == 0: |
| 90 | - fail("There is no room for any %s ranges on this subnet." % ( |
| 91 | - self.type)) |
| 92 | + self._raise_validation_error( |
| 93 | + "There is no room for any %s ranges on this subnet." % ( |
| 94 | + self.type)) |
| 95 | + |
| 96 | + message = "Requested %s range conflicts with an existing " % self.type |
| 97 | + if self.type == IPRANGE_TYPE.RESERVED: |
| 98 | + message += "range." |
| 99 | + else: |
| 100 | + message += "IP address or range." |
| 101 | |
| 102 | # Find unused range for start_ip |
| 103 | for range in unused: |
| 104 | @@ -245,11 +265,5 @@ |
| 105 | # Success, start and end IP are in an unused range. |
| 106 | return |
| 107 | else: |
| 108 | - message = ("Requested %s range conflicts with " |
| 109 | - "an existing ") % (self.type) |
| 110 | - if self.type == IPRANGE_TYPE.RESERVED: |
| 111 | - fail(message + "range.") |
| 112 | - else: |
| 113 | - fail(message + "IP address or range.") |
| 114 | - fail("No %s range can be created at requested start IP." % self.type, |
| 115 | - ['start_ip', 'type']) |
| 116 | + self._raise_validation_error(message) |
| 117 | + self._raise_validation_error(message) |
| 118 | |
| 119 | === modified file 'src/maasserver/models/tests/test_iprange.py' |
| 120 | --- src/maasserver/models/tests/test_iprange.py 2017-03-29 12:44:00 +0000 |
| 121 | +++ src/maasserver/models/tests/test_iprange.py 2017-03-31 23:23:16 +0000 |
| 122 | @@ -6,7 +6,6 @@ |
| 123 | __all__ = [] |
| 124 | |
| 125 | import random |
| 126 | -from unittest import skip |
| 127 | |
| 128 | from django.core.exceptions import ValidationError |
| 129 | from maasserver.enum import ( |
| 130 | @@ -17,6 +16,7 @@ |
| 131 | from maasserver.testing.factory import factory |
| 132 | from maasserver.testing.testcase import MAASServerTestCase |
| 133 | from maasserver.utils.orm import reload_object |
| 134 | +from netaddr import IPNetwork |
| 135 | from testtools import ExpectedException |
| 136 | |
| 137 | |
| 138 | @@ -27,6 +27,13 @@ |
| 139 | dns_servers=[]) |
| 140 | |
| 141 | |
| 142 | +def make_plain_ipv6_subnet(): |
| 143 | + return factory.make_Subnet( |
| 144 | + cidr='2001::/64', |
| 145 | + gateway_ip='2001::1', |
| 146 | + dns_servers=[]) |
| 147 | + |
| 148 | + |
| 149 | class IPRangeTest(MAASServerTestCase): |
| 150 | |
| 151 | def test__create(self): |
| 152 | @@ -135,11 +142,44 @@ |
| 153 | ValidationError, '.*End IP address must not be less than.*'): |
| 154 | iprange.save() |
| 155 | |
| 156 | + def test__requires_end_ip_to_not_be_broadcast(self): |
| 157 | + subnet = make_plain_subnet() |
| 158 | + iprange = IPRange( |
| 159 | + start_ip='192.168.0.254', end_ip='192.168.0.255', |
| 160 | + user=factory.make_User(), subnet=subnet, |
| 161 | + type=IPRANGE_TYPE.RESERVED) |
| 162 | + with ExpectedException( |
| 163 | + ValidationError, |
| 164 | + '.*Broadcast address cannot be included in IP range.*'): |
| 165 | + iprange.save() |
| 166 | + |
| 167 | + def test__requires_start_ip_to_not_be_network(self): |
| 168 | + subnet = make_plain_subnet() |
| 169 | + iprange = IPRange( |
| 170 | + start_ip='192.168.0.0', end_ip='192.168.0.5', |
| 171 | + user=factory.make_User(), subnet=subnet, |
| 172 | + type=IPRANGE_TYPE.RESERVED) |
| 173 | + with ExpectedException( |
| 174 | + ValidationError, |
| 175 | + '.*Reserved network address cannot be included in IP range.*'): |
| 176 | + iprange.save() |
| 177 | + |
| 178 | + def test__requires_start_ip_to_not_be_ipv6_reserved_anycast(self): |
| 179 | + subnet = make_plain_ipv6_subnet() |
| 180 | + iprange = IPRange( |
| 181 | + start_ip='2001::', end_ip='2001::1', |
| 182 | + user=factory.make_User(), subnet=subnet, |
| 183 | + type=IPRANGE_TYPE.RESERVED) |
| 184 | + with ExpectedException( |
| 185 | + ValidationError, |
| 186 | + '.*Reserved network address cannot be included in IP range.*'): |
| 187 | + iprange.save() |
| 188 | + |
| 189 | def test__requires_256_addresses_for_ipv6_dynamic(self): |
| 190 | subnet = factory.make_Subnet( |
| 191 | cidr='2001:db8::/64', gateway_ip='fe80::1', dns_servers=[]) |
| 192 | iprange = IPRange( |
| 193 | - start_ip='2001:db8::', end_ip='2001:db8::fe', |
| 194 | + start_ip='2001:db8::1', end_ip='2001:db8::ff', |
| 195 | user=factory.make_User(), subnet=subnet, |
| 196 | type=IPRANGE_TYPE.DYNAMIC, |
| 197 | comment="This is a comment.") |
| 198 | @@ -176,10 +216,6 @@ |
| 199 | |
| 200 | class TestIPRangeSavePreventsOverlapping(MAASServerTestCase): |
| 201 | |
| 202 | - no_fit = ".*No %s range can be created at requested start IP." |
| 203 | - dynamic_no_fit = no_fit % IPRANGE_TYPE.DYNAMIC |
| 204 | - reserved_no_fit = no_fit % IPRANGE_TYPE.RESERVED |
| 205 | - |
| 206 | overlaps = ".*Requested %s range conflicts with an existing %srange.*" |
| 207 | dynamic_overlaps = overlaps % (IPRANGE_TYPE.DYNAMIC, "IP address or ") |
| 208 | reserved_overlaps = overlaps % (IPRANGE_TYPE.RESERVED, "") |
| 209 | @@ -203,7 +239,7 @@ |
| 210 | start_ip="192.168.0.100", |
| 211 | end_ip="192.168.0.150", |
| 212 | ) |
| 213 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 214 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 215 | iprange.save() |
| 216 | |
| 217 | def test__no_save_range_overlap_begin(self): |
| 218 | @@ -243,7 +279,7 @@ |
| 219 | start_ip="192.168.0.140", |
| 220 | end_ip="192.168.0.160", |
| 221 | ) |
| 222 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 223 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 224 | iprange.save() |
| 225 | |
| 226 | def test__no_save_range_within_ranges(self): |
| 227 | @@ -261,7 +297,7 @@ |
| 228 | start_ip="192.168.0.110", |
| 229 | end_ip="192.168.0.140", |
| 230 | ) |
| 231 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 232 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 233 | iprange.save() |
| 234 | |
| 235 | def test__no_save_range_spanning_existing_range(self): |
| 236 | @@ -297,7 +333,7 @@ |
| 237 | start_ip="192.168.0.110", |
| 238 | end_ip="192.168.0.140", |
| 239 | ) |
| 240 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 241 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 242 | iprange.save() |
| 243 | |
| 244 | def test__no_save_range_within_existing_reserved_range(self): |
| 245 | @@ -315,7 +351,7 @@ |
| 246 | start_ip="192.168.0.110", |
| 247 | end_ip="192.168.0.140", |
| 248 | ) |
| 249 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 250 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 251 | iprange.save() |
| 252 | |
| 253 | def test__no_save_when_no_ranges_available(self): |
| 254 | @@ -376,7 +412,7 @@ |
| 255 | instance_id = iprange.id |
| 256 | iprange.start_ip = "192.168.0.110" |
| 257 | iprange.end_ip = "192.168.0.140" |
| 258 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 259 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 260 | iprange.save() |
| 261 | # Make sure original range isn't deleted after failure to modify. |
| 262 | iprange = reload_object(iprange) |
| 263 | @@ -393,7 +429,7 @@ |
| 264 | iprange.save() |
| 265 | # A DYNAMIC range cannot overlap the gateway IP. |
| 266 | iprange.start_ip = "192.168.0.1" |
| 267 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 268 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 269 | iprange.save() |
| 270 | |
| 271 | def test__reserved_range_can_overlap_gateway_ip(self): |
| 272 | @@ -426,7 +462,7 @@ |
| 273 | start_ip="192.168.0.25", |
| 274 | end_ip="192.168.0.30", |
| 275 | ) |
| 276 | - with ExpectedException(ValidationError, self.reserved_no_fit): |
| 277 | + with ExpectedException(ValidationError, self.reserved_overlaps): |
| 278 | iprange.save() |
| 279 | |
| 280 | def test__reserved_range_cannot_overlap_reserved_ranges(self): |
| 281 | @@ -446,7 +482,7 @@ |
| 282 | start_ip="192.168.0.250", |
| 283 | end_ip="192.168.0.254", |
| 284 | ) |
| 285 | - with ExpectedException(ValidationError, self.reserved_no_fit): |
| 286 | + with ExpectedException(ValidationError, self.reserved_overlaps): |
| 287 | iprange.save() |
| 288 | |
| 289 | def test__reserved_range_can_overlap_most_ip_types(self): |
| 290 | @@ -466,15 +502,48 @@ |
| 291 | ) |
| 292 | iprange.save() |
| 293 | |
| 294 | - @skip("XXX: GavinPanella 2017-03-29 bug=1594146: Fails spuriously.") |
| 295 | - def test__dynamic_range_cannot_overlap_most_ip_types(self): |
| 296 | - subnet = make_plain_subnet() |
| 297 | - factory.make_StaticIPAddress( |
| 298 | - subnet=subnet, |
| 299 | - alloc_type=random.choice(( |
| 300 | - IPADDRESS_TYPE.AUTO, |
| 301 | - IPADDRESS_TYPE.STICKY, |
| 302 | - IPADDRESS_TYPE.USER_RESERVED))) |
| 303 | + def test__dynamic_range_cannot_overlap_auto_address(self): |
| 304 | + subnet = make_plain_subnet() |
| 305 | + factory.make_StaticIPAddress( |
| 306 | + subnet=subnet, |
| 307 | + alloc_type=IPADDRESS_TYPE.AUTO, |
| 308 | + ip=factory.pick_ip_in_network( |
| 309 | + IPNetwork(subnet.cidr), |
| 310 | + but_not=['192.168.0.1'])) |
| 311 | + iprange = IPRange( |
| 312 | + subnet=subnet, |
| 313 | + type=IPRANGE_TYPE.DYNAMIC, |
| 314 | + start_ip="192.168.0.2", |
| 315 | + end_ip="192.168.0.254", |
| 316 | + ) |
| 317 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 318 | + iprange.save() |
| 319 | + |
| 320 | + def test__dynamic_range_cannot_overlap_sticky_address(self): |
| 321 | + subnet = make_plain_subnet() |
| 322 | + factory.make_StaticIPAddress( |
| 323 | + subnet=subnet, |
| 324 | + alloc_type=IPADDRESS_TYPE.STICKY, |
| 325 | + ip=factory.pick_ip_in_network( |
| 326 | + IPNetwork(subnet.cidr), |
| 327 | + but_not=['192.168.0.1'])) |
| 328 | + iprange = IPRange( |
| 329 | + subnet=subnet, |
| 330 | + type=IPRANGE_TYPE.DYNAMIC, |
| 331 | + start_ip="192.168.0.2", |
| 332 | + end_ip="192.168.0.254", |
| 333 | + ) |
| 334 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 335 | + iprange.save() |
| 336 | + |
| 337 | + def test__dynamic_range_cannot_overlap_user_reserved_address(self): |
| 338 | + subnet = make_plain_subnet() |
| 339 | + factory.make_StaticIPAddress( |
| 340 | + subnet=subnet, |
| 341 | + alloc_type=IPADDRESS_TYPE.USER_RESERVED, |
| 342 | + ip=factory.pick_ip_in_network( |
| 343 | + IPNetwork(subnet.cidr), |
| 344 | + but_not=['192.168.0.1'])) |
| 345 | iprange = IPRange( |
| 346 | subnet=subnet, |
| 347 | type=IPRANGE_TYPE.DYNAMIC, |
| 348 | @@ -525,7 +594,7 @@ |
| 349 | start_ip="192.168.0.1", |
| 350 | end_ip="192.168.0.254", |
| 351 | ) |
| 352 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 353 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 354 | iprange.save() |
| 355 | |
| 356 | def test__reserved_range_can_overlap_dns_servers(self): |
| 357 | @@ -554,7 +623,7 @@ |
| 358 | |
| 359 | # Dynamic should not save overlapping gateway IP. |
| 360 | iprange.type = IPRANGE_TYPE.DYNAMIC |
| 361 | - with ExpectedException(ValidationError, self.dynamic_no_fit): |
| 362 | + with ExpectedException(ValidationError, self.dynamic_overlaps): |
| 363 | iprange.save() |
| 364 | # Fix start_ip and now it should save. |
| 365 | iprange.start_ip = "192.168.0.2" |
| 366 | |
| 367 | === modified file 'src/maasserver/rpc/tests/test_nodes.py' |
| 368 | --- src/maasserver/rpc/tests/test_nodes.py 2017-01-28 00:51:47 +0000 |
| 369 | +++ src/maasserver/rpc/tests/test_nodes.py 2017-03-31 23:23:16 +0000 |
| 370 | @@ -370,10 +370,10 @@ |
| 371 | # Accessible nodes. |
| 372 | node_ids = [ |
| 373 | self.make_Node(bmc_connected_to=rack).system_id |
| 374 | - for _ in range(5) |
| 375 | + for _ in range(3) |
| 376 | ] |
| 377 | # Inaccessible nodes. |
| 378 | - for _ in range(5): |
| 379 | + for _ in range(3): |
| 380 | node = self.make_Node(bmc_connected_to=rack) |
| 381 | node.bmc = None |
| 382 | node.save() |
| 383 | |
| 384 | === modified file 'src/maastesting/factory.py' |
| 385 | --- src/maastesting/factory.py 2016-10-05 01:31:22 +0000 |
| 386 | +++ src/maastesting/factory.py 2017-03-31 23:23:16 +0000 |
| 387 | @@ -371,37 +371,55 @@ |
| 388 | "Could not find available IP in static range") |
| 389 | |
| 390 | def pick_ip_in_network(self, network, *, but_not=EMPTY_SET): |
| 391 | - but_not = {IPAddress(but) for but in but_not if but is not None} |
| 392 | - if network.version == 6: |
| 393 | + but_not = { |
| 394 | + IPAddress(but) for but in but_not |
| 395 | + if but is not None and IPAddress(but) in network |
| 396 | + } |
| 397 | + # Unless the prefix length is very small, make sure we don't select |
| 398 | + # a normally-unusable IP address. |
| 399 | + if network.version == 6 and network.prefixlen < 127: |
| 400 | + # Don't pick the all-zeroes address, since it has special meaning |
| 401 | + # in IPv6 as the subnet-router anycast address. IPv6 does not have |
| 402 | + # a broadcast address, though. |
| 403 | first, last = network.first + 1, network.last |
| 404 | + network_size = network.size - 1 |
| 405 | + elif network.prefixlen < 31: |
| 406 | + # Don't pick broadcast or network addresses. |
| 407 | + first, last = network.first + 1, network.last - 1 |
| 408 | + network_size = network.size - 2 |
| 409 | else: |
| 410 | first, last = network.first, network.last |
| 411 | + network_size = network.size |
| 412 | + if len(but_not) == network_size: |
| 413 | + raise ValueError( |
| 414 | + "No IP addresses available in network: %s (but_not=%r)" % ( |
| 415 | + network, but_not)) |
| 416 | for _ in range(100): |
| 417 | address = IPAddress(random.randint(first, last)) |
| 418 | if address not in but_not: |
| 419 | return str(address) |
| 420 | - raise TooManyRandomRetries("Could not find available IP in network") |
| 421 | + raise TooManyRandomRetries( |
| 422 | + "Could not find available IP in network: %s (but_not=%r)" % ( |
| 423 | + network, but_not)) |
| 424 | |
| 425 | - def make_ip_range(self, network, *, but_not=None): |
| 426 | + def make_ip_range(self, network): |
| 427 | """Return a pair of IP addresses from the given network. |
| 428 | |
| 429 | :param network: Return IP addresses within this network. |
| 430 | :param but_not: A pair of addresses that should not be returned. |
| 431 | :return: A pair of `IPAddress`. |
| 432 | """ |
| 433 | - if but_not is not None: |
| 434 | - low, high = but_not |
| 435 | - but_not = IPAddress(low), IPAddress(high) |
| 436 | for _ in range(100): |
| 437 | ip_range = tuple(sorted( |
| 438 | IPAddress(factory.pick_ip_in_network(network)) |
| 439 | for _ in range(2) |
| 440 | )) |
| 441 | - if ip_range[0] < ip_range[1] and ip_range != but_not: |
| 442 | + if ip_range[0] < ip_range[1]: |
| 443 | return ip_range |
| 444 | - raise TooManyRandomRetries("Could not find available IP range") |
| 445 | + raise TooManyRandomRetries( |
| 446 | + "Could not find available IP range in network: %s" % network) |
| 447 | |
| 448 | - def make_ipv4_range(self, network=None, *, but_not=None): |
| 449 | + def make_ipv4_range(self, network=None): |
| 450 | """Return a pair of IPv4 addresses. |
| 451 | |
| 452 | :param network: Return IP addresses within this network. |
| 453 | @@ -410,9 +428,9 @@ |
| 454 | """ |
| 455 | if network is None: |
| 456 | network = self.make_ipv4_network() |
| 457 | - return self.make_ip_range(network=network, but_not=but_not) |
| 458 | + return self.make_ip_range(network=network) |
| 459 | |
| 460 | - def make_ipv6_range(self, network=None, *, but_not=None): |
| 461 | + def make_ipv6_range(self, network=None): |
| 462 | """Return a pair of IPv6 addresses. |
| 463 | |
| 464 | :param network: Return IP addresses within this network. |
| 465 | @@ -421,7 +439,7 @@ |
| 466 | """ |
| 467 | if network is None: |
| 468 | network = self.make_ipv6_network() |
| 469 | - return self.make_ip_range(network=network, but_not=but_not) |
| 470 | + return self.make_ip_range(network=network) |
| 471 | |
| 472 | def make_mac_address(self, delimiter=":"): |
| 473 | assert isinstance(delimiter, str) |
| 474 | |
| 475 | === modified file 'src/maastesting/tests/test_factory.py' |
| 476 | --- src/maastesting/tests/test_factory.py 2017-01-28 00:51:47 +0000 |
| 477 | +++ src/maastesting/tests/test_factory.py 2017-03-31 23:23:16 +0000 |
| 478 | @@ -153,12 +153,18 @@ |
| 479 | self.assertNotIn(new_network, existing_network) |
| 480 | self.assertNotIn(existing_network, new_network) |
| 481 | |
| 482 | - def test_pick_ip_in_network_for_ipv4(self): |
| 483 | + def test_pick_ip_in_network_for_ipv4_slash_31(self): |
| 484 | network = factory.make_ipv4_network(slash=31) |
| 485 | ip = factory.pick_ip_in_network(network) |
| 486 | self.assertTrue( |
| 487 | network.first <= IPAddress(ip).value <= network.last) |
| 488 | |
| 489 | + def test_pick_ip_in_network_for_ipv4_slash_30(self): |
| 490 | + network = factory.make_ipv4_network(slash=30) |
| 491 | + ip = factory.pick_ip_in_network(network) |
| 492 | + self.assertTrue( |
| 493 | + network.first < IPAddress(ip).value < network.last) |
| 494 | + |
| 495 | def test_pick_ip_in_network_for_ipv6(self): |
| 496 | # For IPv6, pick_ip_in_network will not consider the very first |
| 497 | # address in a network because this is reserved for routers. |
| 498 | @@ -392,28 +398,19 @@ |
| 499 | self.make_network(slash=(31 if self.version == 4 else 126))) |
| 500 | self.assertLess(low, high) |
| 501 | |
| 502 | - def test_make_ip_range_obeys_but_not(self): |
| 503 | - # Make a very very small network, to maximise the chances of exposure |
| 504 | - # if the method gets this wrong. |
| 505 | - network = self.make_network(slash=(30 if self.version == 4 else 126)) |
| 506 | - first_low, first_high = factory.make_ip_range(network) |
| 507 | - second_low, second_high = factory.make_ip_range( |
| 508 | - network, but_not=(first_low, first_high)) |
| 509 | - self.assertNotEqual((first_low, first_high), (second_low, second_high)) |
| 510 | - |
| 511 | def test_make_ipvN_range_calls_make_ip_range(self): |
| 512 | self.patch_autospec(factory, "make_ip_range") |
| 513 | factory.make_ip_range.return_value = sentinel.ip_range |
| 514 | network = self.make_network() |
| 515 | - ip_range = self.make_range(network, but_not=sentinel.but_not) |
| 516 | + ip_range = self.make_range(network) |
| 517 | self.assertThat(ip_range, Is(sentinel.ip_range)) |
| 518 | self.assertThat(factory.make_ip_range, MockCalledOnceWith( |
| 519 | - network=network, but_not=sentinel.but_not)) |
| 520 | + network=network)) |
| 521 | |
| 522 | def test_make_ipvN_range_creates_random_network_if_not_supplied(self): |
| 523 | self.patch_autospec(factory, "make_ip_range") |
| 524 | factory.make_ip_range.return_value = sentinel.ip_range |
| 525 | - ip_range = self.make_range(but_not=sentinel.but_not) |
| 526 | + ip_range = self.make_range() |
| 527 | self.assertThat(ip_range, Is(sentinel.ip_range)) |
| 528 | self.assertThat( |
| 529 | factory.make_ip_range, |
| 530 | @@ -425,6 +422,5 @@ |
| 531 | first_only=True, |
| 532 | ), |
| 533 | ), |
| 534 | - but_not=sentinel.but_not, |
| 535 | ), |
| 536 | ) |
| 537 | |
| 538 | === modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py' |
| 539 | --- src/provisioningserver/rpc/tests/test_clusterservice.py 2017-03-30 01:00:53 +0000 |
| 540 | +++ src/provisioningserver/rpc/tests/test_clusterservice.py 2017-03-31 23:23:16 +0000 |
| 541 | @@ -1968,8 +1968,6 @@ |
| 542 | configure.side_effect = ( |
| 543 | exceptions.CannotConfigureDHCP("Deliberate failure")) |
| 544 | omapi_key = factory.make_name('key') |
| 545 | - network = self.make_network() |
| 546 | - ip_low, ip_high = factory.make_ip_range(network) |
| 547 | failover_peers = [make_failover_peer_config()] |
| 548 | shared_networks = [self.make_shared_network()] |
| 549 | shared_networks = fix_shared_networks_failover( |
lgtm!