openstack/deb-cinder
Code Issues Proposed changes

Enforce _usage_from_snapshot to fetch info from deleted volume

Browse Source 
The fix introduced for LP: #1631561 enforces the availability_zone to be
a empty string if the volume cannot be loaded because it has been marked
as deleted. This change enforces the load of the deleted volume in order
to load the volume's availability_zone.

Change-Id: I926f90d49fa0b352975232a68be5fbb2f8304d3b
Closes-Bug: #1666686
(cherry picked from commit dda0090696)
This commit is contained in:
Jorge Niedbalski 2017-02-23 14:47:54 -03:00 committed by Jorge Niedbalski R.
parent a7521f509d
commit 1831ee7d42
3 changed files with 23 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cinder/tests/unit/api/v2/test_snapshots.py
View File

@ -191,12 +191,12 @@ class SnapshotApiTest(test.TestCase):
@mock.patch.object(volume.api.API, "update_snapshot",
side_effect=v2_fakes.fake_snapshot_update)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
@mock.patch('cinder.objects.Volume.get_by_id')
@mock.patch('cinder.db.volume_get')
@mock.patch('cinder.objects.Snapshot.get_by_id')
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
def test_snapshot_update(
self, mock_validate, snapshot_get_by_id, volume_get_by_id,
self, mock_validate, snapshot_get_by_id, volume_get,
snapshot_metadata_get, update_snapshot):
snapshot = {
'id': UUID,
@ -211,7 +211,7 @@ class SnapshotApiTest(test.TestCase):
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctx, **snapshot)
fake_volume_obj = fake_volume.fake_volume_obj(ctx)
snapshot_get_by_id.return_value = snapshot_obj
volume_get_by_id.return_value = fake_volume_obj
volume_get.return_value = fake_volume_obj
updates = {
"name": "Updated Test Name",

21
cinder/tests/unit/test_volume_utils.py
View File

@ -97,7 +97,8 @@ class NotifyUsageTestCase(test.TestCase):
mock.sentinel.snapshot,
'test_suffix')
self.assertIsNone(output)
mock_usage.assert_called_once_with(mock.sentinel.snapshot)
mock_usage.assert_called_once_with(mock.sentinel.snapshot,
mock.sentinel.context)
mock_rpc.get_notifier.assert_called_once_with('snapshot', 'host1')
mock_rpc.get_notifier.return_value.info.assert_called_once_with(
mock.sentinel.context,
@ -118,6 +119,7 @@ class NotifyUsageTestCase(test.TestCase):
host='host2')
self.assertIsNone(output)
mock_usage.assert_called_once_with(mock.sentinel.snapshot,
mock.sentinel.context,
a='b', c='d')
mock_rpc.get_notifier.assert_called_once_with('snapshot', 'host2')
mock_rpc.get_notifier.return_value.info.assert_called_once_with(
@ -125,15 +127,15 @@ class NotifyUsageTestCase(test.TestCase):
'snapshot.test_suffix',
mock_usage.return_value)
@mock.patch('cinder.objects.Volume.get_by_id')
def test_usage_from_snapshot(self, volume_get_by_id):
@mock.patch('cinder.db.volume_get')
def test_usage_from_snapshot(self, volume_get):
raw_volume = {
'id': fake.VOLUME_ID,
'availability_zone': 'nova'
}
ctxt = context.get_admin_context()
volume_obj = fake_volume.fake_volume_obj(ctxt, **raw_volume)
volume_get_by_id.return_value = volume_obj
volume_get.return_value = volume_obj
raw_snapshot = {
'project_id': fake.PROJECT_ID,
'user_id': fake.USER_ID,
@ -151,7 +153,7 @@ class NotifyUsageTestCase(test.TestCase):
}
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctxt, **raw_snapshot)
usage_info = volume_utils._usage_from_snapshot(snapshot_obj)
usage_info = volume_utils._usage_from_snapshot(snapshot_obj, ctxt)
expected_snapshot = {
'tenant_id': fake.PROJECT_ID,
'user_id': fake.USER_ID,
@ -168,8 +170,8 @@ class NotifyUsageTestCase(test.TestCase):
}
self.assertDictEqual(expected_snapshot, usage_info)
@mock.patch('cinder.objects.Volume.get_by_id')
def test_usage_from_deleted_snapshot(self, volume_get_by_id):
@mock.patch('cinder.db.volume_get')
def test_usage_from_deleted_snapshot(self, volume_get):
raw_volume = {
'id': fake.VOLUME_ID,
'availability_zone': 'nova',
@ -177,8 +179,7 @@ class NotifyUsageTestCase(test.TestCase):
}
ctxt = context.get_admin_context()
volume_obj = fake_volume.fake_volume_obj(ctxt, **raw_volume)
volume_get_by_id.side_effect = exception.VolumeNotFound(
volume_id=fake.VOLUME_ID)
volume_get.return_value = volume_obj
raw_snapshot = {
'project_id': fake.PROJECT_ID,
@ -197,7 +198,7 @@ class NotifyUsageTestCase(test.TestCase):
}
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctxt, **raw_snapshot)
usage_info = volume_utils._usage_from_snapshot(snapshot_obj)
usage_info = volume_utils._usage_from_snapshot(snapshot_obj, ctxt)
expected_snapshot = {
'tenant_id': fake.PROJECT_ID,
'user_id': fake.USER_ID,

24
cinder/volume/utils.py
View File

@ -154,23 +154,17 @@ def notify_about_backup_usage(context, backup, event_suffix,
usage_info)
def _usage_from_snapshot(snapshot, **extra_usage_info):
try:
az = snapshot.volume['availability_zone']
except exception.VolumeNotFound:
# (zhiteng) Snapshot's source volume could have been deleted
# (which means snapshot has been deleted as well),
# lazy-loading volume would raise VolumeNotFound exception.
# In that case, not going any further by abusing low level
# DB API to fetch deleted volume but simply return empty
# string for snapshot's AZ info.
az = ''
LOG.debug("Source volume %s deleted", snapshot.volume_id)
def _usage_from_snapshot(snapshot, context, **extra_usage_info):
# (niedbalski) a snapshot might be related to a deleted
# volume, if that's the case, the volume information is still
# required for filling the usage_info, so we enforce to read
# the volume data even if the volume has been deleted.
context.read_deleted = "yes"
volume = db.volume_get(context, snapshot.volume_id)
usage_info = {
'tenant_id': snapshot.project_id,
'user_id': snapshot.user_id,
'availability_zone': az,
'availability_zone': volume['availability_zone'],
'volume_id': snapshot.volume_id,
'volume_size': snapshot.volume_size,
'snapshot_id': snapshot.id,
@ -194,7 +188,7 @@ def notify_about_snapshot_usage(context, snapshot, event_suffix,
if not extra_usage_info:
extra_usage_info = {}
usage_info = _usage_from_snapshot(snapshot, **extra_usage_info)
usage_info = _usage_from_snapshot(snapshot, context, **extra_usage_info)
rpc.get_notifier('snapshot', host).info(context,
'snapshot.%s' % event_suffix,