openstack
/
manila
Code Issues Proposed changes

Hide snapshots with no instances from listing 

When a snapshot is being destroyed; for a small window of time
between the deletion of all of its instances and the snapshot ref
itself, the 'status' of the snapshot becomes 'None'. This provides
a bad user experience if a list/show query is made within that
window.

Fix this problem by filtering out snapshots that have this 'invalid'
status.

Corrected a few unit tests, to use 'status' as a manadatory
field in snapshot record.

Change-Id: I2ec83eecc9788278459846502625df7fde1fccbd
Closes-Bug: #1503390
This commit is contained in:
nidhimittalhada 2015-12-24 17:31:28 +05:30
parent b2947757e0
commit 12548c9cea
2 changed files with 67 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
manila/api/v1/share_snapshots.py
View File

@ -76,6 +76,10 @@ class ShareSnapshotsController(wsgi.Controller, wsgi.AdminActionsMixin):
try:
snapshot = self.share_api.get_snapshot(context, id)
# Snapshot with no instances is filtered out.
if(snapshot.get('status') is None):
raise exc.HTTPNotFound()
except exception.NotFound:
raise exc.HTTPNotFound()
@ -130,6 +134,11 @@ class ShareSnapshotsController(wsgi.Controller, wsgi.AdminActionsMixin):
sort_key=sort_key,
sort_dir=sort_dir,
)
# Snapshots with no instances are filtered out.
snapshots = list(filter(lambda x: x.get('status') is not None,
snapshots))
limited_list = common.limited(snapshots, req)
if is_detail:
snapshots = self._view_builder.detail_list(req, limited_list)

63
manila/tests/api/v1/test_share_snapshots.py
View File

@ -57,6 +57,20 @@ class ShareSnapshotAPITest(test.TestCase):
}
self.maxDiff = None
def test_snapshot_show_status_none(self):
return_snapshot = {
'share_id': 100,
'name': 'fake_share_name',
'description': 'fake_share_description',
'status': None,
}
self.mock_object(share_api.API, 'get_snapshot',
mock.Mock(return_value=return_snapshot))
req = fakes.HTTPRequest.blank('/snapshots/200')
self.assertRaises(webob.exc.HTTPNotFound,
self.controller.show,
req, '200')
@ddt.data('true', 'True', '<is> True', '1')
def test_snapshot_create(self, snapshot_support):
self.mock_object(share_api.API, 'create_snapshot',
@ -224,9 +238,12 @@ class ShareSnapshotAPITest(test.TestCase):
req = fakes.HTTPRequest.blank(url, use_admin_context=use_admin_context)
snapshots = [
{'id': 'id1', 'display_name': 'n1'},
{'id': 'id2', 'display_name': 'n2'},
{'id': 'id3', 'display_name': 'n3'},
{'id': 'id1', 'display_name': 'n1',
'status': 'fake_status', 'share_id': 'fake_share_id'},
{'id': 'id2', 'display_name': 'n2',
'status': 'fake_status', 'share_id': 'fake_share_id'},
{'id': 'id3', 'display_name': 'n3',
'status': 'fake_status', 'share_id': 'fake_share_id'},
]
self.mock_object(share_api.API, 'get_all_snapshots',
mock.Mock(return_value=snapshots))
@ -274,14 +291,24 @@ class ShareSnapshotAPITest(test.TestCase):
req = fakes.HTTPRequest.blank(url, use_admin_context=use_admin_context)
snapshots = [
{'id': 'id1', 'display_name': 'n1'},
{
'id': 'id1',
'display_name': 'n1',
'status': 'fake_status',
'share_id': 'fake_share_id',
},
{
'id': 'id2',
'display_name': 'n2',
'status': 'fake_status',
'share_id': 'fake_share_id',
},
{'id': 'id3', 'display_name': 'n3'},
{
'id': 'id3',
'display_name': 'n3',
'status': 'fake_status',
'share_id': 'fake_share_id',
},
]
self.mock_object(share_api.API, 'get_all_snapshots',
@ -349,6 +376,32 @@ class ShareSnapshotAPITest(test.TestCase):
}
self.assertEqual(expected, res_dict)
def test_snapshot_list_status_none(self):
snapshots = [
{
'id': 2,
'share_id': 'fakeshareid',
'size': 1,
'status': 'fakesnapstatus',
'name': 'displaysnapname',
'description': 'displaysnapdesc',
},
{
'id': 3,
'share_id': 'fakeshareid',
'size': 1,
'status': None,
'name': 'displaysnapname',
'description': 'displaysnapdesc',
}
]
self.mock_object(share_api.API, 'get_all_snapshots',
mock.Mock(return_value=snapshots))
req = fakes.HTTPRequest.blank('/snapshots')
result = self.controller.index(req)
self.assertEqual(1, len(result['snapshots']))
self.assertEqual(snapshots[0]['id'], result['snapshots'][0]['id'])
def test_snapshot_updates_description(self):
snp = self.snp_example
body = {"snapshot": snp}