Merge lp:~jtv/maas/bug-1077075 into lp:maas/trunk

Proposed by Jeroen T. Vermeulen on 2012-11-09
Status: Merged
Approved by: Jeroen T. Vermeulen on 2012-11-09
Approved revision: 1345
Merged at revision: 1344
Proposed branch: lp:~jtv/maas/bug-1077075
Merge into: lp:maas/trunk
Diff against target: 41 lines (+16/-0)
2 files modified
src/maasserver/forms.py (+5/-0)
src/maasserver/tests/test_forms.py (+11/-0)
To merge this branch: bzr merge lp:~jtv/maas/bug-1077075
Reviewer Review Type Date Requested Status
Raphaël Badin (community) 2012-11-09 Approve on 2012-11-09
Review via email: mp+133718@code.launchpad.net

Commit message

Fix oops when renaming accepted nodegroup without interfaces.

Description of the change

This came up during Q/A. Minimal discussion with Raphaël. The “clean” method for NodeGroup.name dereferences the result of NodeGroup.get_managed_interface() — but the result could be None. It's not an easy situation to get into, but it's possible.

If there's no interface on a nodegroup, there's no reason to stop the user from renaming the thing. So I changed the method to permit this scenario.

Jeroen

To post a comment you must log in.
Raphaël Badin (rvb) wrote :

Looks good to me.

review: Approve

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 2012-11-09 09:11:51 +0000
3+++ src/maasserver/forms.py 2012-11-09 17:18:25 +0000
4@@ -857,6 +857,11 @@
5 return new_name
6
7 interface = self.instance.get_managed_interface()
8+ if interface is None:
9+ # No network interfaces. It's weird, but there certainly
10+ # won't be a problem with the name change.
11+ return new_name
12+
13 if interface.management != NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:
14 # MAAS is not managing DNS on this network, so the user can
15 # rename the zone at will.
16
17=== modified file 'src/maasserver/tests/test_forms.py'
18--- src/maasserver/tests/test_forms.py 2012-11-09 09:11:51 +0000
19+++ src/maasserver/tests/test_forms.py 2012-11-09 17:18:25 +0000
20@@ -57,6 +57,7 @@
21 MACAddress,
22 Node,
23 NodeGroup,
24+ NodeGroupInterface,
25 )
26 from maasserver.models.config import DEFAULT_CONFIG
27 from maasserver.node_action import (
28@@ -981,3 +982,13 @@
29 self.assertTrue(form.is_valid())
30 form.save()
31 self.assertEqual(data['name'], reload_object(nodegroup).name)
32+
33+ def test_accepts_name_change_if_nodegroup_has_no_interface(self):
34+ nodegroup, node = make_unrenamable_nodegroup_with_node()
35+ NodeGroupInterface.objects.filter(nodegroup=nodegroup).delete()
36+ data = self.make_form_data(nodegroup)
37+ data['name'] = factory.make_name('new-name')
38+ form = NodeGroupEdit(instance=nodegroup, data=data)
39+ self.assertTrue(form.is_valid())
40+ form.save()
41+ self.assertEqual(data['name'], reload_object(nodegroup).name)