Merge "Add retry on volume attach error in Generic driver"

manila/share/drivers/generic.py

@@ -25,6 +25,7 @@ from oslo_log import log
 from oslo_utils import excutils
 from oslo_utils import importutils
 from oslo_utils import strutils
+import retrying
 import six
 
 from manila.common import constants as const
@@ -339,10 +340,16 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
                     raise exception.ManilaException(
                         _('Volume %s is already attached to another instance')
                         % volume['id'])
-            self.compute_api.instance_volume_attach(self.admin_context,
-                                                    instance_id,
-                                                    volume['id'],
-                                                    )
+
+            @retrying.retry(stop_max_attempt_number=3,
+                            wait_fixed=2000,
+                            retry_on_exception=lambda exc: True)
+            def attach_volume():
+                self.compute_api.instance_volume_attach(
+                    self.admin_context, instance_id, volume['id'])
+
+            attach_volume()
+
             t = time.time()
             while time.time() - t < self.configuration.max_time_to_attach:
                 volume = self.volume_api.get(context, volume['id'])

manila/tests/share/drivers/test_generic.py

@@ -433,15 +433,39 @@ class GenericShareDriverTestCase(test.TestCase):
                           self._driver._attach_volume, self._context,
                           self.share, fake_server, attached_volume)
 
-    def test_attach_volume_failed_attach(self):
+    @ddt.data(exception.ManilaException, exception.Invalid)
+    def test_attach_volume_failed_attach(self, side_effect):
         fake_server = fake_compute.FakeServer()
         available_volume = fake_volume.FakeVolume()
         self.mock_object(self._driver.compute_api, 'instance_volume_attach',
-                         mock.Mock(side_effect=exception.ManilaException))
+                         mock.Mock(side_effect=side_effect))
         self.assertRaises(exception.ManilaException,
                           self._driver._attach_volume,
                           self._context, self.share, fake_server,
                           available_volume)
+        self.assertEqual(
+            3, self._driver.compute_api.instance_volume_attach.call_count)
+
+    def test_attach_volume_attached_retry_correct(self):
+        fake_server = fake_compute.FakeServer()
+        attached_volume = fake_volume.FakeVolume(status='available')
+        in_use_volume = fake_volume.FakeVolume(status='in-use')
+
+        side_effect = [exception.Invalid("Fake"), attached_volume]
+        attach_mock = mock.Mock(side_effect=side_effect)
+        self.mock_object(self._driver.compute_api, 'instance_volume_attach',
+                         attach_mock)
+        self.mock_object(self._driver.compute_api, 'instance_volumes_list',
+                         mock.Mock(return_value=[attached_volume]))
+        self.mock_object(self._driver.volume_api, 'get',
+                         mock.Mock(return_value=in_use_volume))
+
+        result = self._driver._attach_volume(self._context, self.share,
+                                             fake_server, attached_volume)
+
+        self.assertEqual(result, in_use_volume)
+        self.assertEqual(
+            2, self._driver.compute_api.instance_volume_attach.call_count)
 
     def test_attach_volume_error(self):
         fake_server = fake_compute.FakeServer()

requirements.txt

@@ -31,6 +31,7 @@ PasteDeploy>=1.5.0
 python-neutronclient>=2.3.11,<3
 keystonemiddleware>=1.5.0
 requests>=2.5.2
+retrying>=1.2.3,!=1.3.0 # Apache-2.0
 Routes>=1.12.3,!=2.0
 six>=1.9.0
 SQLAlchemy>=0.9.7,<1.1.0