HPE3PAR: Handle exceptions on deleted shares

Fix to ensure that an attempt to delete a manila share will not
fail because the 3PAR filestore does not exist. When the filestore
does not exist, that means the share is already gone (or never was
created). Avoid the delete and all filestore use in this case to
prevent failures during delete.

Also catch and log exceptions during the post-delete cleanup.

Change-Id: I0e477d3dae5706547354bd029b40e3308349fabb
Closes-Bug: #1597940

manila/share/drivers/hpe/hpe_3par_mediator.py

@@ -662,24 +662,35 @@ class HPE3ParMediator(object):
         if fstore:
             self._delete_share(share_name_ro, protocol, fpg, vfs, fstore)
 
-        if not self.hpe3par_fstore_per_share:
+            if fstore == share_name:
-            # Attempt to remove file tree on delete when using nested shares.
+                try:
-            # If the file tree cannot be removed for whatever reason, we will
+                    self._client.removefstore(vfs, fstore, fpg=fpg)
-            # not treat this as an error_deleting issue. We will allow the
+                except Exception as e:
-            # delete to continue as requested.
+                    msg = (_('Failed to remove fstore %(fstore)s: %(e)s') %
-            self._delete_file_tree(share_name, protocol, fpg, vfs, fstore)
+                           {'fstore': fstore, 'e': six.text_type(e)})
-            if fstore:
+                    LOG.exception(msg)
-                # reduce the fsquota by share size when a share is deleted.
+                    raise exception.ShareBackendException(msg=msg)
-                self._update_capacity_quotas(fstore, 0, share_size, fpg, vfs)
 
-        if fstore == share_name:
+            else:
-            try:
+                try:
-                self._client.removefstore(vfs, fstore, fpg=fpg)
+                    # Attempt to remove file tree on delete when using nested
-            except Exception as e:
+                    # shares. If the file tree cannot be removed for whatever
-                msg = (_('Failed to remove fstore %(fstore)s: %(e)s') %
+                    # reason, we will not treat this as an error_deleting
-                       {'fstore': fstore, 'e': six.text_type(e)})
+                    # issue. We will allow the delete to continue as requested.
-                LOG.exception(msg)
+                    self._delete_file_tree(
-                raise exception.ShareBackendException(msg=msg)
+                        share_name, protocol, fpg, vfs, fstore)
+                    # reduce the fsquota by share size when a tree is deleted.
+                    self._update_capacity_quotas(
+                        fstore, 0, share_size, fpg, vfs)
+                except Exception as e:
+                    msg = _LW('Exception during cleanup of deleted '
+                              'share %(share)s in filestore %(fstore)s: %(e)s')
+                    data = {
+                        'fstore': fstore,
+                        'share': share_name,
+                        'e': six.text_type(e),
+                    }
+                    LOG.warning(msg, data)
 
     def _delete_file_tree(self, share_name, protocol, fpg, vfs, fstore):
         # If the share protocol is CIFS, we need to make sure the admin

manila/tests/share/drivers/hpe/test_hpe_3par_constants.py

@@ -40,7 +40,9 @@ EXPECTED_VLAN_TYPE = 'vlan'
 EXPECTED_VLAN_TAG = '101'
 EXPECTED_SERVER_ID = '1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e'
 EXPECTED_PROJECT_ID = 'osf-nfs-project-id'
+SHARE_ID = 'share-id'
 EXPECTED_SHARE_ID = 'osf-share-id'
+EXPECTED_SHARE_ID_RO = 'osf-ro-share-id'
 EXPECTED_SHARE_NAME = 'share-name'
 EXPECTED_HOST = 'hostname@backend#pool'
 EXPECTED_SHARE_PATH = '/anyfpg/anyvfs/anyfstore'

manila/tests/share/drivers/hpe/test_hpe_3par_mediator.py

@@ -599,21 +599,8 @@ class HPE3ParMediatorTestCase(test.TestCase):
         self.mock_object(self.mediator,
                          '_find_fstore',
                          mock.Mock(return_value=fstore))
-        self.mock_object(self.mediator,
+        self.mock_object(self.mediator, '_delete_file_tree')
-                         '_create_mount_directory',
+        self.mock_object(self.mediator, '_update_capacity_quotas')
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_mount_super_share',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_delete_share_directory',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_unmount_super_share',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_update_capacity_quotas',
-                         mock.Mock(return_value={}))
 
         self.mediator.delete_share(constants.EXPECTED_PROJECT_ID,
                                    share_id,
@@ -633,35 +620,14 @@ class HPE3ParMediatorTestCase(test.TestCase):
                                    osf_ro_share_id,
                                    fpg=constants.EXPECTED_FPG,
                                    fstore=fstore),
-            mock.call.createfshare(constants.NFS_LOWER,
-                                   constants.EXPECTED_VFS,
-                                   constants.EXPECTED_SUPER_SHARE,
-                                   clientip=None,
-                                   comment=(
-                                       constants.EXPECTED_SUPER_SHARE_COMMENT),
-                                   fpg=constants.EXPECTED_FPG,
-                                   fstore=fstore,
-                                   options='rw,no_root_squash,insecure',
-                                   sharedir=''),
             mock.call.removefstore(constants.EXPECTED_VFS,
                                    fstore,
                                    fpg=constants.EXPECTED_FPG),
         ]
         self.mock_client.assert_has_calls(expected_calls)
 
-        expected_mount_path = constants.EXPECTED_MOUNT_PATH + osf_share_id
+        self.assertFalse(self.mediator._delete_file_tree.called)
-        self.mediator._create_mount_directory.assert_called_with(
+        self.assertFalse(self.mediator._update_capacity_quotas.called)
-            expected_mount_path)
-        self.mediator._mount_super_share.assert_called_with(
-            constants.NFS_LOWER, expected_mount_path, constants.EXPECTED_FPG,
-            constants.EXPECTED_VFS, fstore)
-        self.mediator._delete_share_directory.assert_called_with(
-            expected_mount_path)
-        self.mediator._unmount_super_share.assert_called_with(
-            expected_mount_path)
-        self.mediator._update_capacity_quotas.assert_called_with(
-            fstore, 0, constants.EXPECTED_SIZE_1,
-            constants.EXPECTED_FPG, constants.EXPECTED_VFS)
 
     def test_mediator_delete_share_not_found(self):
         self.init_mediator()
@@ -669,18 +635,8 @@ class HPE3ParMediatorTestCase(test.TestCase):
         self.mock_object(self.mediator,
                          '_find_fstore',
                          mock.Mock(return_value=None))
-        self.mock_object(self.mediator,
+        self.mock_object(self.mediator, '_delete_file_tree')
-                         '_create_mount_directory',
+        self.mock_object(self.mediator, '_update_capacity_quotas')
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_mount_super_share',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_delete_share_directory',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_unmount_super_share',
-                         mock.Mock(return_value={}))
 
         self.mediator.delete_share(constants.EXPECTED_PROJECT_ID,
                                    constants.EXPECTED_SHARE_ID,
@@ -691,17 +647,8 @@ class HPE3ParMediatorTestCase(test.TestCase):
 
         self.assertFalse(self.mock_client.removefshare.called)
 
-        expected_mount_path = constants.EXPECTED_MOUNT_PATH + (
+        self.assertFalse(self.mediator._delete_file_tree.called)
-            constants.EXPECTED_SHARE_ID)
+        self.assertFalse(self.mediator._update_capacity_quotas.called)
-        self.mediator._create_mount_directory.assert_called_with(
-            expected_mount_path)
-        self.mediator._mount_super_share.assert_called_with(
-            constants.SMB_LOWER, expected_mount_path, constants.EXPECTED_FPG,
-            constants.EXPECTED_VFS, None)
-        self.mediator._delete_share_directory.assert_called_with(
-            expected_mount_path)
-        self.mediator._unmount_super_share.assert_called_with(
-            expected_mount_path)
 
     def test_mediator_delete_nfs_share_only_readonly(self):
         self.init_mediator()
@@ -779,28 +726,15 @@ class HPE3ParMediatorTestCase(test.TestCase):
         self.mock_object(self.mediator,
                          '_find_fstore',
                          mock.Mock(return_value=constants.EXPECTED_SHARE_ID))
-        self.mock_object(self.mediator,
+        self.mock_object(self.mediator, '_delete_file_tree')
-                         '_create_mount_directory',
+        self.mock_object(self.mediator, '_update_capacity_quotas')
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_mount_super_share',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_delete_share_directory',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_unmount_super_share',
-                         mock.Mock(return_value={}))
-        self.mock_object(self.mediator,
-                         '_update_capacity_quotas',
-                         mock.Mock(return_value={}))
         self.mock_client.removefstore.side_effect = Exception(
             'removefstore fail.')
 
         self.assertRaises(exception.ShareBackendException,
                           self.mediator.delete_share,
                           constants.EXPECTED_PROJECT_ID,
-                          constants.EXPECTED_SHARE_ID,
+                          constants.SHARE_ID,
                           constants.EXPECTED_SIZE_1,
                           constants.CIFS,
                           constants.EXPECTED_FPG,
@@ -812,51 +746,62 @@ class HPE3ParMediatorTestCase(test.TestCase):
                                    constants.EXPECTED_SHARE_ID,
                                    fpg=constants.EXPECTED_FPG,
                                    fstore=constants.EXPECTED_SHARE_ID),
-            mock.call.createfshare(constants.SMB_LOWER,
+            mock.call.removefshare(constants.SMB_LOWER,
                                    constants.EXPECTED_VFS,
-                                   constants.EXPECTED_SUPER_SHARE,
+                                   constants.EXPECTED_SHARE_ID_RO,
-                                   allowip=None,
-                                   comment=(
-                                       constants.EXPECTED_SUPER_SHARE_COMMENT),
                                    fpg=constants.EXPECTED_FPG,
-                                   fstore=constants.EXPECTED_SHARE_ID,
+                                   fstore=constants.EXPECTED_SHARE_ID),
-                                   sharedir=''),
-            mock.call.setfshare(constants.SMB_LOWER,
-                                constants.EXPECTED_VFS,
-                                constants.EXPECTED_SUPER_SHARE,
-                                comment=(
-                                    constants.EXPECTED_SUPER_SHARE_COMMENT),
-                                allowperm=(
-                                    '+' + constants.USERNAME + ':fullcontrol'),
-                                fpg=constants.EXPECTED_FPG,
-                                fstore=constants.EXPECTED_SHARE_ID),
             mock.call.removefstore(constants.EXPECTED_VFS,
                                    constants.EXPECTED_SHARE_ID,
                                    fpg=constants.EXPECTED_FPG),
         ]
         self.mock_client.assert_has_calls(expected_calls)
 
-        expected_mount_path = constants.EXPECTED_MOUNT_PATH + (
+        self.assertFalse(self.mediator._delete_file_tree.called)
-            constants.EXPECTED_SHARE_ID)
+        self.assertFalse(self.mediator._update_capacity_quotas.called)
-        self.mediator._create_mount_directory.assert_called_with(
+
-            expected_mount_path)
+    def test_mediator_delete_file_tree_exception(self):
-        self.mediator._mount_super_share.assert_called_with(
+        self.init_mediator()
-            constants.SMB_LOWER, expected_mount_path, constants.EXPECTED_FPG,
+        mock_log = self.mock_object(hpe3parmediator, 'LOG')
-            constants.EXPECTED_VFS, constants.EXPECTED_SHARE_ID)
+        self.mock_object(self.mediator,
-        self.mediator._delete_share_directory.assert_called_with(
+                         '_find_fstore',
-            expected_mount_path)
+                         mock.Mock(return_value=constants.EXPECTED_FSTORE))
-        self.mediator._unmount_super_share.assert_called_with(
+        self.mock_object(self.mediator,
-            expected_mount_path)
+                         '_delete_file_tree',
-        self.mediator._update_capacity_quotas.assert_called_with(
+                         mock.Mock(side_effect=Exception('test')))
-            constants.EXPECTED_SHARE_ID, 0, constants.EXPECTED_SIZE_1,
+        self.mock_object(self.mediator, '_update_capacity_quotas')
-            constants.EXPECTED_FPG, constants.EXPECTED_VFS)
+
+        self.mediator.delete_share(constants.EXPECTED_PROJECT_ID,
+                                   constants.SHARE_ID,
+                                   constants.EXPECTED_SIZE_1,
+                                   constants.CIFS,
+                                   constants.EXPECTED_FPG,
+                                   constants.EXPECTED_VFS)
+
+        expected_calls = [
+            mock.call.removefshare(constants.SMB_LOWER,
+                                   constants.EXPECTED_VFS,
+                                   constants.EXPECTED_SHARE_ID,
+                                   fpg=constants.EXPECTED_FPG,
+                                   fstore=constants.EXPECTED_FSTORE),
+            mock.call.removefshare(constants.SMB_LOWER,
+                                   constants.EXPECTED_VFS,
+                                   constants.EXPECTED_SHARE_ID_RO,
+                                   fpg=constants.EXPECTED_FPG,
+                                   fstore=constants.EXPECTED_FSTORE),
+        ]
+        self.mock_client.assert_has_calls(expected_calls)
+
+        self.assertTrue(self.mediator._delete_file_tree.called)
+        self.assertFalse(self.mediator._update_capacity_quotas.called)
+        mock_log.warning.assert_called_once_with(mock.ANY, mock.ANY)
 
     def test_mediator_delete_cifs_share(self):
         self.init_mediator()
 
         self.mock_object(self.mediator,
                          '_find_fstore',
-                         mock.Mock(return_value=constants.EXPECTED_SHARE_ID))
+                         mock.Mock(return_value=constants.EXPECTED_FSTORE))
         self.mock_object(self.mediator,
                          '_create_mount_directory',
                          mock.Mock(return_value={}))
@@ -885,7 +830,7 @@ class HPE3ParMediatorTestCase(test.TestCase):
                                    constants.EXPECTED_VFS,
                                    constants.EXPECTED_SHARE_ID,
                                    fpg=constants.EXPECTED_FPG,
-                                   fstore=constants.EXPECTED_SHARE_ID),
+                                   fstore=constants.EXPECTED_FSTORE),
             mock.call.createfshare(constants.SMB_LOWER,
                                    constants.EXPECTED_VFS,
                                    constants.EXPECTED_SUPER_SHARE,
@@ -893,7 +838,7 @@ class HPE3ParMediatorTestCase(test.TestCase):
                                    comment=(
                                        constants.EXPECTED_SUPER_SHARE_COMMENT),
                                    fpg=constants.EXPECTED_FPG,
-                                   fstore=constants.EXPECTED_SHARE_ID,
+                                   fstore=constants.EXPECTED_FSTORE,
                                    sharedir=''),
             mock.call.setfshare(constants.SMB_LOWER,
                                 constants.EXPECTED_VFS,
@@ -903,27 +848,65 @@ class HPE3ParMediatorTestCase(test.TestCase):
                                 allowperm=(
                                     '+' + constants.USERNAME + ':fullcontrol'),
                                 fpg=constants.EXPECTED_FPG,
-                                fstore=constants.EXPECTED_SHARE_ID),
+                                fstore=constants.EXPECTED_FSTORE),
+        ]
+        self.mock_client.assert_has_calls(expected_calls)
+
+        expected_mount_path = constants.EXPECTED_MOUNT_PATH + (
+            constants.EXPECTED_SHARE_ID)
+        expected_share_path = '/'.join((expected_mount_path,
+                                        constants.EXPECTED_SHARE_ID))
+        self.mediator._create_mount_directory.assert_called_once_with(
+            expected_mount_path)
+        self.mediator._mount_super_share.assert_called_once_with(
+            constants.SMB_LOWER,
+            expected_mount_path,
+            constants.EXPECTED_FPG,
+            constants.EXPECTED_VFS,
+            constants.EXPECTED_FSTORE)
+        self.mediator._delete_share_directory.assert_has_calls([
+            mock.call(expected_share_path),
+            mock.call(expected_mount_path),
+        ])
+        self.mediator._unmount_super_share.assert_called_once_with(
+            expected_mount_path)
+        self.mediator._update_capacity_quotas.assert_called_once_with(
+            constants.EXPECTED_FSTORE,
+            0,
+            constants.EXPECTED_SIZE_1,
+            constants.EXPECTED_FPG,
+            constants.EXPECTED_VFS)
+
+    def test_mediator_delete_cifs_share_and_fstore(self):
+        self.init_mediator()
+
+        self.mock_object(self.mediator,
+                         '_find_fstore',
+                         mock.Mock(return_value=constants.EXPECTED_SHARE_ID))
+        self.mock_object(self.mediator, '_delete_file_tree')
+        self.mock_object(self.mediator, '_update_capacity_quotas')
+
+        self.mediator.delete_share(constants.EXPECTED_PROJECT_ID,
+                                   constants.EXPECTED_SHARE_ID,
+                                   constants.EXPECTED_SIZE_1,
+                                   constants.CIFS,
+                                   constants.EXPECTED_FPG,
+                                   constants.EXPECTED_VFS)
+
+        expected_calls = [
+            mock.call.removefshare(constants.SMB_LOWER,
+                                   constants.EXPECTED_VFS,
+                                   constants.EXPECTED_SHARE_ID,
+                                   fpg=constants.EXPECTED_FPG,
+                                   fstore=constants.EXPECTED_SHARE_ID),
             mock.call.removefstore(constants.EXPECTED_VFS,
                                    constants.EXPECTED_SHARE_ID,
                                    fpg=constants.EXPECTED_FPG),
         ]
         self.mock_client.assert_has_calls(expected_calls)
 
-        expected_mount_path = constants.EXPECTED_MOUNT_PATH + (
+        self.assertFalse(self.mediator._delete_file_tree.called)
-            constants.EXPECTED_SHARE_ID)
+        self.assertFalse(self.mediator._update_capacity_quotas.called)
-        self.mediator._create_mount_directory.assert_called_with(
-            expected_mount_path)
-        self.mediator._mount_super_share.assert_called_with(
-            constants.SMB_LOWER, expected_mount_path, constants.EXPECTED_FPG,
-            constants.EXPECTED_VFS, constants.EXPECTED_SHARE_ID)
-        self.mediator._delete_share_directory.assert_called_with(
-            expected_mount_path)
-        self.mediator._unmount_super_share.assert_called_with(
-            expected_mount_path)
-        self.mediator._update_capacity_quotas.assert_called_with(
-            constants.EXPECTED_SHARE_ID, 0, constants.EXPECTED_SIZE_1,
-            constants.EXPECTED_FPG, constants.EXPECTED_VFS)
 
     def test_mediator_delete_share_with_fstore_per_share_false(self):
         self.init_mediator()

releasenotes/notes/bug-1597940-fix-hpe3par-delete-share-0daf75193f318c41.yaml

@@ -0,0 +1,3 @@
+---
+fixes:
+  - HPE3PAR driver fix to allow delete of a share that does not exist on the backend.