RBD: Fix handling of RBD errors in get_manageable_volumes

The "except ImageNotFound" was outside of the required try block,
so ImageNotFound errors would result in exceptions being thrown
up to the API layer.

Add handling for other errors that could be thrown by RBD
here, too.

Reference info:
https://github.com/ceph/ceph/blob/08d7ff952d/src/pybind/rbd/rbd.pyx#L8

Closes-Bug: #2065713
Change-Id: Ifaceb04263973b1445c7f13a6155dc63647d7176

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

@@ -2205,10 +2205,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(
@@ -2236,8 +2236,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.