Fix shares getting stuck in ensuring status

Shares are getting stuck in ensuring status when the share backend
does not support the bulk ensure shares operation.

At the beginning of the operation we update the shares status but
we never revert the shares' status in case the updated_instances
list is not updated when the backend ensure the shares
indivudually.

This change fixes that behavior by rolling back the status of the
shares individually and also updating the service status when the
operation is done.

Closes-Bug: #2127023
Change-Id: I223b035220788111502f4187dcb75f1d456c72f1
Signed-off-by: Carlos da Silva <ces.eduardo98@gmail.com>
(cherry picked from commit 795ff69c55)
(cherry picked from commit d9638f2790)

manila/share/manager.py

@@ -508,6 +508,15 @@ class ShareManager(manager.SchedulerDependentManager):
                                                     ctxt, share_instance)
                         else:
                             self._ensure_share(ctxt, share_instance)
+                    # At this point, we assume that the ensuring operation is
+                    # complete or everything is okay, even though it might be
+                    # running on different threads.
+                    LOG.debug(
+                        "Shares' export locations were ensured individually, "
+                        "so triggering the ensure shares operation is "
+                        "complete.")
+                    self.db.service_update(
+                        ctxt, service['id'], {'ensuring': False})
 
         if new_backend_info:
             self.db.backend_info_update(
@@ -617,6 +626,14 @@ class ShareManager(manager.SchedulerDependentManager):
             self.db.export_locations_update(
                 ctxt, share_instance['id'], export_locations)
 
+        # NOTE(carloss): we can't determine if the share is actually alright,
+        # but we expect that after the export location is updated in the
+        # database, everything is okay.
+        self.db.share_instance_update(
+            ctxt, share_instance['id'],
+            {'status': constants.STATUS_AVAILABLE}
+        )
+
     def _check_share_server_backend_limits(
             self, context, available_share_servers, share_instance=None):
         max_shares_limit = self.driver.max_shares_per_share_server

manila/tests/share/test_manager.py

@@ -404,6 +404,74 @@ class ShareManagerTestCase(test.TestCase):
             )
         ])
 
+    def test_ensure_driver_resources_ensure_shares_not_implemented(self):
+        old_hash = {'info_hash': '1e5ff444cfdc4a154126ddebc0223ffeae2d10c9'}
+        self.mock_object(self.share_manager.db,
+                         'backend_info_get',
+                         mock.Mock(return_value=old_hash))
+        self.mock_object(self.share_manager.driver,
+                         'get_backend_info',
+                         mock.Mock(return_value={'val': 'newval'}))
+        instances, rules = self._setup_init_mocks()
+        fake_service = {'id': 'fake_service_id', 'binary': 'manila-share'}
+        update_instances = [instances[0], instances[2], instances[4]]
+        update_instances_dict_list = [
+            self._get_share_instance_dict(instance)
+            for instance in update_instances
+        ]
+        self.mock_object(self.share_manager.db,
+                         'service_get_by_args',
+                         mock.Mock(return_value=fake_service))
+        self.mock_object(self.share_manager.db, 'service_update')
+        self.mock_object(self.share_manager.db, 'backend_info_update')
+        mock_share_get_all_by_host = self.mock_object(
+            self.share_manager.db, 'share_instance_get_all_by_host',
+            mock.Mock(return_value=instances))
+        self.mock_object(self.share_manager.db, 'share_instance_get',
+                         mock.Mock(side_effect=update_instances))
+        self.mock_object(self.share_manager.db, 'share_metadata_update')
+        mock_ensure_shares = self.mock_object(
+            self.share_manager.driver, 'ensure_shares',
+            mock.Mock(side_effect=NotImplementedError()))
+        mock_ensure_share = self.mock_object(
+            self.share_manager, '_ensure_share')
+
+        self.share_manager.ensure_driver_resources(self.context)
+
+        mock_share_get_all_by_host.assert_called_once_with(
+            utils.IsAMatcher(context.RequestContext), self.share_manager.host)
+        mock_ensure_shares.assert_called_once()
+        mock_ensure_share.assert_has_calls(
+            [mock.call(utils.IsAMatcher(context.RequestContext), instance)
+             for instance in update_instances_dict_list])
+
+    def test__ensure_share(self):
+        fake_export_locations = [{'path': 'fake/path'}]
+        share_instance = {
+            'id': 'fake_id',
+            'share_server': 'fake_share_server'
+        }
+
+        mock_ensure_share = self.mock_object(
+            self.share_manager.driver, 'ensure_share',
+            mock.Mock(return_value=fake_export_locations))
+        mock_export_location_update = self.mock_object(
+            self.share_manager.db, 'export_locations_update')
+        mock_instance_update = self.mock_object(
+            self.share_manager.db, 'share_instance_update')
+
+        self.share_manager._ensure_share(self.context, share_instance)
+
+        mock_ensure_share.assert_called_once_with(
+            self.context, share_instance,
+            share_server=share_instance['share_server']
+        )
+        mock_export_location_update.assert_called_once_with(
+            self.context, share_instance['id'], fake_export_locations)
+        mock_instance_update.assert_called_once_with(
+            self.context, share_instance['id'],
+            {'status': constants.STATUS_AVAILABLE})
+
     def test_init_host_with_no_shares(self):
         self.mock_object(self.share_manager.db,
                          'share_instance_get_all_by_host',

releasenotes/notes/bug-2127023-fix-shares-stuck-in-ensuring-44fbb778fa5b3d2d.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixed an issue that caused shares to be stuck in `ensuring` status when
+    the back end driver does not support the bulk ensure shares operation.