openstack/manila
Code Issues Proposed changes

[ZFSonLinux] Fix replicated snapshot deletion error

Browse Source 
Commit [1] implemented bug when deletion of replicated snapshots
may fail if active replica deleted not as last among all snapshot
replicas. It was caused by deletion of private data of snapshot,
only by active replica.
So, make that data be deleted only in the end of driver method
that deletes replicated snapshots.

[1] If240ecde5676c334d39faaccd5703e93544aaa06

Change-Id: Ic2b7b7daf0ba5db8eb7166969413b590b27effb5
Closes-Bug: #1607765
This commit is contained in:
Valeriy Ponomaryov 2016-08-01 17:05:01 +03:00
parent d6637a43b8
commit 839ba23018
2 changed files with 32 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
manila/share/drivers/zfsonlinux/driver.py
View File

@ -498,7 +498,8 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
@ensure_share_server_not_provided
def delete_snapshot(self, context, snapshot, share_server=None):
"""Is called to remove a snapshot."""
return self._delete_snapshot(context, snapshot)
self._delete_snapshot(context, snapshot)
self.private_storage.delete(snapshot['snapshot_id'])
def _get_saved_snapshot_name(self, snapshot_instance):
snapshot_tag = self.private_storage.get(
@ -521,7 +522,6 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
_LW("Snapshot with '%(id)s' ID and '%(name)s' NAME is "
"absent on backend. Nothing has been deleted."),
{'id': snapshot['id'], 'name': snapshot_name})
self.private_storage.delete(snapshot['snapshot_id'])
@ensure_share_server_not_provided
def create_share_from_snapshot(self, context, share, snapshot,
@ -1252,6 +1252,7 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
replica_snapshots_dict[replica_snapshot['id']]['status'] = (
constants.STATUS_DELETED)
self.private_storage.delete(replica_snapshot['snapshot_id'])
return list(replica_snapshots_dict.values())
@ensure_share_server_not_provided

45
manila/tests/share/drivers/zfsonlinux/test_driver.py
View File

@ -673,8 +673,17 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
self.driver.private_storage.update(
snapshot['share_instance_id'], {'dataset_name': dataset_name})
self.assertEqual(
snap_tag,
self.driver.private_storage.get(
snapshot['snapshot_id'], 'snapshot_tag'))
self.driver.delete_snapshot(context, snapshot, share_server=None)
self.assertIsNone(
self.driver.private_storage.get(
snapshot['snapshot_id'], 'snapshot_tag'))
self.assertEqual(0, zfs_driver.LOG.warning.call_count)
self.driver.zfs.assert_called_once_with(
'list', '-r', '-t', 'snapshot', snap_name)
@ -1924,6 +1933,8 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
old_repl_snapshot_tag = (
self.driver._get_replication_snapshot_prefix(
active_replica) + 'foo')
new_repl_snapshot_tag = 'foo_snapshot_tag'
dataset_name = 'some_dataset_name'
self.driver.private_storage.update(
active_replica['id'],
{'dataset_name': src_dataset_name,
@ -1933,20 +1944,17 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
for replica in (replica, second_replica):
self.driver.private_storage.update(
replica['id'],
{'dataset_name': 'some_dataset_name',
{'dataset_name': dataset_name,
'ssh_cmd': 'fake_ssh_cmd'}
)
self.driver.private_storage.update(
snapshot_instances[0]['snapshot_id'],
{'snapshot_tag': 'foo_snapshot_tag'}
{'snapshot_tag': new_repl_snapshot_tag}
)
snap_name = 'fake_snap_name'
self.mock_object(self.driver, '_delete_snapshot')
self.mock_object(
self.driver, '_get_saved_snapshot_name',
mock.Mock(return_value=snap_name))
self.mock_object(self.driver, 'execute_with_retry')
self.driver, 'zfs', mock.Mock(return_value=['out', 'err']))
self.mock_object(
self.driver, 'execute', mock.Mock(side_effect=[
('a', 'b'),
@ -1957,31 +1965,36 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
self.driver, 'parse_zfs_answer', mock.Mock(side_effect=[
({'NAME': 'foo'}, {'NAME': snap_name}),
({'NAME': 'bar'}, {'NAME': snap_name}),
[],
]))
expected = sorted([
{'id': si['id'], 'status': 'deleted'} for si in snapshot_instances
], key=lambda item: item['id'])
self.assertEqual(
new_repl_snapshot_tag,
self.driver.private_storage.get(
snapshot_instances[0]['snapshot_id'], 'snapshot_tag'))
result = self.driver.delete_replicated_snapshot(
'fake_context', replica_list, snapshot_instances)
self.driver._get_saved_snapshot_name.assert_has_calls([
mock.call(si) for si in snapshot_instances
])
self.driver._delete_snapshot.assert_called_once_with(
'fake_context', active_snapshot_instance)
self.assertIsNone(
self.driver.private_storage.get(
snapshot_instances[0]['snapshot_id'], 'snapshot_tag'))
self.driver.execute.assert_has_calls([
mock.call('ssh', 'fake_ssh_cmd', 'sudo', 'zfs', 'list', '-r', '-t',
'snapshot', snap_name) for i in (0, 1)
])
self.driver.execute_with_retry.assert_has_calls([
mock.call('ssh', 'fake_ssh_cmd', 'sudo', 'zfs', 'destroy',
'-f', snap_name) for i in (0, 1)
'snapshot', dataset_name + '@' + new_repl_snapshot_tag)
for i in (0, 1)
])
self.assertIsInstance(result, list)
self.assertEqual(3, len(result))
self.assertEqual(expected, sorted(result, key=lambda item: item['id']))
self.driver.parse_zfs_answer.assert_has_calls([
mock.call('out'),
])
@ddt.data(
({'NAME': 'fake'}, zfs_driver.constants.STATUS_ERROR),