launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/metadata-instance-id into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/metadata-instance-id/+merge/94250

This adds a meta-data attribute to the metadata service: “instance-id.”  Apparently it's preferable to return something here that identifies _an install_ of a system, which would change if a new OS were installed on the node for instance, but the model doesn't have a direct equivalent yet.  For now, it will just return the node's system_id.

Along the way I also added tests to ensure that all fields listed in the piston handler are actually supported, and that they are ordered alphabetically (because consistency may become subtly relevant in all sorts of nasty test situations).  Which in turn uncovered that the index of /metadata/latest/meta-data/ wasn't actually being produced properly.  There wasn't much point in testing this when there was only 1 field, but now it'd covered.
-- 
https://code.launchpad.net/~jtv/maas/metadata-instance-id/+merge/94250
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/metadata-instance-id into lp:maas.

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2012-02-22 17:19:51 +0000
+++ src/metadataserver/api.py	2012-02-22 18:59:17 +0000
@@ -77,7 +77,7 @@
     allowed_methods = ('GET',)
 
     def read(self, request):
-        return make_list_response(self.fields)
+        return make_list_response(sorted(self.fields))
 
 
 class IndexHandler(MetadataViewHandler):
@@ -99,7 +99,7 @@
 class MetaDataHandler(VersionIndexHandler):
     """Meta-data listing for a given version."""
 
-    fields = ('local-hostname',)
+    fields = ('instance-id', 'local-hostname',)
 
     def get_attribute_producer(self, item):
         """Return a callable to deliver a given metadata item.
@@ -118,24 +118,30 @@
 
         producers = {
             'local-hostname': self.local_hostname,
+            'instance-id': self.instance_id,
         }
 
         return producers[field]
 
     def read(self, request, version, item=None):
-        check_version(version)
         if item is None or len(item) == 0:
             # Requesting the list of attributes, not any particular
             # attribute.
-            return super(MetaDataHandler, self).read(request)
+            return super(MetaDataHandler, self).read(request, version)
 
+        check_version(version)
         node = get_node_for_request(request)
         producer = self.get_attribute_producer(item)
         return producer(node, version, item)
 
     def local_hostname(self, node, version, item):
+        """Produce local-hostname attribute."""
         return make_text_response(node.hostname)
 
+    def instance_id(self, node, version, item):
+        """Produce instance-id attribute."""
+        return make_text_response(node.system_id)
+
 
 class UserDataHandler(MetadataViewHandler):
     """User-data blob for a given version."""

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-02-22 14:03:31 +0000
+++ src/metadataserver/tests/test_api.py	2012-02-22 18:59:17 +0000
@@ -26,6 +26,7 @@
     get_node_for_request,
     make_list_response,
     make_text_response,
+    MetaDataHandler,
     UnknownMetadataVersion,
     )
 from metadataserver.models import NodeKey
@@ -150,17 +151,30 @@
         self.assertIn('meta-data', items)
         self.assertIn('user-data', items)
 
-    def test_meta_data_view_returns_text_response(self):
+    def test_meta_data_view_lists_fields(self):
         client = self.make_node_client()
         response = self.get('/latest/meta-data/', client)
         self.assertIn('text/plain', response['Content-Type'])
+        self.assertItemsEqual(
+            MetaDataHandler.fields, response.content.split())
+
+    def test_meta_data_view_is_sorted(self):
+        client = self.make_node_client()
+        response = self.get('/latest/meta-data/', client)
+        attributes = response.content.split()
+        self.assertEqual(sorted(attributes), attributes)
 
     def test_meta_data_unknown_item_is_not_found(self):
         client = self.make_node_client()
         response = self.get('/latest/meta-data/UNKNOWN-ITEM-HA-HA-HA', client)
         self.assertEqual(httplib.NOT_FOUND, response.status_code)
 
-    def test_meta_data_local_hostname(self):
+    def test_get_attribute_producer_supports_all_fields(self):
+        handler = MetaDataHandler()
+        producers = map(handler.get_attribute_producer, handler.fields)
+        self.assertNotIn(None, producers)
+
+    def test_meta_data_local_hostname_returns_hostname(self):
         hostname = factory.getRandomString()
         client = self.make_node_client(factory.make_node(hostname=hostname))
         response = self.get('/latest/meta-data/local-hostname', client)
@@ -169,6 +183,15 @@
             (response.status_code, response.content.decode('ascii')))
         self.assertIn('text/plain', response['Content-Type'])
 
+    def test_meta_data_instance_id_returns_system_id(self):
+        node = factory.make_node()
+        client = self.make_node_client(node)
+        response = self.get('/latest/meta-data/instance-id', client)
+        self.assertEqual(
+            (httplib.OK, node.system_id),
+            (response.status_code, response.content.decode('ascii')))
+        self.assertIn('text/plain', response['Content-Type'])
+
     def test_user_data_view_returns_binary_blob(self):
         client = self.make_node_client()
         response = self.get('/latest/user-data', client)