Merge lp:~blake-rouse/maas/fix-1495775 into lp:maas/trunk

Proposed by Blake Rouse on 2015-09-15
Status: Merged
Approved by: Blake Rouse on 2015-09-15
Approved revision: 4262
Merged at revision: 4263
Proposed branch: lp:~blake-rouse/maas/fix-1495775
Merge into: lp:maas/trunk
Diff against target: 281 lines (+110/-11)
11 files modified
src/maasserver/fields.py (+10/-0)
src/maasserver/forms_interface_link.py (+2/-1)
src/maasserver/models/interface.py (+37/-3)
src/maasserver/models/signals/dhcp.py (+1/-1)
src/maasserver/models/signals/dns.py (+1/-1)
src/maasserver/models/signals/events.py (+1/-1)
src/maasserver/models/signals/monitors.py (+1/-1)
src/maasserver/models/signals/power.py (+1/-1)
src/maasserver/models/tests/test_interface.py (+54/-0)
src/maasserver/tests/test_forms_interface_link.py (+1/-1)
src/maasserver/utils/tests/test_signals.py (+1/-1)
To merge this branch: bzr merge lp:~blake-rouse/maas/fix-1495775
Reviewer Review Type Date Requested Status
Andres Rodriguez 2015-09-15 Approve on 2015-09-15
Review via email: mp+271133@code.launchpad.net

Commit message

Fix the link-subnet to allow mode to be case-insensitive. Fix disabling and enabling an interface where the links are cleaned correctly.

To post a comment you must log in.
Andres Rodriguez (andreserl) wrote :

lgtm!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/fields.py'
2--- src/maasserver/fields.py 2015-07-06 08:52:20 +0000
3+++ src/maasserver/fields.py 2015-09-15 14:16:03 +0000
4@@ -44,6 +44,7 @@
5 from django.db.models.fields.subclassing import Creator
6 from django.forms import (
7 CharField,
8+ ChoiceField,
9 ModelChoiceField,
10 )
11 from django.utils.encoding import force_text
12@@ -600,3 +601,12 @@
13 "Invalid IP address: %s; provide a list of "
14 "space-separated IP addresses" % ip)
15 return ' '.join(ips)
16+
17+
18+class CaseInsensitiveChoiceField(ChoiceField):
19+ """ChoiceField that allows the input to be case insensitive."""
20+
21+ def to_python(self, value):
22+ if value not in self.empty_values:
23+ value = value.lower()
24+ return super(CaseInsensitiveChoiceField, self).to_python(value)
25
26=== modified file 'src/maasserver/forms_interface_link.py'
27--- src/maasserver/forms_interface_link.py 2015-09-03 16:23:08 +0000
28+++ src/maasserver/forms_interface_link.py 2015-09-15 14:16:03 +0000
29@@ -22,6 +22,7 @@
30 INTERFACE_LINK_TYPE_CHOICES,
31 IPADDRESS_TYPE,
32 )
33+from maasserver.fields import CaseInsensitiveChoiceField
34 from maasserver.models.interface import Interface
35 from maasserver.utils.forms import (
36 compose_invalid_choice_text,
37@@ -34,7 +35,7 @@
38 class InterfaceLinkForm(forms.Form):
39 """Interface link form."""
40
41- mode = forms.ChoiceField(
42+ mode = CaseInsensitiveChoiceField(
43 choices=INTERFACE_LINK_TYPE_CHOICES, required=True,
44 error_messages={
45 'invalid_choice': compose_invalid_choice_text(
46
47=== modified file 'src/maasserver/models/interface.py'
48--- src/maasserver/models/interface.py 2015-09-14 19:31:45 +0000
49+++ src/maasserver/models/interface.py 2015-09-15 14:16:03 +0000
50@@ -57,6 +57,7 @@
51 VerboseRegexValidator,
52 MACAddressField,
53 )
54+from maasserver.utils.signals import connect_to_field_change
55 from maasserver.clusterrpc.dhcp import update_host_maps
56 from maasserver.models.staticipaddress import StaticIPAddress
57 from maasserver.models.cleansave import CleanSave
58@@ -235,6 +236,9 @@
59 def get_name(self):
60 return self.name
61
62+ def is_enabled(self):
63+ return self.enabled
64+
65 def get_links(self):
66 """Return the definition of links connected to this interface.
67
68@@ -1115,9 +1119,6 @@
69 def get_type(self):
70 return INTERFACE_TYPE.PHYSICAL
71
72- def is_enabled(self):
73- return self.enabled
74-
75 def clean(self):
76 super(PhysicalInterface, self).clean()
77 # Node and MAC address is always required for a physical interface.
78@@ -1151,6 +1152,39 @@
79 })
80
81
82+def interface_enabled_or_disabled(instance, old_values, **kwargs):
83+ """When an interface is enabled be sure at minimum a LINK_UP is created.
84+ When an interface is disabled make sure that all its links are removed,
85+ even for all its children that are now disabled."""
86+ if instance.type != INTERFACE_TYPE.PHYSICAL:
87+ return
88+ if instance.is_enabled():
89+ # Make sure it has a LINK_UP link, and for its children.
90+ instance.ensure_link_up()
91+ for rel in instance.children_relationships.all():
92+ rel.child.ensure_link_up()
93+ else:
94+ # Was disabled. Remove the links.
95+ for ip_address in instance.ip_addresses.exclude(
96+ alloc_type=IPADDRESS_TYPE.DISCOVERED):
97+ instance.unlink_ip_address(ip_address, clearing_config=True)
98+ # If any of the children of this interface are now disabled, all of
99+ # their links need to be removed as well.
100+ for rel in instance.children_relationships.all():
101+ if not rel.child.is_enabled():
102+ for ip_address in rel.child.ip_addresses.all():
103+ rel.child.unlink_ip_address(
104+ ip_address, clearing_config=True)
105+
106+
107+connect_to_field_change(
108+ interface_enabled_or_disabled,
109+ Interface, ['enabled'], delete=False)
110+connect_to_field_change(
111+ interface_enabled_or_disabled,
112+ PhysicalInterface, ['enabled'], delete=False)
113+
114+
115 class BondInterface(Interface):
116
117 class Meta(Interface.Meta):
118
119=== modified file 'src/maasserver/models/signals/dhcp.py'
120--- src/maasserver/models/signals/dhcp.py 2015-09-08 18:41:57 +0000
121+++ src/maasserver/models/signals/dhcp.py 2015-09-15 14:16:03 +0000
122@@ -27,7 +27,7 @@
123 NodeGroupInterface,
124 Subnet,
125 )
126-from maasserver.models.signals.base import connect_to_field_change
127+from maasserver.utils.signals import connect_to_field_change
128
129
130 def dhcp_post_change_NodeGroupInterface(instance, old_values, **kwargs):
131
132=== modified file 'src/maasserver/models/signals/dns.py'
133--- src/maasserver/models/signals/dns.py 2015-09-08 18:41:57 +0000
134+++ src/maasserver/models/signals/dns.py 2015-09-15 14:16:03 +0000
135@@ -29,7 +29,7 @@
136 NodeGroupInterface,
137 Subnet,
138 )
139-from maasserver.models.signals.base import connect_to_field_change
140+from maasserver.utils.signals import connect_to_field_change
141
142
143 @receiver(post_save, sender=NodeGroup)
144
145=== modified file 'src/maasserver/models/signals/events.py'
146--- src/maasserver/models/signals/events.py 2015-06-10 11:24:44 +0000
147+++ src/maasserver/models/signals/events.py 2015-09-15 14:16:03 +0000
148@@ -21,7 +21,7 @@
149 Node,
150 )
151 from maasserver.models.node import NODE_STATUS
152-from maasserver.models.signals.base import connect_to_field_change
153+from maasserver.utils.signals import connect_to_field_change
154 from provisioningserver.events import (
155 EVENT_DETAILS,
156 EVENT_TYPES,
157
158=== modified file 'src/maasserver/models/signals/monitors.py'
159--- src/maasserver/models/signals/monitors.py 2015-04-01 11:49:45 +0000
160+++ src/maasserver/models/signals/monitors.py 2015-09-15 14:16:03 +0000
161@@ -17,8 +17,8 @@
162
163
164 from maasserver.models import Node
165-from maasserver.models.signals.base import connect_to_field_change
166 from maasserver.node_status import get_failed_status
167+from maasserver.utils.signals import connect_to_field_change
168
169 # Useful to disconnect this in testing.
170 MONITOR_CANCEL_CONNECT = True
171
172=== modified file 'src/maasserver/models/signals/power.py'
173--- src/maasserver/models/signals/power.py 2015-06-10 11:24:44 +0000
174+++ src/maasserver/models/signals/power.py 2015-09-15 14:16:03 +0000
175@@ -18,13 +18,13 @@
176
177 from maasserver.enum import POWER_STATE
178 from maasserver.models import Node
179-from maasserver.models.signals.base import connect_to_field_change
180 from maasserver.node_status import QUERY_TRANSITIONS
181 from maasserver.rpc import getClientFor
182 from maasserver.utils.orm import (
183 post_commit,
184 transactional,
185 )
186+from maasserver.utils.signals import connect_to_field_change
187 from provisioningserver.logger import get_maas_logger
188 from provisioningserver.power.poweraction import (
189 PowerActionFail,
190
191=== modified file 'src/maasserver/models/tests/test_interface.py'
192--- src/maasserver/models/tests/test_interface.py 2015-09-14 19:31:45 +0000
193+++ src/maasserver/models/tests/test_interface.py 2015-09-15 14:16:03 +0000
194@@ -1730,3 +1730,57 @@
195 INTERFACE_LINK_TYPE.STATIC, subnet_v4,
196 ip_address=requested_ip))
197 self.assertEquals(sentinel.claimed_ip, claimed_ip)
198+
199+
200+class TestEnableAndDisableInterface(MAASServerTestCase):
201+
202+ def test__enable_interface_creates_link_up(self):
203+ interface = factory.make_Interface(
204+ INTERFACE_TYPE.PHYSICAL, enabled=False)
205+ interface.enabled = True
206+ interface.save()
207+ link_ip = interface.ip_addresses.get(
208+ alloc_type=IPADDRESS_TYPE.STICKY, ip=None)
209+ self.assertIsNotNone(link_ip)
210+
211+ def test__enable_interface_creates_link_up_on_children(self):
212+ interface = factory.make_Interface(
213+ INTERFACE_TYPE.PHYSICAL, enabled=False)
214+ vlan_interface = factory.make_Interface(
215+ INTERFACE_TYPE.VLAN, parents=[interface])
216+ interface.enabled = True
217+ interface.save()
218+ link_ip = vlan_interface.ip_addresses.get(
219+ alloc_type=IPADDRESS_TYPE.STICKY, ip=None)
220+ self.assertIsNotNone(link_ip)
221+
222+ def test__disable_interface_removes_links(self):
223+ interface = factory.make_Interface(
224+ INTERFACE_TYPE.PHYSICAL, enabled=True)
225+ interface.ensure_link_up()
226+ interface.enabled = False
227+ interface.save()
228+ self.assertItemsEqual([], interface.ip_addresses.all())
229+
230+ def test__disable_interface_removes_links_on_children(self):
231+ interface = factory.make_Interface(
232+ INTERFACE_TYPE.PHYSICAL, enabled=True)
233+ vlan_interface = factory.make_Interface(
234+ INTERFACE_TYPE.VLAN, parents=[interface])
235+ vlan_interface.ensure_link_up()
236+ interface.enabled = False
237+ interface.save()
238+ self.assertItemsEqual([], vlan_interface.ip_addresses.all())
239+
240+ def test__disable_interface_doesnt_remove_links_on_enabled_children(self):
241+ node = factory.make_Node()
242+ nic0 = factory.make_Interface(
243+ INTERFACE_TYPE.PHYSICAL, node=node, enabled=True)
244+ nic1 = factory.make_Interface(
245+ INTERFACE_TYPE.PHYSICAL, node=node, enabled=True)
246+ bond_interface = factory.make_Interface(
247+ INTERFACE_TYPE.BOND, parents=[nic0, nic1])
248+ bond_interface.ensure_link_up()
249+ nic0.enabled = False
250+ nic0.save()
251+ self.assertEquals(1, bond_interface.ip_addresses.count())
252
253=== modified file 'src/maasserver/tests/test_forms_interface_link.py'
254--- src/maasserver/tests/test_forms_interface_link.py 2015-09-08 21:59:19 +0000
255+++ src/maasserver/tests/test_forms_interface_link.py 2015-09-15 14:16:03 +0000
256@@ -48,7 +48,7 @@
257 def test__mode_is_case_insensitive(self):
258 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
259 form = InterfaceLinkForm(instance=interface, data={
260- "mode": INTERFACE_LINK_TYPE.DHCP.lower(),
261+ "mode": INTERFACE_LINK_TYPE.DHCP.upper(),
262 })
263 self.assertTrue(form.is_valid(), form.errors)
264
265
266=== renamed file 'src/maasserver/models/signals/base.py' => 'src/maasserver/utils/signals.py'
267=== renamed file 'src/maasserver/models/signals/tests/test_base.py' => 'src/maasserver/utils/tests/test_signals.py'
268--- src/maasserver/models/signals/tests/test_base.py 2015-06-10 11:24:44 +0000
269+++ src/maasserver/utils/tests/test_signals.py 2015-09-15 14:16:03 +0000
270@@ -14,10 +14,10 @@
271 __metaclass__ = type
272 __all__ = []
273
274-from maasserver.models.signals.base import connect_to_field_change
275 from maasserver.testing.factory import factory
276 from maasserver.testing.testcase import MAASServerTestCase
277 from maasserver.tests.models import FieldChangeTestModel
278+from maasserver.utils.signals import connect_to_field_change
279 from maastesting.djangotestcase import TestModelMixin
280 from maastesting.matchers import (
281 IsCallable,