From f41e3c220c209aab35691e95a6db3a8ac303ea09 Mon Sep 17 00:00:00 2001
From: Thomas Bechtold <tbechtold@suse.com>
Date: Fri, 12 May 2017 15:54:48 +0200
Subject: [PATCH] Retry backend initialization

Since commit 4b87f6f40d1f09b4, exceptions in the init_host() call (which is
called for every backend) are catched and backends might end up
uninitialized and unusable. This might be okish (but is not good) in a
multi-backend scenario but is definitely wrong in a single backend scenario.
In that case, the manila-share process would successfully start but the
backend would never be usable.
So retry to initialize the driver for every backend in case there was an error
during initialization. That way even a temporary broken backend can be
initialized later without restarting manila-share.

Change-Id: I2194c61fa9e9bdb32d252284eea1864151d9eef7
Closes-Bug: #1690159
---
 manila/share/manager.py                       | 42 ++++++++++---------
 manila/tests/share/test_manager.py            | 21 +++++++---
 ...9-retry-backend-init-58486ea420feaf51.yaml |  5 +++
 3 files changed, 43 insertions(+), 25 deletions(-)
 create mode 100644 releasenotes/notes/bug-1690159-retry-backend-init-58486ea420feaf51.yaml

diff --git a/manila/share/manager.py b/manila/share/manager.py
index a455b30550..9e75f3fc86 100644
--- a/manila/share/manager.py
+++ b/manila/share/manager.py
@@ -270,27 +270,29 @@ class ShareManager(manager.SchedulerDependentManager):
         """Initialization for a standalone service."""
 
         ctxt = context.get_admin_context()
-        LOG.debug("Start initialization of driver: '%(driver)s"
-                  "@%(host)s'",
-                  {"driver": self.driver.__class__.__name__,
-                   "host": self.host})
-        try:
-            self.driver.do_setup(ctxt)
-            self.driver.check_for_setup_error()
-        except Exception:
-            LOG.exception(
-                ("Error encountered during initialization of driver "
-                 "'%(name)s' on '%(host)s' host."), {
-                    "name": self.driver.__class__.__name__,
-                    "host": self.host,
-                }
-            )
+        driver_host_pair = "{}@{}".format(
+            self.driver.__class__.__name__,
+            self.host)
+
+        # we want to retry to setup the driver. In case of a multi-backend
+        # scenario, working backends are usable and the non-working ones (where
+        # do_setup() or check_for_setup_error() fail) retry.
+        @utils.retry(Exception, interval=2, backoff_rate=2,
+                     backoff_sleep_max=600, retries=0)
+        def _driver_setup():
             self.driver.initialized = False
-            # we don't want to continue since we failed
-            # to initialize the driver correctly.
-            return
-        else:
-            self.driver.initialized = True
+            LOG.debug("Start initialization of driver: '%s'", driver_host_pair)
+            try:
+                self.driver.do_setup(ctxt)
+                self.driver.check_for_setup_error()
+            except Exception:
+                LOG.exception("Error encountered during initialization of "
+                              "driver %s", driver_host_pair)
+                raise
+            else:
+                self.driver.initialized = True
+
+        _driver_setup()
 
         share_instances = self.db.share_instances_get_all_by_host(ctxt,
                                                                   self.host)
diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py
index e01805d752..f010ff02e0 100644
--- a/manila/tests/share/test_manager.py
+++ b/manila/tests/share/test_manager.py
@@ -52,6 +52,10 @@ def fake_replica(**kwargs):
     return fakes.fake_replica(for_manager=True, **kwargs)
 
 
+class CustomTimeSleepException(Exception):
+    pass
+
+
 class LockedOperationsTestCase(test.TestCase):
 
     class FakeManager(object):
@@ -219,8 +223,11 @@ class ShareManagerTestCase(test.TestCase):
     def test_call_driver_when_its_init_failed(self, method_name):
         self.mock_object(self.share_manager.driver, 'do_setup',
                          mock.Mock(side_effect=Exception()))
-        self.share_manager.init_host()
-
+        # break the endless retry loop
+        with mock.patch("time.sleep",
+                        side_effect=CustomTimeSleepException()):
+            self.assertRaises(CustomTimeSleepException,
+                              self.share_manager.init_host)
         self.assertRaises(
             exception.DriverNotInitialized,
             getattr(self.share_manager, method_name),
@@ -234,11 +241,15 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(manager.LOG, 'exception')
         self.share_manager.driver.initialized = False
 
-        self.share_manager.init_host()
+        with mock.patch("time.sleep",
+                        side_effect=CustomTimeSleepException()):
+            self.assertRaises(CustomTimeSleepException,
+                              self.share_manager.init_host)
 
         manager.LOG.exception.assert_called_once_with(
-            mock.ANY, {'name': self.share_manager.driver.__class__.__name__,
-                       'host': self.share_manager.host})
+            mock.ANY, "%(name)s@%(host)s" %
+            {'name': self.share_manager.driver.__class__.__name__,
+             'host': self.share_manager.host})
         self.assertFalse(self.share_manager.driver.initialized)
 
     def _setup_init_mocks(self, setup_access_rules=True):
diff --git a/releasenotes/notes/bug-1690159-retry-backend-init-58486ea420feaf51.yaml b/releasenotes/notes/bug-1690159-retry-backend-init-58486ea420feaf51.yaml
new file mode 100644
index 0000000000..723fd75663
--- /dev/null
+++ b/releasenotes/notes/bug-1690159-retry-backend-init-58486ea420feaf51.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - Retry to initialize the manila-share driver for every backend in case
+    there was an error during initialization. That way even a temporary broken
+    backend can be initialized later without restarting manila-share.