openstack/cinder
Code Issues Proposed changes

RBD: Handle ImageNotFound exception in _get_usage_info correctly

Browse Source 
Change https://review.openstack.org/#/c/486734 moved try-except
statement to the wrong place. We need to open image in Ceph inside
try-except block.

This patch also fixes race condition between volume deletion and
_get_usage_info calls.

Closed-Bug: #1765845
Change-Id: I7d3d006b023ca4b7963c4c684e4c036399d1295c
Co-Authored-By: Melanie Witt <melwittt@gmail.com>
This commit is contained in:
Ivan Kolodyazhny 2018-04-21 00:02:13 +03:00 committed by melanie witt
parent a49eb33a66
commit 3cc8cbdee7
2 changed files with 26 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
cinder/tests/unit/volume/drivers/test_rbd.py
View File

@ -1968,38 +1968,33 @@ class RBDTestCase(test.TestCase):
@mock.patch('cinder.volume.drivers.rbd.RADOSClient') @mock.patch('cinder.volume.drivers.rbd.RADOSClient')
@mock.patch('cinder.volume.drivers.rbd.RBDDriver.RBDProxy') @mock.patch('cinder.volume.drivers.rbd.RBDDriver.RBDProxy')
def test__get_usage_info(self, rbdproxy_mock, client_mock, volproxy_mock): def test__get_usage_info(self, rbdproxy_mock, client_mock, volproxy_mock):
def FakeVolProxy(size):
if size == -1: def FakeVolProxy(size_or_exc):
size_mock = mock.Mock(side_effect=MockImageNotFoundException) return mock.Mock(return_value=mock.Mock(
else: size=mock.Mock(side_effect=(size_or_exc,))))
size_mock = mock.Mock(return_value=size * units.Gi)
return mock.Mock(return_value=mock.Mock(size=size_mock))
volumes = ['volume-1', 'non-existent', 'non-cinder-volume'] volumes = ['volume-1', 'non-existent', 'non-cinder-volume']
client = client_mock.return_value.__enter__.return_value client = client_mock.return_value.__enter__.return_value
rbdproxy_mock.return_value.list.return_value = volumes rbdproxy_mock.return_value.list.return_value = volumes
volproxy_mock.side_effect = [ with mock.patch.object(self.driver, 'rbd',
mock.Mock(**{'__enter__': FakeVolProxy(1.0), ImageNotFound=MockImageNotFoundException):
'__exit__': mock.Mock()}), volproxy_mock.side_effect = [
mock.Mock(**{'__enter__': FakeVolProxy(-1), mock.MagicMock(**{'__enter__': FakeVolProxy(s)})
'__exit__': mock.Mock()}), for s in (1.0 * units.Gi,
mock.Mock(**{'__enter__': FakeVolProxy(2.0), self.driver.rbd.ImageNotFound,
'__exit__': mock.Mock()}) 2.0 * units.Gi)
] ]
with mock.patch.object(self.driver, 'rbd') as mock_rbd:
mock_rbd.ImageNotFound = MockImageNotFoundException
total_provision = self.driver._get_usage_info() total_provision = self.driver._get_usage_info()
rbdproxy_mock.return_value.list.assert_called_once_with(client.ioctx) rbdproxy_mock.return_value.list.assert_called_once_with(client.ioctx)
volproxy_mock.assert_has_calls([
mock.call(self.driver, volumes[0], read_only=True, expected_volproxy_calls = [
client=client.cluster, ioctx=client.ioctx), mock.call(self.driver, v, read_only=True,
mock.call(self.driver, volumes[1], read_only=True, client=client.cluster, ioctx=client.ioctx)
client=client.cluster, ioctx=client.ioctx), for v in volumes]
]) self.assertEqual(expected_volproxy_calls, volproxy_mock.mock_calls)
self.assertEqual(3.00, total_provision) self.assertEqual(3.00, total_provision)

16
cinder/volume/drivers/rbd.py
View File

@ -403,15 +403,15 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
total_provisioned = 0 total_provisioned = 0
with RADOSClient(self) as client: with RADOSClient(self) as client:
for t in self.RBDProxy().list(client.ioctx): for t in self.RBDProxy().list(client.ioctx):
with RBDVolumeProxy(self, t, read_only=True, try:
client=client.cluster, with RBDVolumeProxy(self, t, read_only=True,
ioctx=client.ioctx) as v: client=client.cluster,
try: ioctx=client.ioctx) as v:
size = v.size() size = v.size()
except self.rbd.ImageNotFound: except self.rbd.ImageNotFound:
LOG.debug("Image %s is not found.", t) LOG.debug("Image %s is not found.", t)
else: else:
total_provisioned += size total_provisioned += size
total_provisioned = math.ceil(float(total_provisioned) / units.Gi) total_provisioned = math.ceil(float(total_provisioned) / units.Gi)
return total_provisioned return total_provisioned