Return request ID to callers

Currently, calls (create, get, etc.) return only the Image to a
caller. In order to log a mapping the request IDs of both glanceclient
and the caller, the x-openstack-request-id header value is needed on
the server side. This change allows that value to be bubbled up and
returned to the caller so that the appropriate logging can occur.
The return_req_id parameter can be set by services that are logging
request IDs. Glance's request ID will then be returned via the
return_req_id parameter.

This is a prerequisite for Log Request ID Mappings nova-spec to
be completed; Change Ib9b820a0feeb0c0e828ed3e4fab8261f8761ba9a

Change I43be05c351f901cee5509c76cff6d69f060c0b3f is an example of
a caller using this.

Implements: blueprint return-req-id
Change-Id: Ia82aa14db5f0e453010514fffb9a25d7b0fc2fd1

glanceclient/common/base.py

@@ -57,10 +57,12 @@ class Manager(object):
             obj_class = self.resource_class
 
         data = body[response_key]
-        return [obj_class(self, res, loaded=True) for res in data if res]
+        return ([obj_class(self, res, loaded=True) for res in data if res],
+                resp)
 
     def _delete(self, url):
-        self.api.raw_request('DELETE', url)
+        resp = self.api.raw_request('DELETE', url)
+        return resp[0]
 
     def _update(self, url, body, response_key=None):
         resp, body = self.api.json_request('PUT', url, body=body)

glanceclient/v1/images.py

@@ -39,6 +39,8 @@ SORT_DIR_VALUES = ('asc', 'desc')
 SORT_KEY_VALUES = ('name', 'status', 'container_format', 'disk_format',
                    'size', 'id', 'created_at', 'updated_at')
 
+OS_REQ_ID_HDR = 'x-openstack-request-id'
+
 
 class Image(base.Resource):
     def __repr__(self):
@@ -47,7 +49,7 @@ class Image(base.Resource):
     def update(self, **fields):
         self.manager.update(self, **fields)
 
-    def delete(self):
+    def delete(self, **kwargs):
         return self.manager.delete(self)
 
     def data(self, **kwargs):
@@ -104,7 +106,7 @@ class ImageManager(base.Manager):
                     pass
         return meta
 
-    def get(self, image):
+    def get(self, image, **kwargs):
         """Get the metadata for a specific image.
 
         :param image: image object or id to look up
@@ -115,9 +117,13 @@ class ImageManager(base.Manager):
         resp, body = self.api.raw_request('HEAD', '/v1/images/%s'
                                           % parse.quote(str(image_id)))
         meta = self._image_meta_from_headers(dict(resp.getheaders()))
+        return_request_id = kwargs.get('return_req_id', None)
+        if return_request_id is not None:
+            return_request_id.append(resp.getheader(OS_REQ_ID_HDR, None))
+
         return Image(self, meta)
 
-    def data(self, image, do_checksum=True):
+    def data(self, image, do_checksum=True, **kwargs):
         """Get the raw data for a specific image.
 
         :param image: image object or id to look up
@@ -130,6 +136,10 @@ class ImageManager(base.Manager):
         checksum = resp.getheader('x-image-meta-checksum', None)
         if do_checksum and checksum is not None:
             body.set_checksum(checksum)
+        return_request_id = kwargs.get('return_req_id', None)
+        if return_request_id is not None:
+            return_request_id.append(resp.getheader(OS_REQ_ID_HDR, None))
+
         return body
 
     def list(self, **kwargs):
@@ -144,11 +154,14 @@ class ImageManager(base.Manager):
         :param owner: If provided, only images with this owner (tenant id)
                       will be listed. An empty string ('') matches ownerless
                       images.
+        :param return_request_id: If an empty list is provided, populate this
+                              list with the request ID value from the header
+                              x-openstack-request-id
         :rtype: list of :class:`Image`
         """
         absolute_limit = kwargs.get('limit')
 
-        def paginate(qp, seen=0):
+        def paginate(qp, seen=0, return_request_id=None):
             def filter_owner(owner, image):
                 # If client side owner 'filter' is specified
                 # only return images that match 'owner'.
@@ -173,7 +186,11 @@ class ImageManager(base.Manager):
                     qp[param] = strutils.safe_encode(value)
 
             url = '/v1/images/detail?%s' % parse.urlencode(qp)
-            images = self._list(url, "images")
+            images, resp = self._list(url, "images")
+
+            if return_request_id is not None:
+                return_request_id.append(resp.getheader(OS_REQ_ID_HDR, None))
+
             for image in images:
                 if filter_owner(owner, image):
                     continue
@@ -186,7 +203,7 @@ class ImageManager(base.Manager):
             if (page_size and len(images) == page_size and
                     (absolute_limit is None or 0 < seen < absolute_limit)):
                 qp['marker'] = image.id
-                for image in paginate(qp, seen):
+                for image in paginate(qp, seen, return_request_id):
                     yield image
 
         params = {'limit': kwargs.get('page_size', DEFAULT_PAGE_SIZE)}
@@ -221,11 +238,16 @@ class ImageManager(base.Manager):
         if 'is_public' in kwargs:
             params['is_public'] = kwargs['is_public']
 
-        return paginate(params)
+        return_request_id = kwargs.get('return_req_id', None)
 
-    def delete(self, image):
+        return paginate(params, 0, return_request_id)
+
+    def delete(self, image, **kwargs):
         """Delete an image."""
-        self._delete("/v1/images/%s" % base.getid(image))
+        resp = self._delete("/v1/images/%s" % base.getid(image))
+        return_request_id = kwargs.get('return_req_id', None)
+        if return_request_id is not None:
+            return_request_id.append(resp.getheader(OS_REQ_ID_HDR, None))
 
     def create(self, **kwargs):
         """Create an image
@@ -242,6 +264,8 @@ class ImageManager(base.Manager):
         for field in kwargs:
             if field in CREATE_PARAMS:
                 fields[field] = kwargs[field]
+            elif field == 'return_req_id':
+                continue
             else:
                 msg = 'create() got an unexpected keyword argument \'%s\''
                 raise TypeError(msg % field)
@@ -254,6 +278,10 @@ class ImageManager(base.Manager):
         resp, body_iter = self.api.raw_request(
             'POST', '/v1/images', headers=hdrs, body=image_data)
         body = json.loads(''.join([c for c in body_iter]))
+        return_request_id = kwargs.get('return_req_id', None)
+        if return_request_id is not None:
+            return_request_id.append(resp.getheader(OS_REQ_ID_HDR, None))
+
         return Image(self, self._format_image_meta_for_user(body['image']))
 
     def update(self, image, **kwargs):
@@ -279,6 +307,8 @@ class ImageManager(base.Manager):
         for field in kwargs:
             if field in UPDATE_PARAMS:
                 fields[field] = kwargs[field]
+            elif field == 'return_req_id':
+                continue
             else:
                 msg = 'update() got an unexpected keyword argument \'%s\''
                 raise TypeError(msg % field)
@@ -292,4 +322,8 @@ class ImageManager(base.Manager):
         resp, body_iter = self.api.raw_request(
             'PUT', url, headers=hdrs, body=image_data)
         body = json.loads(''.join([c for c in body_iter]))
+        return_request_id = kwargs.get('return_req_id', None)
+        if return_request_id is not None:
+            return_request_id.append(resp.getheader(OS_REQ_ID_HDR, None))
+
         return Image(self, self._format_image_meta_for_user(body['image']))

tests/v1/test_images.py

@@ -33,6 +33,7 @@ fixtures = {
         'POST': (
             {
                 'location': '/v1/images/1',
+                'x-openstack-request-id': 'req-1234',
             },
             json.dumps(
                 {'image': {
@@ -71,7 +72,7 @@ fixtures = {
     },
     '/v1/images/detail?is_public=None&limit=20': {
         'GET': (
-            {},
+            {'x-openstack-request-id': 'req-1234'},
             {'images': [
                 {
                     'id': 'a',
@@ -358,6 +359,50 @@ fixtures = {
             'ZZZ',
         ),
     },
+    '/v1/images/4': {
+        'HEAD': (
+            {
+                'x-image-meta-id': '4',
+                'x-image-meta-name': 'image-4',
+                'x-image-meta-property-arch': 'x86_64',
+                'x-image-meta-is_public': 'false',
+                'x-image-meta-protected': 'false',
+                'x-image-meta-deleted': 'false',
+                'x-openstack-request-id': 'req-1234',
+            },
+            None),
+        'GET': (
+            {
+                'x-openstack-request-id': 'req-1234',
+            },
+            'XXX',
+        ),
+        'PUT': (
+            {
+                'x-openstack-request-id': 'req-1234',
+            },
+            json.dumps(
+                {'image': {
+                    'id': '4',
+                    'name': 'image-4',
+                    'container_format': 'ovf',
+                    'disk_format': 'vhd',
+                    'owner': 'asdf',
+                    'size': '1024',
+                    'min_ram': '512',
+                    'min_disk': '10',
+                    'properties': {'a': 'b', 'c': 'd'},
+                    'is_public': False,
+                    'protected': False,
+                }},
+            ),
+        ),
+        'DELETE': (
+            {
+                'x-openstack-request-id': 'req-1234',
+            },
+            None),
+    },
     '/v1/images/v2_created_img': {
         'PUT': (
             {},
@@ -480,6 +525,12 @@ class ImageManagerTest(testtools.TestCase):
         expect = [('HEAD', '/v1/images/3', {}, None)]
         self.assertEqual(u"ni\xf1o", image.name)
 
+    def test_get_req_id(self):
+        params = {'return_req_id': []}
+        image = self.mgr.get('4', **params)
+        expect_req_id = ['req-1234']
+        self.assertEqual(expect_req_id, params['return_req_id'])
+
     def test_data(self):
         data = ''.join([b for b in self.mgr.data('1', do_checksum=False)])
         expect = [('GET', '/v1/images/1', {}, None)]
@@ -508,6 +559,15 @@ class ImageManagerTest(testtools.TestCase):
             msg = 'was fd7c5c4fdaa97163ee4ba8842baa537a expected wrong'
             self.assertTrue(msg in str(e))
 
+    def test_data_req_id(self):
+        params = {
+            'do_checksum': False,
+            'return_req_id': [],
+        }
+        data = ''.join([b for b in self.mgr.data('4', **params)])
+        expect_req_id = ['req-1234']
+        self.assertEqual(expect_req_id, params['return_req_id'])
+
     def test_data_with_checksum(self):
         data = ''.join([b for b in self.mgr.data('3', do_checksum=False)])
         expect = [('GET', '/v1/images/3', {}, None)]
@@ -524,6 +584,16 @@ class ImageManagerTest(testtools.TestCase):
         expect = [('DELETE', '/v1/images/1', {}, None)]
         self.assertEqual(expect, self.api.calls)
 
+    def test_delete_req_id(self):
+        params = {
+            'return_req_id': []
+        }
+        self.mgr.delete('4', **params)
+        expect = [('DELETE', '/v1/images/4', {}, None)]
+        self.assertEqual(self.api.calls, expect)
+        expect_req_id = ['req-1234']
+        self.assertEqual(expect_req_id, params['return_req_id'])
+
     def test_create_without_data(self):
         params = {
             'id': '1',
@@ -573,6 +643,40 @@ class ImageManagerTest(testtools.TestCase):
         expect = [('POST', '/v1/images', expect_headers, image_data)]
         self.assertEqual(expect, self.api.calls)
 
+    def test_create_req_id(self):
+        params = {
+            'id': '4',
+            'name': 'image-4',
+            'container_format': 'ovf',
+            'disk_format': 'vhd',
+            'owner': 'asdf',
+            'size': 1024,
+            'min_ram': 512,
+            'min_disk': 10,
+            'copy_from': 'http://example.com',
+            'properties': {'a': 'b', 'c': 'd'},
+            'return_req_id': [],
+        }
+        image = self.mgr.create(**params)
+        expect_headers = {
+            'x-image-meta-id': '4',
+            'x-image-meta-name': 'image-4',
+            'x-image-meta-container_format': 'ovf',
+            'x-image-meta-disk_format': 'vhd',
+            'x-image-meta-owner': 'asdf',
+            'x-image-meta-size': '1024',
+            'x-image-meta-min_ram': '512',
+            'x-image-meta-min_disk': '10',
+            'x-glance-api-copy-from': 'http://example.com',
+            'x-image-meta-property-a': 'b',
+            'x-image-meta-property-c': 'd',
+        }
+        expect = [('POST', '/v1/images', expect_headers, None)]
+        self.assertEqual(self.api.calls, expect)
+        self.assertEqual(image.id, '1')
+        expect_req_id = ['req-1234']
+        self.assertEqual(expect_req_id, params['return_req_id'])
+
     def test_update(self):
         fields = {
             'name': 'image-2',
@@ -621,6 +725,18 @@ class ImageManagerTest(testtools.TestCase):
         expect = [('PUT', '/v1/images/1', expect_headers, None)]
         self.assertEqual(expect, self.api.calls)
 
+    def test_update_req_id(self):
+        fields = {
+            'purge_props': True,
+            'return_req_id': [],
+        }
+        self.mgr.update('4', **fields)
+        expect_headers = {'x-glance-registry-purge-props': 'true'}
+        expect = [('PUT', '/v1/images/4', expect_headers, None)]
+        self.assertEqual(self.api.calls, expect)
+        expect_req_id = ['req-1234']
+        self.assertEqual(expect_req_id, fields['return_req_id'])
+
     def test_image_meta_from_headers_encoding(self):
         fields = {"x-image-meta-name": "ni\xc3\xb1o"}
         headers = self.mgr._image_meta_from_headers(fields)
@@ -633,6 +749,15 @@ class ImageManagerTest(testtools.TestCase):
         self.assertEqual('a', image_list[0].id)
         self.assertEqual(1, len(image_list))
 
+    def test_image_list_with_owner_req_id(self):
+        fields = {
+            'owner': 'A',
+            'return_req_id': [],
+        }
+        images = self.mgr.list(**fields)
+        images.next()
+        self.assertEqual(fields['return_req_id'], ['req-1234'])
+
     def test_image_list_with_notfound_owner(self):
         images = self.mgr.list(owner='X', page_size=20)
         self.assertEqual(0, len(list(images)))