diff --git a/manila/share/drivers/helpers.py b/manila/share/drivers/helpers.py index b69d0267a2..085e97d520 100644 --- a/manila/share/drivers/helpers.py +++ b/manila/share/drivers/helpers.py @@ -278,8 +278,18 @@ class NFSHelper(NASHelperBase): continue access_to = self._get_parsed_address_or_cidr( access['access_to']) - self._ssh_exec(server, ['sudo', 'exportfs', '-u', - ':'.join((access_to, local_path))]) + try: + self._ssh_exec(server, ['sudo', 'exportfs', '-u', + ':'.join((access_to, local_path))]) + except exception.ProcessExecutionError as e: + if "could not find" in e.stderr.lower(): + LOG.debug( + "Client/s with IP address/es %(host)s did not " + "have access to %(share)s. Nothing to deny.", + {'host': access_to, 'share': share_name}) + else: + raise + if delete_rules: self._sync_nfs_temp_and_perm_files(server) for access in add_rules: diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index e991e50fe8..91d532b61c 100644 --- a/manila/share/drivers/lvm.py +++ b/manila/share/drivers/lvm.py @@ -108,7 +108,9 @@ class LVMMixin(driver.ExecuteMixin): (self.configuration.lvm_share_volume_group, share_name), run_as_root=True) except exception.ProcessExecutionError as exc: - if "not found" not in exc.stderr: + err_pattern = re.compile(".*failed to find.*|.*not found.*", + re.IGNORECASE) + if not err_pattern.match(exc.stderr): LOG.exception("Error deleting volume") raise LOG.warning("Volume not found: %s", exc.stderr) @@ -256,11 +258,11 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): return location def delete_share(self, context, share, share_server=None): - self._unmount_device(share) + self._unmount_device(share, raise_if_missing=False) self._delete_share(context, share) self._deallocate_container(share['name']) - def _unmount_device(self, share_or_snapshot): + def _unmount_device(self, share_or_snapshot, raise_if_missing=True): """Unmount the filesystem of a share or snapshot LV.""" mount_path = self._get_mount_path(share_or_snapshot) if os.path.exists(mount_path): @@ -268,9 +270,13 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): try: self._execute('umount', '-f', mount_path, run_as_root=True) except exception.ProcessExecutionError as exc: - if 'device is busy' in six.text_type(exc): + if 'device is busy' in exc.stderr.lower(): raise exception.ShareBusyException( reason=share_or_snapshot['name']) + elif 'not mounted' in exc.stderr.lower(): + if raise_if_missing: + LOG.error('Unable to find device: %s', exc) + raise else: LOG.error('Unable to umount: %s', exc) raise @@ -429,7 +435,7 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): return {'export_locations': exports} def delete_snapshot(self, context, snapshot, share_server=None): - self._unmount_device(snapshot) + self._unmount_device(snapshot, raise_if_missing=False) super(LVMShareDriver, self).delete_snapshot(context, snapshot, share_server) diff --git a/manila/tests/share/drivers/test_helpers.py b/manila/tests/share/drivers/test_helpers.py index 873c3c7a5c..d38aaf2b01 100644 --- a/manila/tests/share/drivers/test_helpers.py +++ b/manila/tests/share/drivers/test_helpers.py @@ -215,12 +215,41 @@ class NFSHelperTestCase(test.TestCase): [], []) - def test_update_access_delete_invalid_rule(self): - delete_rules = [test_generic.get_fake_access_rule( - 'lala', 'fake_level', access_type='user'), ] + @ddt.data({'access_to': 'lala', 'access_type': 'user'}, + {'access_to': '203.0.113.29'}, + {'access_to': '2001:0DB8:7d18:c63e:5f0a:871f:83b8:d244', + 'access_level': 'ro'}) + @ddt.unpack + def test_update_access_delete_invalid_rule( + self, access_to, access_level='rw', access_type='ip'): + mount_path = '%s:/shares/%s' % (access_to, self.share_name) + if access_type == 'ip': + self._helper._get_parsed_address_or_cidr = mock.Mock( + return_value=access_to) + not_found_msg = ( + "exportfs: Could not find '%s' to unexport.\n" % mount_path + ) + exc = exception.ProcessExecutionError + self.mock_object( + self._helper, + '_ssh_exec', + mock.Mock(side_effect=[(0, 0), exc(stderr=not_found_msg)])) + + delete_rules = [ + test_generic.get_fake_access_rule(access_to, + access_level, + access_type), + ] self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files') + self._helper.update_access(self.server, self.share_name, [], [], delete_rules) + + if access_type == 'ip': + self._helper._ssh_exec.assert_has_calls([ + mock.call(self.server, ['sudo', 'exportfs']), + mock.call(self.server, + ['sudo', 'exportfs', '-u', mount_path])]) self._helper._sync_nfs_temp_and_perm_files.assert_called_with( self.server) diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 175e9415fc..4c0aa96068 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/manila/tests/share/drivers/test_lvm.py @@ -248,9 +248,12 @@ class LVMShareDriverTestCase(test.TestCase): self._driver._deallocate_container, self.share['name']) - def test_deallocate_container_not_found_error(self): + @ddt.data( + 'Logical volume "fake/fake-volume" not found\n', + 'Failed to find logical volume "fake/fake-volume"\n') + def test_deallocate_container_not_found_error(self, error_msg): def _fake_exec(*args, **kwargs): - raise exception.ProcessExecutionError(stderr="not found") + raise exception.ProcessExecutionError(stderr=error_msg) self.mock_object(self._driver, '_try_execute', _fake_exec) self._driver._deallocate_container(self.share['name']) @@ -268,6 +271,22 @@ class LVMShareDriverTestCase(test.TestCase): self._driver.get_share_stats(refresh=True)) self._driver._update_share_stats.assert_called_once_with() + def test__unmount_device_not_mounted(self): + def exec_runner(*ignore_args, **ignore_kwargs): + umount_msg = ( + "umount: /opt/stack/data/manila/mnt/share-fake-share: not " + "mounted.\n" + ) + raise exception.ProcessExecutionError(stderr=umount_msg) + self._os.path.exists.return_value = True + mount_path = self._get_mount_path(self.share) + expected_exec = "umount -f %s" % (mount_path) + fake_utils.fake_execute_set_repliers([(expected_exec, exec_runner)]) + + self._driver._unmount_device(self.share, raise_if_missing=False) + + self._os.path.exists.assert_called_with(mount_path) + def test__unmount_device_is_busy_error(self): def exec_runner(*ignore_args, **ignore_kwargs): raise exception.ProcessExecutionError(stderr='device is busy') diff --git a/releasenotes/notes/bug-1888915-harden-lvm-deletions-2a735ab0ee4a4903.yaml b/releasenotes/notes/bug-1888915-harden-lvm-deletions-2a735ab0ee4a4903.yaml new file mode 100644 index 0000000000..537c320c3a --- /dev/null +++ b/releasenotes/notes/bug-1888915-harden-lvm-deletions-2a735ab0ee4a4903.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The LVM driver no longer fails to delete shares, snapshots and access + rules that are missing from storage. See `Launchpad bug #1888915 + `_ for more details.