Merge lp:~rvb/maas/bug-1234853-1.3 into lp:maas/1.3

[Raphaël Badin]

Fix the add-new-interface UI so that it doesn't blow up when the nodegroup is already 'managed' and one is trying to add another 'managed' interface (backport of revision 1661).

[Raphaël Badin (rvb)]

Simple backport, self-approving.

review: Approve

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-03-12 16:00:25 +0000
+++ src/maasserver/api.py	2013-10-04 13:48:39 +0000
@@ -1313,10 +1313,10 @@
         :type ip_range_high: basestring (IP Address)
         """
         nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
-        form = NodeGroupInterfaceForm(request.data)
+        instance = NodeGroupInterface(nodegroup=nodegroup)
+        form = NodeGroupInterfaceForm(request.data, instance=instance)
         if form.is_valid():
-            return form.save(
-                nodegroup=nodegroup)
+            return form.save()
         else:
             raise ValidationError(form.errors)
 

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-12-20 10:08:36 +0000
+++ src/maasserver/forms.py	2013-10-04 13:48:39 +0000
@@ -697,14 +697,6 @@
             'ip_range_high',
             )
 
-    def save(self, nodegroup=None, commit=True, *args, **kwargs):
-        interface = super(NodeGroupInterfaceForm, self).save(commit=False)
-        if nodegroup is not None:
-            interface.nodegroup = nodegroup
-        if commit:
-            interface.save(*args, **kwargs)
-        return interface
-
 
 INTERFACES_VALIDATION_ERROR_MESSAGE = (
     "Invalid json value: should be a list of dictionaries, each containing "
@@ -789,8 +781,9 @@
     def save(self):
         nodegroup = super(NodeGroupWithInterfacesForm, self).save()
         for interface in self.cleaned_data['interfaces']:
-            form = NodeGroupInterfaceForm(data=interface)
-            form.save(nodegroup=nodegroup)
+            instance = NodeGroupInterface(nodegroup=nodegroup)
+            form = NodeGroupInterfaceForm(data=interface, instance=instance)
+            form.save()
         if self.status is not None:
             nodegroup.status = self.status
             nodegroup.save()

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2012-12-20 10:08:36 +0000
+++ src/maasserver/tests/test_forms.py	2013-10-04 13:48:39 +0000
@@ -652,14 +652,6 @@
 
 class TestNodeGroupInterfaceForm(TestCase):
 
-    def test_NodeGroupInterfaceForm_save_sets_nodegroup(self):
-        nodegroup = factory.make_node_group(
-            management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
-        form = NodeGroupInterfaceForm(data=make_interface_settings())
-        self.assertTrue(form.is_valid(), form._errors)
-        interface = form.save(nodegroup=nodegroup)
-        self.assertEqual(nodegroup, interface.nodegroup)
-
     def test_NodeGroupInterfaceForm_validates_parameters(self):
         form = NodeGroupInterfaceForm(data={'ip': factory.getRandomString()})
         self.assertFalse(form.is_valid())
@@ -671,9 +663,10 @@
         settings['management'] = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
         for field_name in nullable_fields:
             del settings[field_name]
-        form = NodeGroupInterfaceForm(data=settings)
         nodegroup = factory.make_node_group()
-        interface = form.save(nodegroup=nodegroup)
+        form = NodeGroupInterfaceForm(
+            data=settings, instance=NodeGroupInterface(nodegroup=nodegroup))
+        interface = form.save()
         field_values = [
             getattr(interface, field_name) for field_name in nullable_fields]
         self.assertThat(field_values, AllMatch(Equals('')))

=== modified file 'src/maasserver/tests/test_views_settings_clusters.py'
--- src/maasserver/tests/test_views_settings_clusters.py	2013-02-08 17:16:07 +0000
+++ src/maasserver/tests/test_views_settings_clusters.py	2013-10-04 13:48:39 +0000
@@ -198,6 +198,21 @@
             reload_object(interface),
             MatchesStructure.byEquality(**data))
 
+    def test_rejects_interface_creation_if_cluster_already_managed(self):
+        nodegroup = factory.make_node_group(
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
+        create_link = reverse(
+            'cluster-interface-create', args=[nodegroup.uuid])
+        # nodegroup already has a 'managed' interface, try adding another
+        # one, also 'managed'.
+        data = factory.get_interface_fields(
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
+        response = self.client.post(create_link, data)
+        self.assertEqual(httplib.OK, response.status_code)
+        error_message = (
+            "Another managed interface already exists for this cluster.")
+        self.assertThat(response.content, Contains(error_message))
+
 
 # XXX: rvb 2012-10-08 bug=1063881: apache transforms '//' into '/' in
 # the urls it passes around and this happens when an interface has an empty

=== modified file 'src/maasserver/views/settings_clusters.py'
--- src/maasserver/views/settings_clusters.py	2012-10-15 09:32:06 +0000
+++ src/maasserver/views/settings_clusters.py	2013-10-04 13:48:39 +0000
@@ -126,12 +126,18 @@
     form_class = NodeGroupInterfaceForm
     context_object_name = 'interface'
 
+    def get_form_kwargs(self):
+        kwargs = super(ClusterInterfaceCreate, self).get_form_kwargs()
+        assert kwargs.get('instance', None) is None
+        kwargs['instance'] = NodeGroupInterface(nodegroup=self.get_nodegroup())
+        return kwargs
+
     def get_success_url(self):
         uuid = self.kwargs.get('uuid', None)
         return reverse('cluster-edit', args=[uuid])
 
     def form_valid(self, form):
-        self.object = form.save(nodegroup=self.get_nodegroup())
+        self.object = form.save()
         messages.info(self.request, "Interface created.")
         return super(ClusterInterfaceCreate, self).form_valid(form)