Merge lp:~jtv/maas/1.2-bug-1070775 into lp:maas/1.2

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: 1281
Merged at revision: 1277
Proposed branch: lp:~jtv/maas/1.2-bug-1070775
Merge into: lp:maas/1.2
Diff against target: 156 lines (+124/-0)
2 files modified
src/maasserver/forms.py (+26/-0)
src/maasserver/tests/test_forms.py (+98/-0)
To merge this branch: bzr merge lp:~jtv/maas/1.2-bug-1070775
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+132054@code.launchpad.net

Commit message

Disallow nodegroup (DNS zone) name changes while allocated nodes are still using that zone name.

Description of the change

This fixes bug 1070775, after discussion with Raphaël: changing a NodeGroup.name will confuse Juju when it has nodes in use in that nodegroup — at least if we're managing DNS for that nodegroup. If we're not managing DNS, we can only assume that the admin knows what they're doing.

I'll be forward-porting this to trunk as well.

Jeroen

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) :
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-10-29 11:04:39 +0000
+++ src/maasserver/forms.py 2012-10-30 10:55:27 +0000
@@ -62,6 +62,7 @@
62 NODE_AFTER_COMMISSIONING_ACTION,62 NODE_AFTER_COMMISSIONING_ACTION,
63 NODE_AFTER_COMMISSIONING_ACTION_CHOICES,63 NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
64 NODE_STATUS,64 NODE_STATUS,
65 NODEGROUP_STATUS,
65 NODEGROUPINTERFACE_MANAGEMENT,66 NODEGROUPINTERFACE_MANAGEMENT,
66 NODEGROUPINTERFACE_MANAGEMENT_CHOICES,67 NODEGROUPINTERFACE_MANAGEMENT_CHOICES,
67 )68 )
@@ -801,6 +802,31 @@
801 'name',802 'name',
802 )803 )
803804
805 def clean_name(self):
806 old_name = self.instance.name
807 new_name = self.cleaned_data['name']
808 if new_name == old_name or not new_name:
809 # No change to the name. Return old name.
810 return old_name
811
812 if self.instance.status != NODEGROUP_STATUS.ACCEPTED:
813 # This nodegroup is not in use. Change it at will.
814 return new_name
815
816 interface = self.instance.get_managed_interface()
817 if interface.management != NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:
818 # MAAS is not managing DNS on this network, so the user can
819 # rename the zone at will.
820 return new_name
821
822 nodes_in_use = Node.objects.filter(
823 nodegroup=self.instance, status=NODE_STATUS.ALLOCATED)
824 if nodes_in_use.exists():
825 raise ValidationError(
826 "Can't rename DNS zone to %s; nodes are in use." % new_name)
827
828 return new_name
829
804830
805class TagForm(ModelForm):831class TagForm(ModelForm):
806832
807833
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-10-29 16:53:58 +0000
+++ src/maasserver/tests/test_forms.py 2012-10-30 10:55:27 +0000
@@ -44,6 +44,7 @@
44 NewUserCreationForm,44 NewUserCreationForm,
45 NodeActionForm,45 NodeActionForm,
46 NodeForm,46 NodeForm,
47 NodeGroupEdit,
47 NodeGroupInterfaceForm,48 NodeGroupInterfaceForm,
48 NodeGroupWithInterfacesForm,49 NodeGroupWithInterfacesForm,
49 NodeWithMACAddressesForm,50 NodeWithMACAddressesForm,
@@ -879,3 +880,100 @@
879 nodegroup.management for nodegroup in880 nodegroup.management for nodegroup in
880 nodegroup.nodegroupinterface_set.all()881 nodegroup.nodegroupinterface_set.all()
881 ])882 ])
883
884
885def make_unrenamable_nodegroup_with_node():
886 """Create a `NodeGroup` that can't be renamed, and `Node`.
887
888 Node groups can't be renamed while they are in an accepted state, have
889 DHCP and DNS management enabled, and have a node that is in allocated
890 state.
891
892 :return: tuple: (`NodeGroup`, `Node`).
893 """
894 name = factory.make_name('original-name')
895 nodegroup = factory.make_node_group(
896 name=name, status=NODEGROUP_STATUS.ACCEPTED)
897 interface = nodegroup.get_managed_interface()
898 interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
899 interface.save()
900 node = factory.make_node(nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED)
901 return nodegroup, node
902
903
904class TestNodeGroupEdit(TestCase):
905
906 def make_form_data(self, nodegroup):
907 """Create `NodeGroupEdit` form data based on `nodegroup`."""
908 return {
909 'name': nodegroup.name,
910 'cluster_name': nodegroup.cluster_name,
911 'status': nodegroup.status,
912 }
913
914 def test_changes_name(self):
915 nodegroup = factory.make_node_group(name=factory.make_name('old-name'))
916 new_name = factory.make_name('new-name')
917 data = self.make_form_data(nodegroup)
918 data['name'] = new_name
919 form = NodeGroupEdit(instance=nodegroup, data=data)
920 self.assertTrue(form.is_valid())
921 form.save()
922 self.assertEqual(new_name, reload_object(nodegroup).name)
923
924 def test_refuses_name_change_if_dns_managed_and_nodes_in_use(self):
925 nodegroup, node = make_unrenamable_nodegroup_with_node()
926 data = self.make_form_data(nodegroup)
927 data['name'] = factory.make_name('new-name')
928 form = NodeGroupEdit(instance=nodegroup, data=data)
929 self.assertFalse(form.is_valid())
930
931 def test_accepts_unchanged_name(self):
932 nodegroup, node = make_unrenamable_nodegroup_with_node()
933 original_name = nodegroup.name
934 form = NodeGroupEdit(
935 instance=nodegroup, data=self.make_form_data(nodegroup))
936 self.assertTrue(form.is_valid())
937 form.save()
938 self.assertEqual(original_name, reload_object(nodegroup).name)
939
940 def test_accepts_omitted_name(self):
941 nodegroup, node = make_unrenamable_nodegroup_with_node()
942 original_name = nodegroup.name
943 data = self.make_form_data(nodegroup)
944 del data['name']
945 form = NodeGroupEdit(instance=nodegroup, data=data)
946 self.assertTrue(form.is_valid())
947 form.save()
948 self.assertEqual(original_name, reload_object(nodegroup).name)
949
950 def test_accepts_name_change_if_nodegroup_not_accepted(self):
951 nodegroup, node = make_unrenamable_nodegroup_with_node()
952 nodegroup.status = NODEGROUP_STATUS.PENDING
953 data = self.make_form_data(nodegroup)
954 data['name'] = factory.make_name('new-name')
955 form = NodeGroupEdit(instance=nodegroup, data=data)
956 self.assertTrue(form.is_valid())
957
958 def test_accepts_name_change_if_dns_managed_but_no_nodes_in_use(self):
959 nodegroup, node = make_unrenamable_nodegroup_with_node()
960 node.status = NODE_STATUS.READY
961 node.save()
962 data = self.make_form_data(nodegroup)
963 data['name'] = factory.make_name('new-name')
964 form = NodeGroupEdit(instance=nodegroup, data=data)
965 self.assertTrue(form.is_valid())
966 form.save()
967 self.assertEqual(data['name'], reload_object(nodegroup).name)
968
969 def test_accepts_name_change_if_nodes_in_use_but_dns_not_managed(self):
970 nodegroup, node = make_unrenamable_nodegroup_with_node()
971 interface = nodegroup.get_managed_interface()
972 interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP
973 interface.save()
974 data = self.make_form_data(nodegroup)
975 data['name'] = factory.make_name('new-name')
976 form = NodeGroupEdit(instance=nodegroup, data=data)
977 self.assertTrue(form.is_valid())
978 form.save()
979 self.assertEqual(data['name'], reload_object(nodegroup).name)

Subscribers

People subscribed via source and target branches

to status/vote changes: