Harden LVM driver deletion paths

During maintenance, administrators may decide
to unmount shares, snapshots or remove them
entirely prior to cleaning up LVM share resources
in manila. The driver should not fail on deletion
of missing resources.

Change-Id: Ieaf37ec10db9a8bdce6bb195b76335fea9b2b52f
Closes-Bug: #1888915
Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>
(cherry picked from commit e57809fd74)
(cherry picked from commit 8620f7efed)
(cherry picked from commit 4e292ea884)
(cherry picked from commit c441ec22ef)
(cherry picked from commit 40abc1bcf6)
This commit is contained in:
Goutham Pacha Ravi 2020-07-27 18:02:51 -07:00 committed by Tom Barron
parent eee6c4218f
commit cdf1b7b17d
5 changed files with 82 additions and 12 deletions

View File

@ -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:

View File

@ -118,7 +118,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)
@ -269,11 +271,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):
@ -281,9 +283,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
@ -442,7 +448,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)

View File

@ -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)

View File

@ -268,9 +268,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'])
@ -288,6 +291,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')

View File

@ -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
<https://launchpad.net/bugs/1888915>`_ for more details.