From 4e3f53c590a6069247312e7bf17b40f5f0dd30d3 Mon Sep 17 00:00:00 2001
From: haixin <haix09@chinatelecom.cn>
Date: Thu, 24 Feb 2022 23:01:58 +0800
Subject: [PATCH] share recycle bin, Fix follow-up suggestions

Change-Id: I34b316f85af2cee3aa22d9111764e2c8af1bbde8
---
 api-ref/source/shares.inc          |  8 ++++++++
 manila/api/v2/shares.py            |  4 ++--
 manila/share/manager.py            | 12 ++++++++++--
 manila/tests/share/test_manager.py | 22 +++++++++++++---------
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/api-ref/source/shares.inc b/api-ref/source/shares.inc
index 9439debb28..b51ff75cd0 100644
--- a/api-ref/source/shares.inc
+++ b/api-ref/source/shares.inc
@@ -324,6 +324,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
 ----------------
@@ -488,6 +490,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
 ----------------
@@ -593,6 +597,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
 ----------------
@@ -691,6 +697,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
 ----------------
diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py
index b4651b7bd0..d41273dfe7 100644
--- a/manila/api/v2/shares.py
+++ b/manila/api/v2/shares.py
@@ -305,8 +305,8 @@ class ShareController(shares.ShareMixin,
         # 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, "
-                    "cannot be restore.")
+            msg = _("Share %s is being deleted or has suffered an error "
+                    "during deletion, cannot be restored.")
             raise exc.HTTPForbidden(explanation=msg % id)
 
         self.share_api.restore(context, share)
diff --git a/manila/share/manager.py b/manila/share/manager.py
index 3e82331ed4..af6e1a6959 100644
--- a/manila/share/manager.py
+++ b/manila/share/manager.py
@@ -3496,8 +3496,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'])
-            self.share_api.delete(ctxt, share, force=True)
+            if share['status'] == constants.STATUS_ERROR_DELETING:
+                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
diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py
index 98ae7d45e7..5605f40e2d 100644
--- a/manila/tests/share/test_manager.py
+++ b/manila/tests/share/test_manager.py
@@ -3939,17 +3939,21 @@ class ShareManagerTestCase(test.TestCase):
             'server1')
         timeutils.utcnow.assert_called_once_with()
 
-    @mock.patch.object(db, 'get_all_expired_shares',
-                       mock.Mock(return_value=[{"id": "share1"}, ]))
-    @mock.patch.object(api.API, 'delete',
-                       mock.Mock())
-    def test_delete_expired_share(self):
+    @ddt.data("available", "error_deleting")
+    def test_delete_expired_share(self, share_status):
+        self.mock_object(db, 'get_all_expired_shares',
+                         mock.Mock(return_value=[{"id": "share1",
+                                                  "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(
-            self.context)
-        share1 = {"id": "share1"}
+        db.get_all_expired_shares.assert_called_once_with(self.context)
+        share1 = {"id": "share1", "status": share_status}
+        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):