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

[Jeroen T. Vermeulen]

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

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

[Gavin Panella (allenap)]

review: Approve

=== 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 @@
     NODE_AFTER_COMMISSIONING_ACTION,
     NODE_AFTER_COMMISSIONING_ACTION_CHOICES,
     NODE_STATUS,
+    NODEGROUP_STATUS,
     NODEGROUPINTERFACE_MANAGEMENT,
     NODEGROUPINTERFACE_MANAGEMENT_CHOICES,
     )
@@ -801,6 +802,31 @@
             'name',
             )
 
+    def clean_name(self):
+        old_name = self.instance.name
+        new_name = self.cleaned_data['name']
+        if new_name == old_name or not new_name:
+            # No change to the name.  Return old name.
+            return old_name
+
+        if self.instance.status != NODEGROUP_STATUS.ACCEPTED:
+            # This nodegroup is not in use.  Change it at will.
+            return new_name
+
+        interface = self.instance.get_managed_interface()
+        if interface.management != NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:
+            # MAAS is not managing DNS on this network, so the user can
+            # rename the zone at will.
+            return new_name
+
+        nodes_in_use = Node.objects.filter(
+            nodegroup=self.instance, status=NODE_STATUS.ALLOCATED)
+        if nodes_in_use.exists():
+            raise ValidationError(
+                "Can't rename DNS zone to %s; nodes are in use." % new_name)
+
+        return new_name
+
 
 class TagForm(ModelForm):
 
 
=== 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 @@
     NewUserCreationForm,
     NodeActionForm,
     NodeForm,
+    NodeGroupEdit,
     NodeGroupInterfaceForm,
     NodeGroupWithInterfacesForm,
     NodeWithMACAddressesForm,
@@ -879,3 +880,100 @@
                 nodegroup.management for nodegroup in
                 nodegroup.nodegroupinterface_set.all()
             ])
+
+
+def make_unrenamable_nodegroup_with_node():
+    """Create a `NodeGroup` that can't be renamed, and `Node`.
+
+    Node groups can't be renamed while they are in an accepted state, have
+    DHCP and DNS management enabled, and have a node that is in allocated
+    state.
+
+    :return: tuple: (`NodeGroup`, `Node`).
+    """
+    name = factory.make_name('original-name')
+    nodegroup = factory.make_node_group(
+        name=name, status=NODEGROUP_STATUS.ACCEPTED)
+    interface = nodegroup.get_managed_interface()
+    interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
+    interface.save()
+    node = factory.make_node(nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED)
+    return nodegroup, node
+
+
+class TestNodeGroupEdit(TestCase):
+
+    def make_form_data(self, nodegroup):
+        """Create `NodeGroupEdit` form data based on `nodegroup`."""
+        return {
+            'name': nodegroup.name,
+            'cluster_name': nodegroup.cluster_name,
+            'status': nodegroup.status,
+        }
+
+    def test_changes_name(self):
+        nodegroup = factory.make_node_group(name=factory.make_name('old-name'))
+        new_name = factory.make_name('new-name')
+        data = self.make_form_data(nodegroup)
+        data['name'] = new_name
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(new_name, reload_object(nodegroup).name)
+
+    def test_refuses_name_change_if_dns_managed_and_nodes_in_use(self):
+        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        data = self.make_form_data(nodegroup)
+        data['name'] = factory.make_name('new-name')
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        self.assertFalse(form.is_valid())
+
+    def test_accepts_unchanged_name(self):
+        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        original_name = nodegroup.name
+        form = NodeGroupEdit(
+            instance=nodegroup, data=self.make_form_data(nodegroup))
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(original_name, reload_object(nodegroup).name)
+
+    def test_accepts_omitted_name(self):
+        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        original_name = nodegroup.name
+        data = self.make_form_data(nodegroup)
+        del data['name']
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(original_name, reload_object(nodegroup).name)
+
+    def test_accepts_name_change_if_nodegroup_not_accepted(self):
+        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        nodegroup.status = NODEGROUP_STATUS.PENDING
+        data = self.make_form_data(nodegroup)
+        data['name'] = factory.make_name('new-name')
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        self.assertTrue(form.is_valid())
+
+    def test_accepts_name_change_if_dns_managed_but_no_nodes_in_use(self):
+        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        node.status = NODE_STATUS.READY
+        node.save()
+        data = self.make_form_data(nodegroup)
+        data['name'] = factory.make_name('new-name')
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(data['name'], reload_object(nodegroup).name)
+
+    def test_accepts_name_change_if_nodes_in_use_but_dns_not_managed(self):
+        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        interface = nodegroup.get_managed_interface()
+        interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP
+        interface.save()
+        data = self.make_form_data(nodegroup)
+        data['name'] = factory.make_name('new-name')
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        self.assertTrue(form.is_valid())
+        form.save()
+        self.assertEqual(data['name'], reload_object(nodegroup).name)