Merge "Add exception handling to share server deletion"

manila/share/manager.py

@@ -3813,7 +3813,14 @@ class ShareManager(manager.SchedulerDependentManager):
                                                                 self.host,
                                                                 updated_before)
         for server in servers:
-            self.delete_share_server(ctxt, server)
+            try:
+                self.delete_share_server(ctxt, server)
+            except exception.ShareServerNotFound:
+                continue
+            except Exception:
+                LOG.exception(
+                    "Unable to delete share server %s, will retry in the next "
+                    "run.", server['id'])
 
     @periodic_task.periodic_task(
         spacing=CONF.check_for_expired_shares_in_recycle_bin_interval)
@@ -4689,6 +4696,12 @@ class ShareManager(manager.SchedulerDependentManager):
             # this method starts executing when amount of dependent shares
             # has been changed.
             server_id = share_server['id']
+            try:
+                self.db.share_server_get(
+                    context, server_id)
+            except exception.ShareServerNotFound:
+                raise
+
             shares = self.db.share_instance_get_all_by_share_server(
                 context, server_id)
 

manila/tests/share/test_manager.py

@@ -4672,6 +4672,85 @@ class ShareManagerTestCase(test.TestCase):
         with test_utils.create_temp_config_with_opts(data):
             manager.ShareManager()
 
+    def test_delete_share_server_server_not_found(self):
+        share_server = db_utils.create_share_server()
+        self.share_manager.driver.initialized = True
+
+        mock_server_get = self.mock_object(
+            self.share_manager.db, 'share_server_get',
+            mock.Mock(side_effect=exception.ShareServerNotFound(
+                share_server_id=share_server['id'])))
+
+        self.assertRaises(
+            exception.ShareServerNotFound,
+            self.share_manager.delete_share_server,
+            self.context,
+            share_server
+        )
+
+        mock_server_get.assert_called_once_with(
+            self.context, share_server['id'])
+
+    def test_delete_share_server_server_in_use(self):
+        share_server = db_utils.create_share_server()
+        share_server_shares = [db_utils.create_share()]
+        self.share_manager.driver.initialized = True
+
+        mock_server_get = self.mock_object(
+            self.share_manager.db, 'share_server_get',
+            mock.Mock(return_value=share_server))
+        mock_instances_get = self.mock_object(
+            self.share_manager.db, 'share_instance_get_all_by_share_server',
+            mock.Mock(return_value=share_server_shares))
+
+        self.assertRaises(
+            exception.ShareServerInUse,
+            self.share_manager.delete_share_server,
+            self.context,
+            share_server
+        )
+
+        mock_server_get.assert_called_once_with(
+            self.context, share_server['id'])
+        mock_instances_get.assert_called_once_with(
+            self.context, share_server['id'])
+
+    def test_delete_share_server_teardown_failure(self):
+        share_server = db_utils.create_share_server()
+        self.share_manager.driver.initialized = True
+
+        mock_server_get = self.mock_object(
+            self.share_manager.db, 'share_server_get',
+            mock.Mock(return_value=share_server))
+        mock_instances_get = self.mock_object(
+            self.share_manager.db, 'share_instance_get_all_by_share_server',
+            mock.Mock(return_value=[]))
+        mock_teardown_server = self.mock_object(
+            self.share_manager.driver, 'teardown_server',
+            mock.Mock(side_effect=exception.ShareBackendException(msg='fake')))
+        mock_server_update = self.mock_object(
+            self.share_manager.db, 'share_server_update')
+
+        self.assertRaises(
+            exception.ShareBackendException,
+            self.share_manager.delete_share_server,
+            self.context,
+            share_server
+        )
+
+        mock_server_get.assert_called_once_with(
+            self.context, share_server['id'])
+        mock_instances_get.assert_called_once_with(
+            self.context, share_server['id'])
+        mock_teardown_server.assert_called_once_with(
+            server_details=share_server['backend_details'],
+            security_services=[]
+        )
+        mock_server_update.assert_called_with(
+            self.context, share_server['id'],
+            {'status': constants.STATUS_ERROR}
+        )
+
     @mock.patch.object(db, 'share_server_get_all_unused_deletable',
                        mock.Mock())
     @mock.patch.object(manager.ShareManager, 'delete_share_server',