Merge "share recycle bin, Fix follow-up suggestions"

api-ref/source/shares.inc

@@ -323,6 +323,8 @@ Response parameters
    - volume_type: volume_type_shares_response
    - export_location: export_location
    - export_locations: export_locations
+   - is_soft_deleted: is_soft_deleted_response
+   - scheduled_to_be_deleted_at: scheduled_to_be_deleted_at_response
 
 Response example
 ----------------
@@ -487,6 +489,8 @@ Response parameters
    - volume_type: volume_type_shares_response
    - export_location: export_location
    - export_locations: export_locations
+   - is_soft_deleted: is_soft_deleted_response
+   - scheduled_to_be_deleted_at: scheduled_to_be_deleted_at_response
 
 Response example
 ----------------
@@ -592,6 +596,8 @@ Response parameters
    - volume_type: volume_type_shares_response
    - export_location: export_location
    - export_locations: export_locations
+   - is_soft_deleted: is_soft_deleted_response
+   - scheduled_to_be_deleted_at: scheduled_to_be_deleted_at_response
 
 Response example
 ----------------
@@ -690,6 +696,8 @@ Response parameters
    - volume_type: volume_type_shares_response
    - export_location: export_location
    - export_locations: export_locations
+   - is_soft_deleted: is_soft_deleted_response
+   - scheduled_to_be_deleted_at: scheduled_to_be_deleted_at_response
 
 Response example
 ----------------

manila/api/v2/shares.py

@@ -308,8 +308,8 @@ class ShareController(wsgi.Controller,
         # it too late to restore the share.
         if share['status'] in [constants.STATUS_DELETING,
                                constants.STATUS_ERROR_DELETING]:
-            msg = _("Share %s is being deleted or error deleted, "
+            msg = _("Share %s is being deleted or has suffered an error "
-                    "cannot be restore.")
+                    "during deletion, cannot be restored.")
             raise exc.HTTPForbidden(explanation=msg % id)
 
         self.share_api.restore(context, share)

manila/share/manager.py

@@ -3514,8 +3514,16 @@ class ShareManager(manager.SchedulerDependentManager):
         expired_shares = self.db.get_all_expired_shares(ctxt)
 
         for share in expired_shares:
-            LOG.debug("share %s has expired, will be deleted", share['id'])
+            if share['status'] == constants.STATUS_ERROR_DELETING:
-            self.share_api.delete(ctxt, share, force=True)
+                LOG.info("Share %s was soft-deleted but a prior deletion "
+                         "attempt failed. Resetting status and re-attempting "
+                         "deletion", share['id'])
+                # reset share status to error in order to try deleting again
+                update_data = {'status': constants.STATUS_ERROR}
+                self.db.share_update(ctxt, share['id'], update_data)
+            else:
+                LOG.info("share %s has expired, will be deleted", share['id'])
+            self.share_api.delete(ctxt, share)
 
     @add_hooks
     @utils.require_driver_initialized

manila/tests/share/test_manager.py

@@ -3976,17 +3976,21 @@ class ShareManagerTestCase(test.TestCase):
             'server1')
         timeutils.utcnow.assert_called_once_with()
 
-    @mock.patch.object(db, 'get_all_expired_shares',
+    @ddt.data("available", "error_deleting")
-                       mock.Mock(return_value=[{"id": "share1"}, ]))
+    def test_delete_expired_share(self, share_status):
-    @mock.patch.object(api.API, 'delete',
+        self.mock_object(db, 'get_all_expired_shares',
-                       mock.Mock())
+                         mock.Mock(return_value=[{"id": "share1",
-    def test_delete_expired_share(self):
+                                                  "status": share_status}, ]))
+        self.mock_object(db, 'share_update')
+        self.mock_object(api.API, 'delete')
         self.share_manager.delete_expired_share(self.context)
-        db.get_all_expired_shares.assert_called_once_with(
+        db.get_all_expired_shares.assert_called_once_with(self.context)
-            self.context)
+        share1 = {"id": "share1", "status": share_status}
-        share1 = {"id": "share1"}
+        if share1["status"] == "error_deleting":
+            db.share_update.assert_called_once_with(
+                self.context, share1["id"], {'status': 'error'})
         api.API.delete.assert_called_once_with(
-            self.context, share1, force=True)
+            self.context, share1)
 
     @mock.patch('manila.tests.fake_notifier.FakeNotifier._notify')
     def test_extend_share_invalid(self, mock_notify):