From 9d6823b3eaf289139e415f025b507901b419b8e4 Mon Sep 17 00:00:00 2001 From: Alyson Rosa Date: Tue, 16 Aug 2016 15:00:24 -0300 Subject: [PATCH] Add cleanup to create from snap in Manila HNAS driver Adding a cleanup to method create_from_snapshot in case of a failure. Also, raising the correct exception on backend layer in case of a failure when exporting shares. Change-Id: I86d2c3c5ff5a790868f8362e065df1eb2be8a3ad Closes-Bug: #1613721 --- manila/share/drivers/hitachi/hnas/driver.py | 24 ++++++++++----- manila/share/drivers/hitachi/hnas/ssh.py | 14 +++++++-- .../share/drivers/hitachi/hnas/test_driver.py | 29 +++++++++++++++++++ .../share/drivers/hitachi/hnas/test_ssh.py | 16 ++++++++++ ...reate-from-snap-hnas-0e0431f1fc861a4e.yaml | 4 +++ 5 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/add-cleanup-create-from-snap-hnas-0e0431f1fc861a4e.yaml diff --git a/manila/share/drivers/hitachi/hnas/driver.py b/manila/share/drivers/hitachi/hnas/driver.py index 7547930f..d899ecc2 100644 --- a/manila/share/drivers/hitachi/hnas/driver.py +++ b/manila/share/drivers/hitachi/hnas/driver.py @@ -23,7 +23,7 @@ import six from manila.common import constants from manila import exception -from manila.i18n import _, _LI, _LW +from manila.i18n import _, _LE, _LI, _LW from manila.share import driver LOG = log.getLogger(__name__) @@ -812,13 +812,21 @@ class HitachiHNASDriver(driver.ShareDriver): self._check_protocol(share['id'], share['share_proto']) - if share['share_proto'].lower() == 'nfs': - self.hnas.nfs_export_add(share['id']) - uri = self.hnas_evs_ip + ":" + dest_path - else: - self.hnas.cifs_share_add(share['id']) - uri = r'\\%s\%s' % (self.hnas_evs_ip, share['id']) - return uri + try: + if share['share_proto'].lower() == 'nfs': + self.hnas.nfs_export_add(share['id']) + uri = self.hnas_evs_ip + ":" + dest_path + else: + self.hnas.cifs_share_add(share['id']) + uri = r'\\%s\%s' % (self.hnas_evs_ip, share['id']) + return uri + except exception.HNASBackendException: + with excutils.save_and_reraise_exception(): + msg = _LE('Failed to create share %(share_id)s from snapshot ' + '%(snap)s.') + LOG.exception(msg, {'share_id': share['id'], + 'snap': snapshot['id']}) + self.hnas.vvol_delete(share['id']) def _check_protocol(self, share_id, protocol): if protocol.lower() not in ('nfs', 'cifs'): diff --git a/manila/share/drivers/hitachi/hnas/ssh.py b/manila/share/drivers/hitachi/hnas/ssh.py index 97bcf9e9..58357388 100644 --- a/manila/share/drivers/hitachi/hnas/ssh.py +++ b/manila/share/drivers/hitachi/hnas/ssh.py @@ -65,7 +65,12 @@ class HNASSSHBackend(object): path = '/shares/' + share_id command = ['nfs-export', 'add', '-S', 'disable', '-c', '127.0.0.1', path, self.fs_name, path] - self._execute(command) + try: + self._execute(command) + except processutils.ProcessExecutionError: + msg = _("Could not create NFS export %s.") % share_id + LOG.exception(msg) + raise exception.HNASBackendException(msg=msg) def nfs_export_del(self, share_id): path = '/shares/' + share_id @@ -85,7 +90,12 @@ class HNASSSHBackend(object): path = r'\\shares\\' + share_id command = ['cifs-share', 'add', '-S', 'disable', '--enable-abe', '--nodefaultsaa', share_id, self.fs_name, path] - self._execute(command) + try: + self._execute(command) + except processutils.ProcessExecutionError: + msg = _("Could not create CIFS share %s.") % share_id + LOG.exception(msg) + raise exception.HNASBackendException(msg=msg) def cifs_share_del(self, share_id): command = ['cifs-share', 'del', '--target-label', self.fs_name, diff --git a/manila/tests/share/drivers/hitachi/hnas/test_driver.py b/manila/tests/share/drivers/hitachi/hnas/test_driver.py index 3b5e30ab..e0677690 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_driver.py +++ b/manila/tests/share/drivers/hitachi/hnas/test_driver.py @@ -707,6 +707,35 @@ class HitachiHNASTestCase(test.TestCase): 'context', invalid_share, snapshot_nfs) self.assertEqual(invalid_protocol_msg, ex.msg) + def test_create_share_from_snapshot_cleanup(self): + dest_path = '/snapshots/' + share_nfs['id'] + '/' + snapshot_nfs['id'] + src_path = '/shares/' + share_nfs['id'] + + self.mock_object(driver.HitachiHNASDriver, "_check_fs_mounted", + mock.Mock()) + self.mock_object(ssh.HNASSSHBackend, "vvol_create") + self.mock_object(ssh.HNASSSHBackend, "quota_add") + self.mock_object(ssh.HNASSSHBackend, "tree_clone") + self.mock_object(ssh.HNASSSHBackend, "vvol_delete") + self.mock_object(ssh.HNASSSHBackend, "nfs_export_add", mock.Mock( + side_effect=exception.HNASBackendException( + msg='Error adding nfs export.'))) + + self.assertRaises(exception.HNASBackendException, + self._driver.create_share_from_snapshot, + 'context', share_nfs, snapshot_nfs) + + ssh.HNASSSHBackend.vvol_create.assert_called_once_with( + share_nfs['id']) + ssh.HNASSSHBackend.quota_add.assert_called_once_with( + share_nfs['id'], share_nfs['size']) + ssh.HNASSSHBackend.tree_clone.assert_called_once_with( + dest_path, src_path) + ssh.HNASSSHBackend.nfs_export_add.assert_called_once_with( + share_nfs['id']) + ssh.HNASSSHBackend.vvol_delete.assert_called_once_with( + share_nfs['id']) + def test__check_fs_mounted(self): self._driver._check_fs_mounted() diff --git a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py index 3dfd5fcc..76c62535 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py +++ b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py @@ -545,6 +545,14 @@ class HNASSSHTestCase(test.TestCase): self._driver_ssh._execute.assert_called_with(fake_nfs_command) + def test_nfs_export_add_error(self): + self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock( + side_effect=[putils.ProcessExecutionError(stderr='')])) + + self.assertRaises(exception.HNASBackendException, + self._driver_ssh.nfs_export_add, 'vvol_test') + self.assertTrue(self.mock_log.exception.called) + def test_nfs_export_del(self): fake_nfs_command = ['nfs-export', 'del', '/shares/vvol_test'] self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock()) @@ -581,6 +589,14 @@ class HNASSSHTestCase(test.TestCase): self._driver_ssh._execute.assert_called_with(fake_cifs_add_command) + def test_cifs_share_add_error(self): + self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock( + side_effect=[putils.ProcessExecutionError(stderr='')])) + + self.assertRaises(exception.HNASBackendException, + self._driver_ssh.cifs_share_add, 'vvol_test') + self.assertTrue(self.mock_log.exception.called) + def test_cifs_share_del(self): fake_cifs_del_command = ['cifs-share', 'del', '--target-label', self.fs_name, 'vvol_test'] diff --git a/releasenotes/notes/add-cleanup-create-from-snap-hnas-0e0431f1fc861a4e.yaml b/releasenotes/notes/add-cleanup-create-from-snap-hnas-0e0431f1fc861a4e.yaml new file mode 100644 index 00000000..8d28fba4 --- /dev/null +++ b/releasenotes/notes/add-cleanup-create-from-snap-hnas-0e0431f1fc861a4e.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixed Hitachi HNAS driver not cleaning up data in backend when failing + to create a share from snapshot.