From b457fdecee471ede90fd40a5559573e3c3f5841b Mon Sep 17 00:00:00 2001
From: nidhimittalhada <nidhimittal19@gmail.com>
Date: Wed, 23 Dec 2015 14:35:25 +0530
Subject: [PATCH] Delete Share Instance of unmanaged share

Currently, for an unmanaged share, the associated share instance
stays with a status as 'unmanaged', which is not correct.
Associated share instance should be deleted.
Hence made the correction

Change-Id: Ib88205db91d69bd8fc4d98e82b479eadac85b604
Closes-Bug: #1510208
---
 manila/share/manager.py                       |  5 ++---
 manila/tests/share/test_manager.py            | 19 ++++++++++---------
 .../tests/api/admin/test_share_manage.py      |  7 +++++++
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/manila/share/manager.py b/manila/share/manager.py
index e6583f7539..8d2de97e70 100644
--- a/manila/share/manager.py
+++ b/manila/share/manager.py
@@ -939,9 +939,8 @@ class ShareManager(manager.SchedulerDependentManager):
                     _LE("Can not remove access rules of share: %s."), e)
                 return
 
-        self.db.share_update(context, share_id,
-                             {'status': constants.STATUS_UNMANAGED,
-                              'deleted': True})
+        self.db.share_instance_delete(context, share_instance['id'])
+        LOG.info(_LI("Share %s: unmanaged successfully."), share_id)
 
     @add_hooks
     @utils.require_driver_initialized
diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py
index b7b1044fd1..ec0ee291a7 100644
--- a/manila/tests/share/test_manager.py
+++ b/manila/tests/share/test_manager.py
@@ -1116,6 +1116,7 @@ class ShareManagerTestCase(test.TestCase):
                              mock_unmanage)
 
         self.mock_object(self.share_manager.db, 'share_update')
+        self.mock_object(self.share_manager.db, 'share_instance_delete')
 
     @ddt.data(True, False)
     def test_unmanage_share_invalid_driver(self, driver_handles_share_servers):
@@ -1150,14 +1151,14 @@ class ShareManagerTestCase(test.TestCase):
                                    mock_unmanage=mock.Mock())
         share = db_utils.create_share()
         share_id = share['id']
+        share_instance_id = share.instance['id']
 
         self.share_manager.unmanage_share(self.context, share_id)
 
         self.share_manager.driver.unmanage.\
             assert_called_once_with(mock.ANY)
-        self.share_manager.db.share_update.assert_called_once_with(
-            mock.ANY, share_id,
-            {'status': constants.STATUS_UNMANAGED, 'deleted': True})
+        self.share_manager.db.share_instance_delete.assert_called_once_with(
+            mock.ANY, share_instance_id)
 
     def test_unmanage_share_valid_share_with_quota_error(self):
         manager.CONF.set_default('driver_handles_share_servers', False)
@@ -1166,14 +1167,14 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(quota.QUOTAS, 'reserve',
                          mock.Mock(side_effect=Exception()))
         share = db_utils.create_share()
+        share_instance_id = share.instance['id']
 
         self.share_manager.unmanage_share(self.context, share['id'])
 
         self.share_manager.driver.unmanage.\
             assert_called_once_with(mock.ANY)
-        self.share_manager.db.share_update.assert_called_once_with(
-            mock.ANY, share['id'],
-            {'status': constants.STATUS_UNMANAGED, 'deleted': True})
+        self.share_manager.db.share_instance_delete.assert_called_once_with(
+            mock.ANY, share_instance_id)
 
     def test_unmanage_share_remove_access_rules_error(self):
         manager.CONF.set_default('driver_handles_share_servers', False)
@@ -1199,6 +1200,7 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(quota.QUOTAS, 'reserve', mock.Mock(return_value=[]))
         share = db_utils.create_share()
         share_id = share['id']
+        share_instance_id = share.instance['id']
 
         self.share_manager.unmanage_share(self.context, share_id)
 
@@ -1207,9 +1209,8 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager._remove_share_access_rules.assert_called_once_with(
             mock.ANY, mock.ANY, mock.ANY, mock.ANY
         )
-        self.share_manager.db.share_update.assert_called_once_with(
-            mock.ANY, share_id,
-            {'status': constants.STATUS_UNMANAGED, 'deleted': True})
+        self.share_manager.db.share_instance_delete.assert_called_once_with(
+            mock.ANY, share_instance_id)
 
     def test_remove_share_access_rules(self):
         self.mock_object(self.share_manager.db,
diff --git a/manila_tempest_tests/tests/api/admin/test_share_manage.py b/manila_tempest_tests/tests/api/admin/test_share_manage.py
index c7efa0183d..bbd98bcab2 100644
--- a/manila_tempest_tests/tests/api/admin/test_share_manage.py
+++ b/manila_tempest_tests/tests/api/admin/test_share_manage.py
@@ -147,6 +147,13 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
 
     @test.attr(type=["gate", "smoke"])
     def test_manage(self):
+        # After 'unmanage' operation, share instance should be deleted.
+        # Assert not related to 'manage' test, but placed here for
+        # resource optimization.
+        share_instance_list = self.shares_v2_client.list_share_instances()
+        share_ids = [si['share_id'] for si in share_instance_list]
+        self.assertNotIn(self.shares[0]['id'], share_ids)
+
         self._test_manage(share=self.shares[0])
 
     @test.attr(type=["gate", "smoke"])