MAAS

Merge lp:~gmb/maas/static-ip-sanity-checks into lp:~maas-committers/maas/trunk

  1. static-ip-sanity-checks
  2. Merge into trunk
Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 2528
Proposed branch: lp:~gmb/maas/static-ip-sanity-checks
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 114 lines (+30/-16)
2 files modified
src/maasserver/forms.py (+5/-4)
src/maasserver/tests/test_forms.py (+25/-12)
To merge this branch: bzr merge lp:~gmb/maas/static-ip-sanity-checks
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+226074@code.launchpad.net

Commit message

Fix validate_new_static_ip_ranges() to allow for the new upper and lower bounds for a static range being already-allocated. Previously, this raised a validation error, despite being a perfectly valid scenario.

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

Cool :)

Although if you can find an actual assertion to check I'd be happier.

review: Needs Fixing
Revision history for this message
Graham Binns (gmb) wrote :

On 9 July 2014 09:30, Julian Edwards <email address hidden> wrote:
> Although if you can find an actual assertion to check I'd be happier.

I've changed the function to return True when it's happy. Please to
approve of me.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Shit I had meant to approve that, sorry!

review: Approve
Revision history for this message
Julian Edwards (julian-edwards) wrote :

But anyway that is indeed much better!

Revision history for this message
Graham Binns (gmb) wrote :

I could put some daddy-issues joke about seeking approval here but… nah. Ta!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/forms.py'
2--- src/maasserver/forms.py 2014-07-08 11:57:31 +0000
3+++ src/maasserver/forms.py 2014-07-09 08:45:48 +0000
4@@ -1081,14 +1081,14 @@
5 # Return early if the instance is not already managed, it currently
6 # has no static IP range, or the static IP range hasn't changed.
7 if not instance.is_managed:
8- return
9+ return True
10 # Deliberately vague check to allow for empty strings.
11 if (not instance.static_ip_range_low or
12 not instance.static_ip_range_high):
13- return
14+ return True
15 if (static_ip_range_low == instance.static_ip_range_low and
16 static_ip_range_high == instance.static_ip_range_high):
17- return
18+ return True
19
20 cursor = connection.cursor()
21
22@@ -1101,7 +1101,7 @@
23 cursor.execute("""
24 SELECT TRUE FROM maasserver_staticipaddress
25 WHERE ip >= %s AND ip <= %s
26- AND (ip <= %s OR ip >= %s)
27+ AND (ip < %s OR ip > %s)
28 """, (
29 instance.static_ip_range_low,
30 instance.static_ip_range_high,
31@@ -1124,6 +1124,7 @@
32 if any(results):
33 raise forms.ValidationError(
34 ERROR_MESSAGE_STATIC_RANGE_IN_USE)
35+ return True
36
37
38 class NodeGroupInterfaceForm(ModelForm):
39
40=== modified file 'src/maasserver/tests/test_forms.py'
41--- src/maasserver/tests/test_forms.py 2014-07-08 11:57:31 +0000
42+++ src/maasserver/tests/test_forms.py 2014-07-09 08:45:48 +0000
43@@ -1143,11 +1143,27 @@
44 def test_allows_range_expansion(self):
45 interface = self.make_interface()
46 StaticIPAddress.objects.allocate_new('10.1.0.56', '10.1.0.60')
47- validate_new_static_ip_ranges(
48+ is_valid = validate_new_static_ip_ranges(
49 interface, static_ip_range_low='10.1.0.40',
50 static_ip_range_high='10.1.0.100')
51- # Success is getting here without error.
52- pass
53+ self.assertTrue(is_valid)
54+
55+ def test_allows_allocated_ip_as_upper_bound(self):
56+ interface = self.make_interface()
57+ StaticIPAddress.objects.allocate_new('10.1.0.55', '10.1.0.55')
58+ is_valid = validate_new_static_ip_ranges(
59+ interface,
60+ static_ip_range_low=interface.static_ip_range_low,
61+ static_ip_range_high='10.1.0.55')
62+ self.assertTrue(is_valid)
63+
64+ def test_allows_allocated_ip_as_lower_bound(self):
65+ interface = self.make_interface()
66+ StaticIPAddress.objects.allocate_new('10.1.0.55', '10.1.0.55')
67+ is_valid = validate_new_static_ip_ranges(
68+ interface, static_ip_range_low='10.1.0.55',
69+ static_ip_range_high=interface.static_ip_range_high)
70+ self.assertTrue(is_valid)
71
72 def test_ignores_unmanaged_interfaces(self):
73 interface = self.make_interface()
74@@ -1155,11 +1171,10 @@
75 interface.save()
76 StaticIPAddress.objects.allocate_new(
77 interface.static_ip_range_low, interface.static_ip_range_high)
78- validate_new_static_ip_ranges(
79+ is_valid = validate_new_static_ip_ranges(
80 interface, static_ip_range_low='10.1.0.57',
81 static_ip_range_high='10.1.0.58')
82- # Success is getting here without error.
83- pass
84+ self.assertTrue(is_valid)
85
86 def test_ignores_interfaces_with_no_static_range(self):
87 interface = self.make_interface()
88@@ -1167,22 +1182,20 @@
89 interface.static_ip_range_high = None
90 interface.save()
91 StaticIPAddress.objects.allocate_new('10.1.0.56', '10.1.0.60')
92- validate_new_static_ip_ranges(
93+ is_valid = validate_new_static_ip_ranges(
94 interface, static_ip_range_low='10.1.0.57',
95 static_ip_range_high='10.1.0.58')
96- # Success is getting here without error.
97- pass
98+ self.assertTrue(is_valid)
99
100 def test_ignores_unchanged_static_range(self):
101 interface = self.make_interface()
102 StaticIPAddress.objects.allocate_new(
103 interface.static_ip_range_low, interface.static_ip_range_high)
104- validate_new_static_ip_ranges(
105+ is_valid = validate_new_static_ip_ranges(
106 interface,
107 static_ip_range_low=interface.static_ip_range_low,
108 static_ip_range_high=interface.static_ip_range_high)
109- # Success is getting here without error.
110- pass
111+ self.assertTrue(is_valid)
112
113
114 class TestNodeGroupInterfaceForeignDHCPForm(MAASServerTestCase):