Merge lp:~jameinel/maas/get-nodes-for-group into lp:~maas-committers/maas/trunk

[John A Meinel]

Add nodegroups/UUID/?op=list_nodes to get the system_ids for all nodes that are part of the group.
The permissions for this is restricted to the worker for the nodegroup, so it shouldn't be visible to users.

This exposes a '/nodegroups/UUID/?op=list_nodes' that returns the list of system_ids for a given nodegroup.

The use case for this is the new Tag changes to allow the nodegroups to process the tag definitions themselves. We will spawn a 'start processing' request, which will then need to come back and determine what nodes need to be updated for this group.

This is the first step along that path.

[Martin Packman (gz)]

Looks good in general.

+ def test_GET_list_with_nodegroup(self):

This test has no assertions? It wants some, or documentation as to what it's testing.

review: Approve

[John A Meinel (jameinel)]

Actually, it just needs to be removed. Originally I was going to to
GET /nodes?op=list&nodegroup=UUID

However I switched to:

GET /nodegroups/UUID/?op=list_nodes

Since it made more sense to hang it off of the nodegroup api rather than the nodes api. (Especially, as it turned out, because the nodegroup already had good code for restricting requests to the worker task that is processing that nodegroup.)

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-10-02 10:12:12 +0000
+++ src/maasserver/api.py	2012-10-02 15:55:25 +0000
@@ -1128,6 +1128,14 @@
                 {ip: leases[ip] for ip in new_leases if ip in leases})
         return HttpResponse("Leases updated.", status=httplib.OK)
 
+    @operation(idempotent=True)
+    def list_nodes(self, request, uuid):
+        """Get the list of node ids that are part of this group."""
+        nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
+        check_nodegroup_access(request, nodegroup)
+        return [node.system_id
+                for node in Node.objects.filter(nodegroup=nodegroup)]
+
 
 DISPLAYED_NODEGROUP_FIELDS = (
     'ip', 'management', 'interface', 'subnet_mask',

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-10-02 13:55:22 +0000
+++ src/maasserver/tests/test_api.py	2012-10-02 15:55:25 +0000
@@ -3452,6 +3452,36 @@
             httplib.FORBIDDEN, response.status_code,
             explain_unexpected_response(httplib.FORBIDDEN, response))
 
+    def test_nodegroup_list_nodes_requires_authentication(self):
+        nodegroup = factory.make_node_group()
+        response = self.client.get(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'op': 'list_nodes'})
+        self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
+
+    def test_nodegroup_list_nodes_does_not_work_for_normal_user(self):
+        nodegroup = factory.make_node_group()
+        log_in_as_normal_user(self.client)
+        response = self.client.get(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'op': 'list_nodes'})
+        self.assertEqual(
+            httplib.FORBIDDEN, response.status_code,
+            explain_unexpected_response(httplib.FORBIDDEN, response))
+
+    def test_nodegroup_list_works_for_nodegroup_worker(self):
+        nodegroup = factory.make_node_group()
+        node = factory.make_node(nodegroup=nodegroup)
+        client = make_worker_client(nodegroup)
+        response = client.get(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'op': 'list_nodes'})
+        self.assertEqual(
+            httplib.OK, response.status_code,
+            explain_unexpected_response(httplib.OK, response))
+        parsed_result = json.loads(response.content)
+        self.assertItemsEqual([node.system_id], parsed_result)
+
 
 class TestBootImagesAPI(APITestCase):