Merge lp:~rvb/maas/api-update-cluster into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Add method to update a cluster using the API.

This has been discussed with Julian. We need this (in trunk and 1.2) because we need to improve the migration story from the version of MAAS that used cobbler and this will allow us to simplify the migration script.

The testing is pretty minimal because the form itself is already tested in src/maasserver/tests/test_forms.py:TestNodeGroupEdit.

[Gavin Panella (allenap)]

Looks good.

[1]

+            raise PermissionDenied("That method is reserved to admin users.")

s/to/for/

[2]

+        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.

Why is this? Is the reason documented somewhere in the tree?

review: Approve

[Raphaël Badin (rvb)]

Thanks for the review.

> [1]
>
> +            raise PermissionDenied("That method is reserved to admin users.")
>
> s/to/for/

Done.

> [2]
>
> +        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.
>
> Why is this? Is the reason documented somewhere in the tree?

Because changing the name of the nodegroup changes the hostname of the nodes attached to it. You don't want to do this when you have node deployed with Juju. see src/maasserver/forms.py:NodeGroupEdit.clean_name()/

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-03-12 16:00:25 +0000
+++ src/maasserver/api.py	2013-04-24 17:44:26 +0000
@@ -67,6 +67,7 @@
     "FilesHandler",
     "get_oauth_token",
     "MaasHandler",
+    "NodeGroupHandler",
     "NodeGroupsHandler",
     "NodeGroupInterfaceHandler",
     "NodeGroupInterfacesHandler",
@@ -161,6 +162,7 @@
 from maasserver.forms import (
     get_node_create_form,
     get_node_edit_form,
+    NodeGroupEdit,
     NodeGroupInterfaceForm,
     NodeGroupWithInterfacesForm,
     TagForm,
@@ -216,6 +218,9 @@
     )
 
 
+METHOD_RESERVED_ADMIN = "That method is reserved for admin users."
+
+
 def store_node_power_parameters(node, request):
     """Store power parameters in request.
 
@@ -1121,13 +1126,13 @@
                 nodegroup.accept()
             return HttpResponse("Nodegroup(s) accepted.", status=httplib.OK)
         else:
-            raise PermissionDenied("That method is reserved to admin users.")
+            raise PermissionDenied(METHOD_RESERVED_ADMIN)
 
     @operation(idempotent=False)
     def import_boot_images(self, request):
         """Import the boot images on all the accepted cluster controllers."""
         if not request.user.is_superuser:
-            raise PermissionDenied("That method is reserved to admin users.")
+            raise PermissionDenied(METHOD_RESERVED_ADMIN)
         NodeGroup.objects.import_boot_images_accepted_clusters()
         return HttpResponse(
             "Import of boot images started on all cluster controllers",
@@ -1149,7 +1154,7 @@
                 nodegroup.reject()
             return HttpResponse("Nodegroup(s) rejected.", status=httplib.OK)
         else:
-            raise PermissionDenied("That method is reserved to admin users.")
+            raise PermissionDenied(METHOD_RESERVED_ADMIN)
 
     @classmethod
     def resource_uri(cls):
@@ -1186,7 +1191,7 @@
     Each NodeGroup has its own uuid.
     """
 
-    create = update = delete = None
+    create = delete = None
     fields = DISPLAYED_NODEGROUP_FIELDS
 
     def read(self, request, uuid):
@@ -1201,6 +1206,27 @@
             uuid = nodegroup.uuid
         return ('nodegroup_handler', [uuid])
 
+    def update(self, request, uuid):
+        """Update a specific cluster.
+
+        :param name: The new DNS name for this cluster.
+        :type name: basestring
+        :param cluster_name: The new name for this cluster.
+        :type cluster_name: basestring
+        :param status: The new status for this cluster (see
+            vocabulary `NODEGROUP_STATUS`).
+        :type status: int
+        """
+        if not request.user.is_superuser:
+            raise PermissionDenied(METHOD_RESERVED_ADMIN)
+        nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
+        data = get_overrided_query_dict(model_to_dict(nodegroup), request.data)
+        form = NodeGroupEdit(instance=nodegroup, data=data)
+        if form.is_valid():
+            return form.save()
+        else:
+            raise ValidationError(form.errors)
+
     @operation(idempotent=False)
     def update_leases(self, request, uuid):
         """Submit latest state of DHCP leases within the cluster.
@@ -1222,7 +1248,7 @@
     def import_boot_images(self, request, uuid):
         """Import the pxe files on this cluster controller."""
         if not request.user.is_superuser:
-            raise PermissionDenied("That method is reserved to admin users.")
+            raise PermissionDenied(METHOD_RESERVED_ADMIN)
         nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
         nodegroup.import_boot_images()
         return HttpResponse(

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2013-03-07 14:41:14 +0000
+++ src/maasserver/testing/factory.py	2013-04-24 17:44:26 +0000
@@ -214,6 +214,25 @@
         ng.save()
         return ng
 
+    def make_unrenamable_nodegroup_with_node(self):
+        """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 = self.make_name('original-name')
+        nodegroup = self.make_node_group(
+            name=name, status=NODEGROUP_STATUS.ACCEPTED)
+        interface = nodegroup.get_managed_interface()
+        interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS
+        interface.save()
+        node = self.make_node(
+            nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED)
+        return nodegroup, node
+
     def make_node_group_interface(self, nodegroup, ip=None,
                                   router_ip=None, network=None,
                                   subnet_mask=None, broadcast_ip=None,

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2013-03-12 15:25:43 +0000
+++ src/maasserver/tests/test_api.py	2013-04-24 17:44:26 +0000
@@ -72,6 +72,7 @@
     NODE_STATUS,
     NODE_STATUS_CHOICES_DICT,
     NODEGROUP_STATUS,
+    NODEGROUP_STATUS_CHOICES,
     NODEGROUPINTERFACE_MANAGEMENT,
     )
 from maasserver.exceptions import MAASAPIBadRequest
@@ -4348,6 +4349,51 @@
             self.get_uri('nodegroups/%s/' % factory.make_name('nodegroup')))
         self.assertEqual(httplib.NOT_FOUND, response.status_code)
 
+    def test_PUT_reserved_to_admin_users(self):
+        nodegroup = factory.make_node_group()
+        response = self.client.put(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'name': factory.make_name("new-name")})
+
+        self.assertEqual(httplib.FORBIDDEN, response.status_code)
+
+    def test_PUT_updates_nodegroup(self):
+        # The api allows the updating of a NodeGroup.
+        nodegroup = factory.make_node_group()
+        self.become_admin()
+        new_name = factory.make_name("new-name")
+        new_cluster_name = factory.make_name("new-cluster-name")
+        new_status = factory.getRandomChoice(
+            NODEGROUP_STATUS_CHOICES, but_not=[nodegroup.status])
+        response = self.client.put(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {
+                'name': new_name,
+                'cluster_name': new_cluster_name,
+                'status': new_status,
+            })
+
+        self.assertEqual(httplib.OK, response.status_code, response.content)
+        nodegroup = reload_object(nodegroup)
+        self.assertEqual(
+            (new_name, new_cluster_name, new_status),
+            (nodegroup.name, nodegroup.cluster_name, nodegroup.status))
+
+    def test_PUT_updates_nodegroup_validates_data(self):
+        nodegroup, _ = factory.make_unrenamable_nodegroup_with_node()
+        self.become_admin()
+        new_name = factory.make_name("new-name")
+        response = self.client.put(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'name': new_name})
+
+        parsed_result = json.loads(response.content)
+
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        self.assertIn(
+            "Can't rename DNS zone",
+            parsed_result['name'][0])
+
     def test_update_leases_processes_empty_leases_dict(self):
         nodegroup = factory.make_node_group()
         factory.make_dhcp_lease(nodegroup=nodegroup)

=== 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-04-24 17:44:26 +0000
@@ -842,25 +842,6 @@
             ])
 
 
-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):
@@ -882,14 +863,14 @@
         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()
+        nodegroup, node = factory.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()
+        nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
         original_name = nodegroup.name
         form = NodeGroupEdit(
             instance=nodegroup, data=self.make_form_data(nodegroup))
@@ -898,7 +879,7 @@
         self.assertEqual(original_name, reload_object(nodegroup).name)
 
     def test_accepts_omitted_name(self):
-        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
         original_name = nodegroup.name
         data = self.make_form_data(nodegroup)
         del data['name']
@@ -908,7 +889,7 @@
         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, node = factory.make_unrenamable_nodegroup_with_node()
         nodegroup.status = NODEGROUP_STATUS.PENDING
         data = self.make_form_data(nodegroup)
         data['name'] = factory.make_name('new-name')
@@ -916,7 +897,7 @@
         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()
+        nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
         node.status = NODE_STATUS.READY
         node.save()
         data = self.make_form_data(nodegroup)
@@ -927,7 +908,7 @@
         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()
+        nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
         interface = nodegroup.get_managed_interface()
         interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP
         interface.save()
@@ -939,7 +920,7 @@
         self.assertEqual(data['name'], reload_object(nodegroup).name)
 
     def test_accepts_name_change_if_nodegroup_has_no_interface(self):
-        nodegroup, node = make_unrenamable_nodegroup_with_node()
+        nodegroup, node = factory.make_unrenamable_nodegroup_with_node()
         NodeGroupInterface.objects.filter(nodegroup=nodegroup).delete()
         data = self.make_form_data(nodegroup)
         data['name'] = factory.make_name('new-name')