From: Edward Hope-Morley <edward.hope-morley@canonical.com>
Date: Sun, 29 Jun 2014 18:08:46 +0000 (+0100)
Subject: Ensure rbd connect exception is properly caught
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=02e4afbd8108ca26af09ccaebb8b1c8b72ea3f3f;p=openstack-build%2Fcinder-build.git

Ensure rbd connect exception is properly caught

If the rbd driver fails to connect to Ceph the exception
was not being properly caught resulting in the volume
remaining in the 'creating' state until the corresponding
task eventually times out (on top of the time it took
for the connect to fail).

Also added config option for rados connect timeout.

DocImpact: new config option 'rados_connect_timout'
Closes-Bug: 1211839
Change-Id: I5e6eaaaf6bed3e139ff476ecf9510ebe214a83f9
---

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index e20338601..43470df15 100644
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -698,6 +698,9 @@ class RBDTestCase(test.TestCase):
 
     @common_mocks
     def test_connect_to_rados(self):
+        # Default
+        self.cfg.rados_connect_timeout = -1
+
         self.mock_rados.Rados.connect = mock.Mock()
         self.mock_rados.Rados.shutdown = mock.Mock()
         self.mock_rados.Rados.open_ioctx = mock.Mock()
@@ -707,6 +710,8 @@ class RBDTestCase(test.TestCase):
         # default configured pool
         ret = self.driver._connect_to_rados()
         self.assertTrue(self.mock_rados.Rados.connect.called)
+        # Expect no timeout if default is used
+        self.mock_rados.Rados.connect.assert_called_once_with()
         self.assertTrue(self.mock_rados.Rados.open_ioctx.called)
         self.assertIsInstance(ret[0], self.mock_rados.Rados)
         self.assertEqual(ret[1], self.mock_rados.Rados.ioctx)
@@ -720,11 +725,18 @@ class RBDTestCase(test.TestCase):
         self.assertEqual(ret[1], self.mock_rados.Rados.ioctx)
         self.mock_rados.Rados.open_ioctx.assert_called_with('alt_pool')
 
+        # With timeout
+        self.cfg.rados_connect_timeout = 1
+        self.mock_rados.Rados.connect.reset_mock()
+        self.driver._connect_to_rados()
+        self.mock_rados.Rados.connect.assert_called_once_with(timeout=1)
+
         # error
         self.mock_rados.Rados.open_ioctx.reset_mock()
         self.mock_rados.Rados.shutdown.reset_mock()
         self.mock_rados.Rados.open_ioctx.side_effect = self.mock_rados.Error
-        self.assertRaises(self.mock_rados.Error, self.driver._connect_to_rados)
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver._connect_to_rados)
         self.mock_rados.Rados.open_ioctx.assert_called_once()
         self.mock_rados.Rados.shutdown.assert_called_once()
 
diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py
index 4b3afed78..13377e21a 100644
--- a/cinder/volume/drivers/rbd.py
+++ b/cinder/volume/drivers/rbd.py
@@ -73,6 +73,10 @@ rbd_opts = [
     cfg.IntOpt('rbd_store_chunk_size', default=4,
                help=_('Volumes will be chunked into objects of this size '
                       '(in megabytes).')),
+    cfg.IntOpt('rados_connect_timeout', default=-1,
+               help=_('Timeout value (in seconds) used when connecting to '
+                      'ceph cluster. If value < 0, no timeout is set and '
+                      'default librados value is used.'))
 ]
 
 CONF = cfg.CONF
@@ -281,6 +285,9 @@ class RBDDriver(driver.VolumeDriver):
         return args
 
     def _connect_to_rados(self, pool=None):
+        LOG.debug("opening connection to ceph cluster (timeout=%s)." %
+                  (self.configuration.rados_connect_timeout))
+
         client = self.rados.Rados(rados_id=self.configuration.rbd_user,
                                   conffile=self.configuration.rbd_ceph_conf)
         if pool is not None:
@@ -289,13 +296,18 @@ class RBDDriver(driver.VolumeDriver):
             pool = self.configuration.rbd_pool
 
         try:
-            client.connect()
+            if self.configuration.rados_connect_timeout >= 0:
+                client.connect(timeout=
+                               self.configuration.rados_connect_timeout)
+            else:
+                client.connect()
             ioctx = client.open_ioctx(pool)
             return client, ioctx
-        except self.rados.Error:
+        except self.rados.Error as exc:
+            LOG.error("error connecting to ceph cluster.")
             # shutdown cannot raise an exception
             client.shutdown()
-            raise
+            raise exception.VolumeBackendAPIException(data=str(exc))
 
     def _disconnect_from_rados(self, client, ioctx):
         # closing an ioctx cannot raise an exception
diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample
index f5a8cc005..1cd028ad4 100644
--- a/etc/cinder/cinder.conf.sample
+++ b/etc/cinder/cinder.conf.sample
@@ -1583,6 +1583,11 @@
 # megabytes). (integer value)
 #rbd_store_chunk_size=4
 
+# Timeout value (in seconds) used when connecting to ceph
+# cluster. If value < 0, no timeout is set and default
+# librados value is used. (integer value)
+#rados_connect_timeout=-1
+
 
 #
 # Options defined in cinder.volume.drivers.san.hp.hp_3par_common