Merge "RBD: Fix handling of RBD errors in get_manageable_volumes"

cinder/tests/unit/volume/drivers/test_rbd.py

@@ -81,6 +81,10 @@ class MockPermissionError(MockException):
     errno = errno.EPERM
 
 
+class MockTimeOutException(MockException):
+    """Used as mock for TimeOut."""
+
+
 class MockImageHasSnapshotsException(MockException):
     """Used as mock for rbd.ImageHasSnapshots."""
 
@@ -114,12 +118,14 @@ def common_mocks(f):
             inst.mock_proxy = mock_proxy
             inst.mock_rbd.RBD.Error = Exception
             inst.mock_rados.Error = Exception
+            inst.mock_rbd.Error = Exception
             inst.mock_rbd.ImageBusy = MockImageBusyException
             inst.mock_rbd.ImageNotFound = MockImageNotFoundException
             inst.mock_rbd.ImageExists = MockImageExistsException
             inst.mock_rbd.ImageHasSnapshots = MockImageHasSnapshotsException
             inst.mock_rbd.InvalidArgument = MockInvalidArgument
             inst.mock_rbd.PermissionError = MockPermissionError
+            inst.mock_rbd.TimeOut = MockTimeOutException
 
             inst.driver.rbd = inst.mock_rbd
             aux = inst.driver.rbd
@@ -766,6 +772,32 @@ class RBDTestCase(test.TestCase):
                ]
         self.assertEqual(exp, res)
 
+    @common_mocks
+    @mock.patch.object(driver.RBDDriver, '_get_image_status')
+    def test_get_manageable_volumes_exc(self, mock_get_image_status):
+        cinder_vols = [{'id': '00000000-0000-0000-0000-000000000000'}]
+        vols = ['volume-00000000-0000-0000-0000-000000000000', 'vol1', 'vol2',
+                'volume-11111111-1111-1111-1111-111111111111.deleted']
+        self.mock_rbd.RBD.return_value.list.return_value = vols
+        image = self.mock_proxy.return_value.__enter__.return_value
+        # Four images are present, but the third image can't be opened
+        image.size.side_effect = [2 * units.Gi,
+                                  self.mock_rbd.ImageNotFound,
+                                  self.mock_rbd.TimeOut,
+                                  self.mock_rbd.PermissionError]
+        mock_get_image_status.side_effect = [
+            {'watchers': []},
+            {'watchers': []},
+            {'watchers': []}]
+        res = self.driver.get_manageable_volumes(
+            cinder_vols, None, 1000, 0, ['size'], ['asc'])
+        exp = [{'size': 2, 'reason_not_safe': 'already managed',
+                'extra_info': None, 'safe_to_manage': False,
+                'reference': {'source-name':
+                              'volume-00000000-0000-0000-0000-000000000000'},
+                'cinder_id': '00000000-0000-0000-0000-000000000000'}]
+        self.assertEqual(exp, res)
+
     @common_mocks
     def test_delete_backup_snaps(self):
         self.driver.rbd.Image.remove_snap = mock.Mock()

cinder/volume/drivers/rbd.py

@@ -2201,10 +2201,10 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
         with RADOSClient(self) as client:
             for image_name in self.RBDProxy().list(client.ioctx):
                 image_id = volume_utils.extract_id_from_volume_name(image_name)
-                with RBDVolumeProxy(self, image_name, read_only=True,
+                try:
-                                    client=client.cluster,
+                    with RBDVolumeProxy(self, image_name, read_only=True,
-                                    ioctx=client.ioctx) as image:
+                                        client=client.cluster,
-                    try:
+                                        ioctx=client.ioctx) as image:
                         image_info = {
                             'reference': {'source-name': image_name},
                             'size': int(math.ceil(
@@ -2232,8 +2232,11 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                             image_info['safe_to_manage'] = True
                             image_info['reason_not_safe'] = None
                         manageable_volumes.append(image_info)
-                    except self.rbd.ImageNotFound:
+                except self.rbd.ImageNotFound:
-                        LOG.debug("Image %s is not found.", image_name)
+                    LOG.debug("Image %s is not found.", image_name)
+                except self.rbd.Error as error:
+                    LOG.debug("Cannot open image %(image)s. (%(error)s)",
+                              ({'image': image_name, 'error': error}))
 
         return volume_utils.paginate_entries_list(
             manageable_volumes, marker, limit, offset, sort_keys, sort_dirs)

releasenotes/notes/rbd-bug-2065713-driver-exc-handling-f8de823cd9acd767.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #2065713 <https://bugs.launchpad.net/cinder/+bug/2065713>`_: Due to
+    incorrect exception handling, ImageNotFound errors in the RBD driver's
+    get_manageable_volumes operation would propagate up to the API layer rather
+    than being caught and handled in the driver.