MAAS

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

  1. bug-1077075
  2. Merge into trunk
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 1344
Proposed branch: lp:~jtv/maas/bug-1077075
Merge into: lp:~maas-committers/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) Approve
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.
Revision history for this message
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
=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py 2012-11-09 09:11:51 +0000
+++ src/maasserver/forms.py 2012-11-09 17:18:25 +0000
@@ -857,6 +857,11 @@
857 return new_name857 return new_name
858858
859 interface = self.instance.get_managed_interface()859 interface = self.instance.get_managed_interface()
860 if interface is None:
861 # No network interfaces. It's weird, but there certainly
862 # won't be a problem with the name change.
863 return new_name
864
860 if interface.management != NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:865 if interface.management != NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:
861 # MAAS is not managing DNS on this network, so the user can866 # MAAS is not managing DNS on this network, so the user can
862 # rename the zone at will.867 # rename the zone at will.
863868
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-11-09 09:11:51 +0000
+++ src/maasserver/tests/test_forms.py 2012-11-09 17:18:25 +0000
@@ -57,6 +57,7 @@
57 MACAddress,57 MACAddress,
58 Node,58 Node,
59 NodeGroup,59 NodeGroup,
60 NodeGroupInterface,
60 )61 )
61from maasserver.models.config import DEFAULT_CONFIG62from maasserver.models.config import DEFAULT_CONFIG
62from maasserver.node_action import (63from maasserver.node_action import (
@@ -981,3 +982,13 @@
981 self.assertTrue(form.is_valid())982 self.assertTrue(form.is_valid())
982 form.save()983 form.save()
983 self.assertEqual(data['name'], reload_object(nodegroup).name)984 self.assertEqual(data['name'], reload_object(nodegroup).name)
985
986 def test_accepts_name_change_if_nodegroup_has_no_interface(self):
987 nodegroup, node = make_unrenamable_nodegroup_with_node()
988 NodeGroupInterface.objects.filter(nodegroup=nodegroup).delete()
989 data = self.make_form_data(nodegroup)
990 data['name'] = factory.make_name('new-name')
991 form = NodeGroupEdit(instance=nodegroup, data=data)
992 self.assertTrue(form.is_valid())
993 form.save()
994 self.assertEqual(data['name'], reload_object(nodegroup).name)