openstack/manila
Code Issues Proposed changes

Merge "HNAS: Fix concurrency creating/deleting snapshots"

Browse Source
This commit is contained in:
Jenkins 2017-02-11 01:13:34 +00:00 committed by Gerrit Code Review
commit 14f05340e5
6 changed files with 198 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
manila/exception.py
View File

@ -816,6 +816,14 @@ class HNASSSCIsBusy(ManilaException):
message = _("HNAS SSC is busy and cannot execute the command: %(msg)s")
class HNASSSCContextChange(ManilaException):
message = _("HNAS SSC Context has been changed unexpectedly: %(msg)s")
class HNASDirectoryNotEmpty(ManilaException):
message = _("HNAS Directory is not empty: %(msg)s")
class HNASItemNotFoundException(StorageResourceNotFound):
message = _("HNAS Item Not Found Exception: %(msg)s")

4
manila/share/drivers/hitachi/hnas/driver.py
View File

@ -1138,7 +1138,7 @@ class HitachiHNASDriver(driver.ShareDriver):
snapshot['share']['share_proto'])
self._check_fs_mounted()
self.hnas.check_snapshot(snapshot['provider_location'])
self.hnas.check_directory(snapshot['provider_location'])
export_list = None
if snapshot['share'].get('mount_snapshot_support'):
@ -1260,7 +1260,7 @@ class HitachiHNASDriver(driver.ShareDriver):
'share_id': snapshot['share_id']}
raise exception.ManageInvalidShareSnapshot(reason=msg)
if not self.hnas.check_snapshot(snapshot['provider_location']):
if not self.hnas.check_directory(snapshot['provider_location']):
msg = _("Snapshot %(snap_id)s does not exist in "
"HNAS.") % {'snap_id': hnas_snapshot_id}
raise exception.ManageInvalidShareSnapshot(reason=msg)

54
manila/share/drivers/hitachi/hnas/ssh.py
View File

@ -353,15 +353,39 @@ class HNASSSHBackend(object):
LOG.exception(msg)
raise exception.HNASBackendException(msg=msg)
@mutils.retry(exception=exception.HNASSSCContextChange, wait_random=True,
retries=5)
def create_directory(self, dest_path):
self._locked_selectfs('create', dest_path)
if not self.check_directory(dest_path):
msg = _("Command to create directory %(path)s was run in another "
"filesystem instead of %(fs)s.") % {
'path': dest_path,
'fs': self.fs_name,
}
LOG.warning(msg)
raise exception.HNASSSCContextChange(msg=msg)
@mutils.retry(exception=exception.HNASSSCContextChange, wait_random=True,
retries=5)
def delete_directory(self, path):
self._locked_selectfs('delete', path)
try:
self._locked_selectfs('delete', path)
except exception.HNASDirectoryNotEmpty:
pass
else:
if self.check_directory(path):
msg = _("Command to delete empty directory %(path)s was run in"
" another filesystem instead of %(fs)s.") % {
'path': path,
'fs': self.fs_name,
}
LOG.debug(msg)
raise exception.HNASSSCContextChange(msg=msg)
@mutils.retry(exception=exception.HNASSSCIsBusy, wait_random=True,
retries=5)
def check_snapshot(self, path):
def check_directory(self, path):
command = ['path-to-object-number', '-f', self.fs_name, path]
try:
@ -652,10 +676,16 @@ class HNASSSHBackend(object):
self.evs_id, 'mkdir', '-p', path]
try:
self._execute(command)
except processutils.ProcessExecutionError:
msg = _("Failed to create directory %s.") % path
LOG.exception(msg)
raise exception.HNASBackendException(msg=msg)
except processutils.ProcessExecutionError as e:
if "Current file system invalid: VolumeNotFound" in e.stderr:
msg = _("Command to create directory %s failed due to "
"context change.") % path
LOG.debug(msg)
raise exception.HNASSSCContextChange(msg=msg)
else:
msg = _("Failed to create directory %s.") % path
LOG.exception(msg)
raise exception.HNASBackendException(msg=msg)
if op == 'delete':
command = ['selectfs', self.fs_name, '\n',
@ -665,11 +695,17 @@ class HNASSSHBackend(object):
self._execute(command)
except processutils.ProcessExecutionError as e:
if 'DirectoryNotEmpty' in e.stderr:
LOG.debug("Share %(path)s has more snapshots.",
{'path': path})
elif 'NotFound' in e.stderr:
msg = _("Share %s has more snapshots.") % path
LOG.debug(msg)
raise exception.HNASDirectoryNotEmpty(msg=msg)
elif 'cannot remove' in e.stderr and 'NotFound' in e.stderr:
LOG.warning(_LW("Attempted to delete path %s but it does "
"not exist."), path)
elif 'Current file system invalid: VolumeNotFound' in e.stderr:
msg = _("Command to delete empty directory %s failed due "
"to context change.") % path
LOG.debug(msg)
raise exception.HNASSSCContextChange(msg=msg)
else:
msg = _("Failed to delete directory %s.") % path
LOG.exception(msg)

28
manila/tests/share/drivers/hitachi/hnas/test_driver.py
View File

@ -235,7 +235,7 @@ class HitachiHNASTestCase(test.TestCase):
self.mock_object(ssh.HNASSSHBackend, "check_quota", mock.Mock())
self.mock_object(ssh.HNASSSHBackend, "check_cifs", mock.Mock())
self.mock_object(ssh.HNASSSHBackend, "check_export", mock.Mock())
self.mock_object(ssh.HNASSSHBackend, 'check_snapshot')
self.mock_object(ssh.HNASSSHBackend, 'check_directory')
@ddt.data('hitachi_hnas_driver_helper', 'hitachi_hnas_evs_id',
'hitachi_hnas_evs_ip', 'hitachi_hnas_ip', 'hitachi_hnas_user')
@ -990,12 +990,12 @@ class HitachiHNASTestCase(test.TestCase):
else:
expected = None
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot['provider_location'])
self.assertEqual(expected, result)
def test_manage_existing_snapshot(self):
self.mock_object(ssh.HNASSSHBackend, 'check_snapshot',
self.mock_object(ssh.HNASSSHBackend, 'check_directory',
mock.Mock(return_value=True))
self.mock_object(self._driver, '_ensure_snapshot',
mock.Mock(return_value=[]))
@ -1006,7 +1006,7 @@ class HitachiHNASTestCase(test.TestCase):
out = self._driver.manage_existing_snapshot(manage_snapshot,
{'size': 20})
ssh.HNASSSHBackend.check_snapshot.assert_called_with(
ssh.HNASSSHBackend.check_directory.assert_called_with(
'/snapshots/aa4a7710-f326-41fb-ad18-b4ad587fc87a'
'/snapshot18-05-2106')
self._driver._ensure_snapshot.assert_called_with(
@ -1022,7 +1022,7 @@ class HitachiHNASTestCase(test.TestCase):
'path': '172.24.44.10:/snapshots/'
'3377b015-a695-4a5a-8aa5-9b931b023380'}]
self.mock_object(ssh.HNASSSHBackend, 'check_snapshot',
self.mock_object(ssh.HNASSSHBackend, 'check_directory',
mock.Mock(return_value=True))
self.mock_object(self._driver, '_ensure_snapshot',
mock.Mock(return_value=[], side_effect=exc))
@ -1038,7 +1038,7 @@ class HitachiHNASTestCase(test.TestCase):
snapshot_mount_support_nfs,
{'size': 20, 'export_locations': export_locations})
ssh.HNASSSHBackend.check_snapshot.assert_called_with(
ssh.HNASSSHBackend.check_directory.assert_called_with(
'/snapshots/62125744-fcdd-4f55-a8c1-d1498102f634'
'/3377b015-a695-4a5a-8aa5-9b931b023380')
self._driver._ensure_snapshot.assert_called_with(
@ -1081,7 +1081,7 @@ class HitachiHNASTestCase(test.TestCase):
self.assertTrue(self.mock_log.debug.called)
def test_manage_inexistent_snapshot_exception(self):
self.mock_object(ssh.HNASSSHBackend, 'check_snapshot',
self.mock_object(ssh.HNASSSHBackend, 'check_directory',
mock.Mock(return_value=False))
self.assertRaises(exception.ManageInvalidShareSnapshot,
@ -1120,7 +1120,7 @@ class HitachiHNASTestCase(test.TestCase):
ssh.HNASSSHBackend.tree_clone.assert_called_once_with(
'/'.join(('/snapshots', snap['share_id'], snap['id'])),
'/'.join(('/shares', snap['share_id'])))
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snap['provider_location'])
if exc:
@ -1146,7 +1146,7 @@ class HitachiHNASTestCase(test.TestCase):
ssh.HNASSSHBackend.update_nfs_access_rule.assert_called_once_with(
[access1['access_to'] + '(ro)', access2['access_to'] + '(ro)'],
snapshot_id=snapshot_nfs['id'])
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot_nfs['provider_location'])
self.assertTrue(self.mock_log.debug.called)
@ -1163,7 +1163,7 @@ class HitachiHNASTestCase(test.TestCase):
ssh.HNASSSHBackend.update_nfs_access_rule.assert_called_once_with(
[], snapshot_id=snapshot_nfs['id'])
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot_nfs['provider_location'])
self.assertTrue(self.mock_log.debug.called)
@ -1176,7 +1176,7 @@ class HitachiHNASTestCase(test.TestCase):
self.assertRaises(exception.InvalidSnapshotAccess,
self._driver.snapshot_update_access, 'ctxt',
snapshot_nfs, [access1], [], [])
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot_nfs['provider_location'])
def test_cifs_snapshot_update_access_allow(self):
@ -1192,7 +1192,7 @@ class HitachiHNASTestCase(test.TestCase):
ssh.HNASSSHBackend.cifs_allow_access.assert_called_with(
snapshot_cifs['id'], access1['access_to'], 'ar', is_snapshot=True)
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot_cifs['provider_location'])
self.assertTrue(self.mock_log.debug.called)
@ -1209,7 +1209,7 @@ class HitachiHNASTestCase(test.TestCase):
ssh.HNASSSHBackend.cifs_deny_access.assert_called_with(
snapshot_cifs['id'], access1['access_to'], is_snapshot=True)
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot_cifs['provider_location'])
self.assertTrue(self.mock_log.debug.called)
@ -1241,6 +1241,6 @@ class HitachiHNASTestCase(test.TestCase):
ssh.HNASSSHBackend.cifs_allow_access.assert_called_with(
snapshot_cifs['id'], access2['access_to'].replace('\\', '\\\\'),
'ar', is_snapshot=True)
ssh.HNASSSHBackend.check_snapshot.assert_called_once_with(
ssh.HNASSSHBackend.check_directory.assert_called_once_with(
snapshot_cifs['provider_location'])
self.assertTrue(self.mock_log.debug.called)

139
manila/tests/share/drivers/hitachi/hnas/test_ssh.py
View File

@ -511,6 +511,7 @@ class HNASSSHTestCase(test.TestCase):
'share_id': 'vvol_test',
'host': 'ubuntu@hitachi2#HITACHI2',
}
self.mock_log.debug.reset_mock()
def test_get_stats(self):
fake_list_command = ['df', '-a', '-f', self.fs_name]
@ -811,7 +812,7 @@ class HNASSSHTestCase(test.TestCase):
self._driver_ssh.cifs_deny_access('vvol_test', 'fake_user')
self._driver_ssh._execute.assert_called_with(fake_cifs_deny_command)
self.assertTrue(self.mock_log.debug.called)
self.assertTrue(self.mock_log.warning.called)
def test_cifs_deny_access_backend_exception(self):
fake_cifs_deny_command = ['cifs-saa', 'delete', '--target-label',
@ -963,35 +964,114 @@ class HNASSSHTestCase(test.TestCase):
def test_create_directory(self):
locked_selectfs_args = ['create', '/path']
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs", mock.Mock())
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs")
self.mock_object(ssh.HNASSSHBackend, "check_directory",
mock.Mock(return_value=True))
self._driver_ssh.create_directory("/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_called_once_with('/path')
self.assertFalse(self.mock_log.warning.called)
def test_create_directory_context_change_fail(self):
locked_selectfs_args = ['create', '/path']
self.mock_object(time, 'sleep')
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs")
self.mock_object(ssh.HNASSSHBackend, "check_directory",
mock.Mock(return_value=False))
self.assertRaises(exception.HNASSSCContextChange,
self._driver_ssh.create_directory, "/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_called_with('/path')
self.assertTrue(self.mock_log.warning.called)
def test_create_directory_context_change_success(self):
locked_selectfs_args = ['create', '/path']
self.mock_object(time, 'sleep')
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs")
self.mock_object(ssh.HNASSSHBackend, "check_directory",
mock.Mock(side_effect=[False, False, True]))
self._driver_ssh.create_directory("/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_called_with('/path')
self.assertTrue(self.mock_log.warning.called)
def test_delete_directory(self):
locked_selectfs_args = ['delete', '/path']
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs", mock.Mock())
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs")
self.mock_object(ssh.HNASSSHBackend, "check_directory",
mock.Mock(return_value=False))
self._driver_ssh.delete_directory("/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_called_once_with('/path')
self.assertFalse(self.mock_log.debug.called)
def test_check_snapshot(self):
def test_delete_directory_directory_not_empty(self):
locked_selectfs_args = ['delete', '/path']
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs", mock.Mock(
side_effect=exception.HNASDirectoryNotEmpty(msg='fake')))
self.mock_object(ssh.HNASSSHBackend, "check_directory")
self._driver_ssh.delete_directory("/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_not_called()
self.assertFalse(self.mock_log.debug.called)
def test_delete_directory_context_change_fail(self):
locked_selectfs_args = ['delete', '/path']
self.mock_object(time, 'sleep')
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs")
self.mock_object(ssh.HNASSSHBackend, "check_directory",
mock.Mock(return_value=True))
self.assertRaises(exception.HNASSSCContextChange,
self._driver_ssh.delete_directory, "/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_called_with('/path')
self.assertTrue(self.mock_log.debug.called)
def test_delete_directory_context_change_success(self):
locked_selectfs_args = ['delete', '/path']
self.mock_object(time, 'sleep')
self.mock_object(ssh.HNASSSHBackend, "_locked_selectfs")
self.mock_object(ssh.HNASSSHBackend, "check_directory",
mock.Mock(side_effect=[True, True, False]))
self._driver_ssh.delete_directory("/path")
self._driver_ssh._locked_selectfs.assert_called_with(
*locked_selectfs_args)
ssh.HNASSSHBackend.check_directory.assert_called_with('/path')
self.assertTrue(self.mock_log.debug.called)
def test_check_directory(self):
path = ("/snapshots/" + self.snapshot['share_id'] + "/" +
self.snapshot['id'])
check_snap_args = ['path-to-object-number', '-f', self.fs_name, path]
self.mock_object(ssh.HNASSSHBackend, '_execute')
out = self._driver_ssh.check_snapshot(path)
out = self._driver_ssh.check_directory(path)
self.assertTrue(out)
self._driver_ssh._execute.assert_called_with(check_snap_args)
def test_check_snapshot_retry(self):
def test_check_directory_retry(self):
error_msg = ("Unable to run path-to-object-number as "
"path-to-object-number is currently running on volume "
"39.")
@ -1006,7 +1086,7 @@ class HNASSSHTestCase(test.TestCase):
stdout=error_msg), putils.ProcessExecutionError(
stdout=error_msg), 'Object number: 0x45a4']))
out = self._driver_ssh.check_snapshot(path)
out = self._driver_ssh.check_directory(path)
self.assertIs(True, out)
self._driver_ssh._execute.assert_called_with(check_snap_args)
@ -1020,12 +1100,12 @@ class HNASSSHTestCase(test.TestCase):
mock.Mock(side_effect=putils.ProcessExecutionError(
stdout=HNAS_RESULT_check_snap_error)))
out = self._driver_ssh.check_snapshot(path)
out = self._driver_ssh.check_directory(path)
self.assertFalse(out)
self._driver_ssh._execute.assert_called_with(check_snap_args)
def test_check_snapshot_error(self):
def test_check_directory_error(self):
path = "/path/snap1/snapshot07-08-2016"
check_snap_args = ['path-to-object-number', '-f', self.fs_name, path]
@ -1035,7 +1115,7 @@ class HNASSSHTestCase(test.TestCase):
stdout="Internal Server Error.")))
self.assertRaises(exception.HNASBackendException,
self._driver_ssh.check_snapshot, path)
self._driver_ssh.check_directory, path)
self._driver_ssh._execute.assert_called_with(check_snap_args)
@ -1100,7 +1180,7 @@ class HNASSSHTestCase(test.TestCase):
self._driver_ssh.vvol_delete("vvol")
self.assertTrue(self.mock_log.debug.called)
self.assertTrue(self.mock_log.warning.called)
self._driver_ssh._execute.assert_called_with(fake_vvol_delete_command)
def test_vvol_delete_error(self):
@ -1422,14 +1502,31 @@ class HNASSSHTestCase(test.TestCase):
exec_command = ['selectfs', self.fs_name, '\n', 'ssc', '127.0.0.1',
'console-context', '--evs', six.text_type(self.evs_id),
'mkdir', '-p', '/path']
self.mock_object(ssh.HNASSSHBackend, '_execute',
mock.Mock(side_effect=putils.ProcessExecutionError))
self.mock_object(
ssh.HNASSSHBackend, '_execute',
mock.Mock(side_effect=putils.ProcessExecutionError(
stderr="some error")))
self.assertRaises(exception.HNASBackendException,
self._driver_ssh._locked_selectfs, 'create', '/path')
self._driver_ssh._execute.assert_called_with(exec_command)
def test__locked_selectfs_create_operation_context_change(self):
exec_command = ['selectfs', self.fs_name, '\n', 'ssc', '127.0.0.1',
'console-context', '--evs', six.text_type(self.evs_id),
'mkdir', '-p', '/path']
self.mock_object(
ssh.HNASSSHBackend, '_execute',
mock.Mock(side_effect=putils.ProcessExecutionError(
stderr="Current file system invalid: VolumeNotFound")))
self.assertRaises(exception.HNASSSCContextChange,
self._driver_ssh._locked_selectfs, 'create', '/path')
self._driver_ssh._execute.assert_called_with(exec_command)
self.assertTrue(self.mock_log.debug.called)
def test__locked_selectfs_delete_operation_successful(self):
exec_command = ['selectfs', self.fs_name, '\n', 'ssc', '127.0.0.1',
'console-context', '--evs', six.text_type(self.evs_id),
@ -1446,7 +1543,8 @@ class HNASSSHTestCase(test.TestCase):
self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock(
side_effect=[putils.ProcessExecutionError(stderr=msg)]))
self._driver_ssh._locked_selectfs('delete', '/path')
self.assertRaises(exception.HNASDirectoryNotEmpty,
self._driver_ssh._locked_selectfs, 'delete', '/path')
self.assertTrue(self.mock_log.debug.called)
@ -1461,7 +1559,7 @@ class HNASSSHTestCase(test.TestCase):
self.assertTrue(self.mock_log.exception.called)
def test__locked_selectfs_delete_not_found(self):
msg = "rmdir: NotFound '/path'"
msg = "rmdir: cannot remove '/path': NotFound"
self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock(
side_effect=[putils.ProcessExecutionError(stderr=msg)]))
@ -1469,3 +1567,14 @@ class HNASSSHTestCase(test.TestCase):
self._driver_ssh._locked_selectfs('delete', 'path')
self.assertTrue(self.mock_log.warning.called)
def test__locked_selectfs_delete_context_change(self):
msg = "Current file system invalid: VolumeNotFound"
self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock(
side_effect=[putils.ProcessExecutionError(stderr=msg)]))
self.assertRaises(exception.HNASSSCContextChange,
self._driver_ssh._locked_selectfs, 'delete', 'path')
self.assertTrue(self.mock_log.debug.called)

5
releasenotes/notes/bug-1662615-hnas-snapshot-concurrency-2147159ea6b086c5.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- Fixed HNAS driver error when creating or deleting
snapshots caused by concurrency in backend.