From 9fe92e1474765b31d4ac76a218ddf1e11cddd0ea Mon Sep 17 00:00:00 2001 From: wanghao Date: Wed, 15 Jul 2015 18:09:36 +0800 Subject: [PATCH] Add updated_at into response of listing detail Now when query cinder resource(volume/snapshot/backup) detail, the response of this resource just show created_at to end user, and do not contain the updated_at property. The updated_at time is an important information to end user, for example, they can delete the useless volumes by checking if the volumes have been updated in a period time. Also users can filter resources by using changes_since for all projects in future. Although cinder haven't yet, this is first step to let users know the time of update. On the other hand, if volume is 'in-use', attached_at time should also be returned. APIImpact Add "updated_at: XXX" into response body of querying resource detail. Add "attached_at: XXX" into attachments with response of querying resource detail. Change-Id: I8353e95f5a962f3b89132a65c31999218cbe79dc Implements: blueprint improve-timestamp-in-response-when-querying-cinder-resources --- cinder/api/v2/views/volumes.py | 2 + cinder/api/views/backups.py | 1 + cinder/api/views/snapshots.py | 1 + cinder/tests/unit/api/contrib/test_backups.py | 20 +++++--- cinder/tests/unit/api/v2/test_snapshots.py | 7 +++ cinder/tests/unit/api/v2/test_volumes.py | 50 ++++++++++++------- 6 files changed, 55 insertions(+), 26 deletions(-) diff --git a/cinder/api/v2/views/volumes.py b/cinder/api/v2/views/volumes.py index c72aec760..cd9cd8913 100644 --- a/cinder/api/v2/views/volumes.py +++ b/cinder/api/v2/views/volumes.py @@ -61,6 +61,7 @@ class ViewBuilder(common.ViewBuilder): 'size': volume.get('size'), 'availability_zone': volume.get('availability_zone'), 'created_at': volume.get('created_at'), + 'updated_at': volume.get('updated_at'), 'attachments': self._get_attachments(volume), 'name': volume.get('display_name'), 'description': volume.get('display_description'), @@ -100,6 +101,7 @@ class ViewBuilder(common.ViewBuilder): 'server_id': attachment.get('instance_uuid'), 'host_name': attachment.get('attached_host'), 'device': attachment.get('mountpoint'), + 'attached_at': attachment.get('attach_time'), } attachments.append(a) diff --git a/cinder/api/views/backups.py b/cinder/api/views/backups.py index 91dc4fa5c..3917e2c67 100644 --- a/cinder/api/views/backups.py +++ b/cinder/api/views/backups.py @@ -70,6 +70,7 @@ class ViewBuilder(common.ViewBuilder): 'availability_zone': backup.get('availability_zone'), 'container': backup.get('container'), 'created_at': backup.get('created_at'), + 'updated_at': backup.get('updated_at'), 'name': backup.get('display_name'), 'description': backup.get('display_description'), 'fail_reason': backup.get('fail_reason'), diff --git a/cinder/api/views/snapshots.py b/cinder/api/views/snapshots.py index b1cc3bd32..79a7b0b2d 100644 --- a/cinder/api/views/snapshots.py +++ b/cinder/api/views/snapshots.py @@ -51,6 +51,7 @@ class ViewBuilder(common.ViewBuilder): 'snapshot': { 'id': snapshot.id, 'created_at': snapshot.created_at, + 'updated_at': snapshot.updated_at, 'name': snapshot.display_name, 'description': snapshot.display_description, 'volume_id': snapshot.volume_id, diff --git a/cinder/tests/unit/api/contrib/test_backups.py b/cinder/tests/unit/api/contrib/test_backups.py index 863987aa3..0c2850e05 100644 --- a/cinder/tests/unit/api/contrib/test_backups.py +++ b/cinder/tests/unit/api/contrib/test_backups.py @@ -109,6 +109,7 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(volume_id, res_dict['backup']['volume_id']) self.assertFalse(res_dict['backup']['is_incremental']) self.assertFalse(res_dict['backup']['has_dependent_backups']) + self.assertIn('updated_at', res_dict['backup']) db.backup_destroy(context.get_admin_context(), backup_id) db.volume_destroy(context.get_admin_context(), volume_id) @@ -282,7 +283,7 @@ class BackupsAPITestCase(test.TestCase): res_dict = json.loads(res.body) self.assertEqual(200, res.status_int) - self.assertEqual(14, len(res_dict['backups'][0])) + self.assertEqual(15, len(res_dict['backups'][0])) self.assertEqual('az1', res_dict['backups'][0]['availability_zone']) self.assertEqual('volumebackups', res_dict['backups'][0]['container']) @@ -295,8 +296,9 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(0, res_dict['backups'][0]['size']) self.assertEqual('creating', res_dict['backups'][0]['status']) self.assertEqual('1', res_dict['backups'][0]['volume_id']) + self.assertIn('updated_at', res_dict['backups'][0]) - self.assertEqual(14, len(res_dict['backups'][1])) + self.assertEqual(15, len(res_dict['backups'][1])) self.assertEqual('az1', res_dict['backups'][1]['availability_zone']) self.assertEqual('volumebackups', res_dict['backups'][1]['container']) @@ -309,8 +311,9 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(0, res_dict['backups'][1]['size']) self.assertEqual('creating', res_dict['backups'][1]['status']) self.assertEqual('1', res_dict['backups'][1]['volume_id']) + self.assertIn('updated_at', res_dict['backups'][1]) - self.assertEqual(14, len(res_dict['backups'][2])) + self.assertEqual(15, len(res_dict['backups'][2])) self.assertEqual('az1', res_dict['backups'][2]['availability_zone']) self.assertEqual('volumebackups', res_dict['backups'][2]['container']) self.assertEqual('this is a test backup', @@ -322,6 +325,7 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(0, res_dict['backups'][2]['size']) self.assertEqual('creating', res_dict['backups'][2]['status']) self.assertEqual('1', res_dict['backups'][2]['volume_id']) + self.assertIn('updated_at', res_dict['backups'][2]) db.backup_destroy(context.get_admin_context(), backup_id3) db.backup_destroy(context.get_admin_context(), backup_id2) @@ -465,9 +469,9 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(200, res.status_int) self.assertEqual(2, len(res_dict['backups'])) - self.assertEqual(14, len(res_dict['backups'][0])) + self.assertEqual(15, len(res_dict['backups'][0])) self.assertEqual(backup_id3, res_dict['backups'][0]['id']) - self.assertEqual(14, len(res_dict['backups'][1])) + self.assertEqual(15, len(res_dict['backups'][1])) self.assertEqual(backup_id2, res_dict['backups'][1]['id']) db.backup_destroy(context.get_admin_context(), backup_id3) @@ -488,9 +492,9 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(200, res.status_int) self.assertEqual(2, len(res_dict['backups'])) - self.assertEqual(14, len(res_dict['backups'][0])) + self.assertEqual(15, len(res_dict['backups'][0])) self.assertEqual(backup_id2, res_dict['backups'][0]['id']) - self.assertEqual(14, len(res_dict['backups'][1])) + self.assertEqual(15, len(res_dict['backups'][1])) self.assertEqual(backup_id1, res_dict['backups'][1]['id']) db.backup_destroy(context.get_admin_context(), backup_id3) @@ -511,7 +515,7 @@ class BackupsAPITestCase(test.TestCase): self.assertEqual(200, res.status_int) self.assertEqual(1, len(res_dict['backups'])) - self.assertEqual(14, len(res_dict['backups'][0])) + self.assertEqual(15, len(res_dict['backups'][0])) self.assertEqual(backup_id2, res_dict['backups'][0]['id']) db.backup_destroy(context.get_admin_context(), backup_id3) diff --git a/cinder/tests/unit/api/v2/test_snapshots.py b/cinder/tests/unit/api/v2/test_snapshots.py index 26352ea47..e37a3dccc 100644 --- a/cinder/tests/unit/api/v2/test_snapshots.py +++ b/cinder/tests/unit/api/v2/test_snapshots.py @@ -48,6 +48,7 @@ def _get_default_snapshot_param(): 'status': 'available', 'volume_size': 100, 'created_at': None, + 'updated_at': None, 'user_id': 'bcb7746c7a41472d88a1ffac89ba6a9b', 'project_id': '7ffe17a15c724e2aa79fc839540aec15', 'display_name': 'Default name', @@ -109,6 +110,7 @@ class SnapshotApiTest(test.TestCase): self.assertEqual(snapshot_description, resp_dict['snapshot']['description']) self.assertTrue(mock_validate.called) + self.assertIn('updated_at', resp_dict['snapshot']) db.volume_destroy(self.ctx, volume.id) def test_snapshot_create_force(self): @@ -130,6 +132,7 @@ class SnapshotApiTest(test.TestCase): resp_dict['snapshot']['name']) self.assertEqual(snapshot_description, resp_dict['snapshot']['description']) + self.assertIn('updated_at', resp_dict['snapshot']) snapshot = { "volume_id": volume.id, @@ -198,6 +201,7 @@ class SnapshotApiTest(test.TestCase): 'status': u'available', 'size': 100, 'created_at': None, + 'updated_at': None, 'name': u'Updated Test Name', 'description': u'Default description', 'metadata': {}, @@ -287,6 +291,7 @@ class SnapshotApiTest(test.TestCase): self.assertIn('snapshot', resp_dict) self.assertEqual(UUID, resp_dict['snapshot']['id']) + self.assertIn('updated_at', resp_dict['snapshot']) def test_snapshot_show_invalid_id(self): snapshot_id = INVALID_UUID @@ -323,6 +328,7 @@ class SnapshotApiTest(test.TestCase): self.assertIn('snapshots', resp_dict) resp_snapshots = resp_dict['snapshots'] self.assertEqual(1, len(resp_snapshots)) + self.assertIn('updated_at', resp_snapshots[0]) resp_snapshot = resp_snapshots.pop() self.assertEqual(UUID, resp_snapshot['id']) @@ -349,6 +355,7 @@ class SnapshotApiTest(test.TestCase): self.assertIn('snapshots', res) self.assertEqual(1, len(res['snapshots'])) self.assertEqual(snaps[1].id, res['snapshots'][0]['id']) + self.assertIn('updated_at', res['snapshots'][0]) # Test that we get an empty list with an offset greater than the # number of items diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 73697464c..f67d1b5ab 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -171,6 +171,7 @@ class VolumeApiTest(test.TestCase): 'bootable': 'false', 'consistencygroup_id': consistencygroup_id, 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'updated_at': datetime.datetime(1900, 1, 1, 1, 1, 1), 'description': description, 'id': stubs.DEFAULT_VOL_ID, 'links': @@ -637,7 +638,9 @@ class VolumeApiTest(test.TestCase): attachment = db.volume_attach(context.get_admin_context(), values) db.volume_attached(context.get_admin_context(), attachment['id'], stubs.FAKE_UUID, None, '/') - + attach_tmp = db.volume_attachment_get(context.get_admin_context(), + attachment['id']) + volume_tmp = db.volume_get(context.get_admin_context(), '1') updates = { "name": "Updated Test Name", } @@ -650,16 +653,17 @@ class VolumeApiTest(test.TestCase): expected = self._expected_vol_from_controller( availability_zone=stubs.DEFAULT_AZ, volume_type=None, status='in-use', name='Updated Test Name', - attachments=[{ - 'id': '1', - 'attachment_id': attachment['id'], - 'volume_id': stubs.DEFAULT_VOL_ID, - 'server_id': stubs.FAKE_UUID, - 'host_name': None, - 'device': '/', - }], + attachments=[{'id': '1', + 'attachment_id': attachment['id'], + 'volume_id': stubs.DEFAULT_VOL_ID, + 'server_id': stubs.FAKE_UUID, + 'host_name': None, + 'device': '/', + 'attached_at': attach_tmp['attach_time'], + }], metadata={'key': 'value', 'readonly': 'True'}, with_migration_status=True) + expected['volume']['updated_at'] = volume_tmp['updated_at'] self.assertEqual(expected, res_dict) self.assertEqual(2, len(self.notifier.notifications)) self.assertTrue(mock_validate.called) @@ -751,6 +755,9 @@ class VolumeApiTest(test.TestCase): attachment = db.volume_attach(context.get_admin_context(), values) db.volume_attached(context.get_admin_context(), attachment['id'], stubs.FAKE_UUID, None, '/') + attach_tmp = db.volume_attachment_get(context.get_admin_context(), + attachment['id']) + volume_tmp = db.volume_get(context.get_admin_context(), '1') req = fakes.HTTPRequest.blank('/v2/volumes/detail') admin_ctx = context.RequestContext('admin', 'fakeproject', True) @@ -764,9 +771,12 @@ class VolumeApiTest(test.TestCase): 'server_id': stubs.FAKE_UUID, 'host_name': None, 'id': '1', - 'volume_id': stubs.DEFAULT_VOL_ID}], + 'volume_id': stubs.DEFAULT_VOL_ID, + 'attached_at': attach_tmp['attach_time'], + }], metadata={'key': 'value', 'readonly': 'True'}, with_migration_status=True) + exp_vol['volume']['updated_at'] = volume_tmp['updated_at'] expected = {'volumes': [exp_vol['volume']]} self.assertEqual(expected, res_dict) @@ -1167,7 +1177,9 @@ class VolumeApiTest(test.TestCase): attachment = db.volume_attach(context.get_admin_context(), values) db.volume_attached(context.get_admin_context(), attachment['id'], stubs.FAKE_UUID, None, '/') - + attach_tmp = db.volume_attachment_get(context.get_admin_context(), + attachment['id']) + volume_tmp = db.volume_get(context.get_admin_context(), '1') req = fakes.HTTPRequest.blank('/v2/volumes/1') admin_ctx = context.RequestContext('admin', 'fakeproject', True) req.environ['cinder.context'] = admin_ctx @@ -1175,15 +1187,17 @@ class VolumeApiTest(test.TestCase): expected = self._expected_vol_from_controller( availability_zone=stubs.DEFAULT_AZ, volume_type=None, status='in-use', - attachments=[{ - 'id': '1', - 'attachment_id': attachment['id'], - 'volume_id': stubs.DEFAULT_VOL_ID, - 'server_id': stubs.FAKE_UUID, - 'host_name': None, - 'device': '/'}], + attachments=[{'id': '1', + 'attachment_id': attachment['id'], + 'volume_id': stubs.DEFAULT_VOL_ID, + 'server_id': stubs.FAKE_UUID, + 'host_name': None, + 'device': '/', + 'attached_at': attach_tmp['attach_time'], + }], metadata={'key': 'value', 'readonly': 'True'}, with_migration_status=True) + expected['volume']['updated_at'] = volume_tmp['updated_at'] self.assertEqual(expected, res_dict) def test_volume_show_with_encrypted_volume(self): -- 2.45.2