launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/get-hw-details-api into lp:maas.

Commit message:
Add a nodegroup/UUID/?op=node_hardware_details to give celery workers a way to request the hardware_details of the nodes they are evaluating.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/get-hw-details-api/+merge/127657

Another step along the 'process hardware details asynchronously from tag creation'.

This gives the workers a way to request a batch of hardware details from the database. We already expose a way to get all the system_ids in one request, but we didn't want to download all of the hardware details in the same request. (a system_id is 40 bytes, hardware_details is 24,000 bytes.)

The workers will grab the full system_id list, but then only process the hardware details in batches of ~100 nodes at a time.

-- 
https://code.launchpad.net/~jameinel/maas/get-hw-details-api/+merge/127657
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/maas/get-hw-details-api into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-10-03 02:28:04 +0000
+++ src/maasserver/api.py	2012-10-03 07:32:21 +0000
@@ -1136,6 +1136,24 @@
         return [node.system_id
                 for node in Node.objects.filter(nodegroup=nodegroup)]
 
+    @operation(idempotent=True)
+    def node_hardware_details(self, request, uuid):
+        """Return specific hardware_details for each node specified.
+
+        For security purposes we do:
+        a) Requests are only fulfilled for the worker assigned to the
+           nodegroup.
+        b) Requests for nodes that are not part of the nodegroup are just
+           ignored.
+        """
+        system_ids = request.GET.getlist('system_ids')
+        nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
+        check_nodegroup_access(request, nodegroup)
+        nodes = Node.objects.filter(
+            system_id__in=system_ids, nodegroup=nodegroup)
+        return [(node.system_id, node.hardware_details) for node in nodes]
+
+
 
 DISPLAYED_NODEGROUP_FIELDS = (
     'ip', 'management', 'interface', 'subnet_mask',

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-10-03 02:28:04 +0000
+++ src/maasserver/tests/test_api.py	2012-10-03 07:32:21 +0000
@@ -3469,7 +3469,7 @@
             httplib.FORBIDDEN, response.status_code,
             explain_unexpected_response(httplib.FORBIDDEN, response))
 
-    def test_nodegroup_list_works_for_nodegroup_worker(self):
+    def test_nodegroup_list_nodes_works_for_nodegroup_worker(self):
         nodegroup = factory.make_node_group()
         node = factory.make_node(nodegroup=nodegroup)
         client = make_worker_client(nodegroup)
@@ -3482,6 +3482,65 @@
         parsed_result = json.loads(response.content)
         self.assertItemsEqual([node.system_id], parsed_result)
 
+    def make_node_hardware_details_request(self, client, nodegroup=None):
+        if nodegroup is None:
+            nodegroup = factory.make_node_group()
+        node = factory.make_node(nodegroup=nodegroup)
+        return client.get(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'op': 'node_hardware_details', 'system_ids': [node.system_id]})
+
+    def test_GET_node_hardware_details_requires_authentication(self):
+        response = self.make_node_hardware_details_request(self.client)
+        self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
+
+    def test_GET_node_hardware_details_refuses_nonworker(self):
+        log_in_as_normal_user(self.client)
+        response = self.make_node_hardware_details_request(self.client)
+        self.assertEqual(
+            httplib.FORBIDDEN, response.status_code,
+            explain_unexpected_response(httplib.FORBIDDEN, response))
+
+    def test_GET_node_hardware_details_returns_hardware_details(self):
+        nodegroup = factory.make_node_group()
+        hardware_details = '<node />'
+        node = factory.make_node(nodegroup=nodegroup)
+        node.set_hardware_details(hardware_details)
+        client = make_worker_client(nodegroup)
+        response = client.get(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {'op': 'node_hardware_details', 'system_ids': [node.system_id]})
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        self.assertEqual([[node.system_id, hardware_details]], parsed_result)
+
+    def test_GET_node_hardware_details_does_not_see_other_groups(self):
+        hardware_details = '<node />'
+        nodegroup_mine = factory.make_node_group()
+        nodegroup_theirs = factory.make_node_group()
+        node_mine = factory.make_node(nodegroup=nodegroup_mine)
+        node_mine.set_hardware_details(hardware_details)
+        node_theirs = factory.make_node(nodegroup=nodegroup_theirs)
+        node_theirs.set_hardware_details(hardware_details)
+        client = make_worker_client(nodegroup_mine)
+        response = client.get(
+            reverse('nodegroup_handler', args=[nodegroup_mine.uuid]),
+            {'op': 'node_hardware_details',
+             'system_ids': [node_mine.system_id, node_theirs.system_id]})
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        self.assertEqual([[node_mine.system_id, hardware_details]],
+                         parsed_result)
+
+    def test_GET_node_hardware_details_with_no_details(self):
+        nodegroup = factory.make_node_group()
+        client = make_worker_client(nodegroup)
+        response = self.make_node_hardware_details_request(client, nodegroup)
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        node_system_id = parsed_result[0][0]
+        self.assertEqual([[node_system_id, None]], parsed_result)
+
 
 class TestBootImagesAPI(APITestCase):