From 4cb13a00662cfd3605df577270e826621c487fbc Mon Sep 17 00:00:00 2001 From: Miriam Yumi Date: Tue, 31 Jan 2017 18:08:36 -0200 Subject: [PATCH] HNAS: Fix managed snapshots not being mounted When managing snapshots, the driver is not checking if the export exists on backend. This patch fixes this issue by checking the export on backend. A new export is created if required. Change-Id: I2da32016ef99bb307a9e075de0472d56a4817e3c Depends-On: Ie0ce3c4f2e7e29bf82f5c09a80a3fb132b0481a8 Closes-bug: #1660415 --- manila/share/drivers/hitachi/hnas/driver.py | 107 ++++++++++++++- manila/share/drivers/hitachi/hnas/ssh.py | 19 +-- .../share/drivers/hitachi/hnas/test_driver.py | 122 ++++++++++++++++++ .../share/drivers/hitachi/hnas/test_ssh.py | 25 ++-- ...t-on-manage-snapshot-91e094c579ddf1a3.yaml | 4 + 5 files changed, 253 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/fix-hnas-mount-on-manage-snapshot-91e094c579ddf1a3.yaml diff --git a/manila/share/drivers/hitachi/hnas/driver.py b/manila/share/drivers/hitachi/hnas/driver.py index 3a98d86482..47da5bbc89 100644 --- a/manila/share/drivers/hitachi/hnas/driver.py +++ b/manila/share/drivers/hitachi/hnas/driver.py @@ -1117,6 +1117,87 @@ class HitachiHNASDriver(driver.ShareDriver): export = r'\\%s\%s' % (ip, hnas_id) return export + def _ensure_snapshot(self, snapshot, hnas_snapshot_id): + """Ensure that snapshot is exported. + + :param snapshot: Snapshot that will be checked. + :param hnas_snapshot_id: HNAS ID of snapshot that will be checked. + + :returns: Returns a list of dicts containing the snapshot's export + locations or None if mount_snapshot_support is False. + """ + self._check_protocol(snapshot['share_id'], + snapshot['share']['share_proto']) + self._check_fs_mounted() + + export_list = None + if snapshot['share'].get('mount_snapshot_support'): + if snapshot['share']['share_proto'].lower() == 'nfs': + self.hnas.check_export(hnas_snapshot_id, is_snapshot=True) + else: + self.hnas.check_cifs(hnas_snapshot_id) + + export_list = self._get_export_locations( + snapshot['share']['share_proto'], + hnas_snapshot_id, + is_snapshot=True) + + return export_list + + def ensure_snapshot(self, context, snapshot, share_server=None): + """Ensure that snapshot is exported. + + :param context: The `context.RequestContext` object for the request. + :param snapshot: Snapshot that will be checked. + :param share_server: Data structure with share server information. + Not used by this driver. + + :returns: Returns a list of dicts containing the EVS IP concatenated + with the path of snapshot in the filesystem or None if + mount_snapshot_support is False. + + Example for NFS:: + + [ + { + 'path': '172.24.44.10:/snapshots/id', + 'metadata': {}, + 'is_admin_only': False + }, + { + 'path': '192.168.0.10:/snapshots/id', + 'metadata': {}, + 'is_admin_only': True + } + ] + + Example for CIFS:: + + [ + { + 'path': '\\172.24.44.10\id', + 'metadata': {}, + 'is_admin_only': False + }, + { + 'path': '\\192.168.0.10\id', + 'metadata': {}, + 'is_admin_only': True + } + ] + """ + LOG.debug("Ensuring snapshot in HNAS: %(snap)s.", + {'snap': snapshot['id']}) + + hnas_snapshot_id = self._get_hnas_snapshot_id(snapshot) + + export_list = self._ensure_snapshot(snapshot, hnas_snapshot_id) + + LOG.debug("Snapshot ensured in HNAS: %(snap)s, protocol %(proto)s.", + {'snap': snapshot['id'], + 'proto': snapshot['share']['share_proto']}) + return export_list + def manage_existing_snapshot(self, snapshot, driver_options): """Manages a snapshot that exists only in HNAS. @@ -1174,16 +1255,30 @@ class HitachiHNASDriver(driver.ShareDriver): "HNAS.") % {'snap_id': hnas_snapshot_id} raise exception.ManageInvalidShareSnapshot(reason=msg) + try: + self._ensure_snapshot(snapshot, hnas_snapshot_id) + except exception.HNASItemNotFoundException: + LOG.warning(_LW("Export does not exist for snapshot %s, " + "creating a new one."), snapshot['id']) + self._create_export(hnas_share_id, + snapshot['share']['share_proto'], + snapshot_id=hnas_snapshot_id) + + output = {'size': snapshot_size} + if snapshot['share'].get('mount_snapshot_support'): + export_locations = self._get_export_locations( + snapshot['share']['share_proto'], + hnas_snapshot_id, + is_snapshot=True) + output['export_locations'] = export_locations + LOG.info(_LI("Snapshot %(snap_path)s for share %(shr_id)s was " "successfully managed with ID %(snap_id)s."), {'snap_path': snapshot['provider_location'], - 'shr_id': snapshot['share_id'], 'snap_id': snapshot['id']}) + 'shr_id': snapshot['share_id'], + 'snap_id': snapshot['id']}) - export_locations = self._get_export_locations( - snapshot['share']['share_proto'], hnas_snapshot_id, - is_snapshot=True) - - return {'size': snapshot_size, 'export_locations': export_locations} + return output def unmanage_snapshot(self, snapshot): """Unmanage a share snapshot diff --git a/manila/share/drivers/hitachi/hnas/ssh.py b/manila/share/drivers/hitachi/hnas/ssh.py index b96ad345fa..e557c478a6 100644 --- a/manila/share/drivers/hitachi/hnas/ssh.py +++ b/manila/share/drivers/hitachi/hnas/ssh.py @@ -130,7 +130,7 @@ class HNASSSHBackend(object): raise exception.HNASBackendException(msg=msg) def get_nfs_host_list(self, share_id): - export = self._get_share_export(share_id) + export = self._get_export(share_id) return export[0].export_configuration def update_nfs_access_rule(self, host_list, share_id=None, @@ -400,8 +400,8 @@ class HNASSSHBackend(object): msg = (_("Virtual volume %s does not have any quota.") % vvol_name) raise exception.HNASItemNotFoundException(msg=msg) - def check_export(self, vvol_name): - export = self._get_share_export(vvol_name) + def check_export(self, vvol_name, is_snapshot=False): + export = self._get_export(vvol_name, is_snapshot=is_snapshot) if (vvol_name in export[0].export_name and self.fs_name in export[0].file_system_label): return @@ -475,17 +475,20 @@ class HNASSSHBackend(object): quota.usage_unit) return bytes_usage / units.Gi - def _get_share_export(self, share_id): - share_id = '/shares/' + share_id + def _get_export(self, name, is_snapshot=False): + if is_snapshot: + name = '/snapshots/' + name + else: + name = '/shares/' + name - command = ['nfs-export', 'list ', share_id] + command = ['nfs-export', 'list ', name] export_list = [] try: output, err = self._execute(command) except processutils.ProcessExecutionError as e: if 'does not exist' in e.stderr: - msg = _("Export %(share)s was not found in EVS " - "%(evs_id)s.") % {'share': share_id, + msg = _("Export %(name)s was not found in EVS " + "%(evs_id)s.") % {'name': name, 'evs_id': self.evs_id} raise exception.HNASItemNotFoundException(msg=msg) else: diff --git a/manila/tests/share/drivers/hitachi/hnas/test_driver.py b/manila/tests/share/drivers/hitachi/hnas/test_driver.py index cd8eada84a..9ac864a425 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_driver.py +++ b/manila/tests/share/drivers/hitachi/hnas/test_driver.py @@ -65,6 +65,34 @@ share_invalid_host = { 'aa4a7710-f326-41fb-ad18-b4ad587fc87a'}], } +share_mount_support_nfs = { + 'id': '62125744-fcdd-4f55-a8c1-d1498102f634', + 'name': '62125744-fcdd-4f55-a8c1-d1498102f634', + 'size': 50, + 'host': 'hnas', + 'share_proto': 'NFS', + 'share_type_id': 1, + 'share_network_id': 'bb329e24-3bdb-491d-acfd-dfe70c09b98d', + 'share_server_id': 'cc345a53-491d-acfd-3bdb-dfe70c09b98d', + 'export_locations': [{'path': '172.24.44.10:/shares/' + '62125744-fcdd-4f55-a8c1-d1498102f634'}], + 'mount_snapshot_support': True, +} + +share_mount_support_cifs = { + 'id': 'd6e7dc6b-f65f-49d9-968d-936f75474f29', + 'name': 'd6e7dc6b-f65f-49d9-968d-936f75474f29', + 'size': 50, + 'host': 'hnas', + 'share_proto': 'CIFS', + 'share_type_id': 1, + 'share_network_id': 'bb329e24-3bdb-491d-acfd-dfe70c09b98d', + 'share_server_id': 'cc345a53-491d-acfd-3bdb-dfe70c09b98d', + 'export_locations': [{'path': '172.24.44.10:/shares/' + 'd6e7dc6b-f65f-49d9-968d-936f75474f29'}], + 'mount_snapshot_support': True, +} + access_nfs_rw = { 'id': 'acdc7172b-fe07-46c4-b78f-df3e0324ccd0', 'access_type': 'ip', @@ -115,6 +143,22 @@ manage_snapshot = { '/snapshot18-05-2106', } +snapshot_mount_support_nfs = { + 'id': '3377b015-a695-4a5a-8aa5-9b931b023380', + 'share_id': '62125744-fcdd-4f55-a8c1-d1498102f634', + 'share': share_mount_support_nfs, + 'provider_location': '/snapshots/62125744-fcdd-4f55-a8c1-d1498102f634' + '/3377b015-a695-4a5a-8aa5-9b931b023380', +} + +snapshot_mount_support_cifs = { + 'id': 'f9916515-5cb8-4612-afa6-7f2baa74223a', + 'share_id': 'd6e7dc6b-f65f-49d9-968d-936f75474f29', + 'share': share_mount_support_cifs, + 'provider_location': '/snapshots/d6e7dc6b-f65f-49d9-968d-936f75474f29' + '/f9916515-5cb8-4612-afa6-7f2baa74223a', +} + invalid_share = { 'id': 'aa4a7710-f326-41fb-ad18-b4ad587fc87a', 'name': 'aa4a7710-f326-41fb-ad18-b4ad587fc87a', @@ -895,9 +939,42 @@ class HitachiHNASTestCase(test.TestCase): assert_called_once_with(fake_data)) self.assertTrue(self.mock_log.info.called) + @ddt.data(snapshot_nfs, snapshot_cifs, + snapshot_mount_support_nfs, snapshot_mount_support_cifs) + def test_ensure_snapshot(self, snapshot): + result = self._driver.ensure_snapshot('context', snapshot) + + if snapshot['share'].get('mount_snapshot_support'): + expected = [ + self._get_export( + snapshot['id'], snapshot['share']['share_proto'], + self._driver.hnas_evs_ip, False, is_snapshot=True), + self._get_export( + snapshot['id'], snapshot['share']['share_proto'], + self._driver.hnas_admin_network_ip, True, + is_snapshot=True)] + + if snapshot['share']['share_proto'].lower() == 'nfs': + ssh.HNASSSHBackend.check_export.assert_called_once_with( + snapshot['id'], is_snapshot=True) + self.assertFalse(ssh.HNASSSHBackend.check_cifs.called) + else: + ssh.HNASSSHBackend.check_cifs.assert_called_once_with( + snapshot['id']) + self.assertFalse(ssh.HNASSSHBackend.check_export.called) + else: + expected = None + + self.assertEqual(expected, result) + def test_manage_existing_snapshot(self): self.mock_object(ssh.HNASSSHBackend, 'check_snapshot', mock.Mock(return_value=True)) + self.mock_object(self._driver, '_ensure_snapshot', + mock.Mock(return_value=[])) + + path_info = manage_snapshot['provider_location'].split('/') + hnas_snapshot_id = path_info[3] out = self._driver.manage_existing_snapshot(manage_snapshot, {'size': 20}) @@ -905,10 +982,55 @@ class HitachiHNASTestCase(test.TestCase): ssh.HNASSSHBackend.check_snapshot.assert_called_with( '/snapshots/aa4a7710-f326-41fb-ad18-b4ad587fc87a' '/snapshot18-05-2106') + self._driver._ensure_snapshot.assert_called_with( + manage_snapshot, + hnas_snapshot_id) self.assertEqual(20, out['size']) self.assertTrue(self.mock_log.debug.called) self.assertTrue(self.mock_log.info.called) + @ddt.data(None, exception.HNASItemNotFoundException('Fake error.')) + def test_manage_existing_snapshot_with_mount_support(self, exc): + export_locations = [{ + 'path': '172.24.44.10:/snapshots/' + '3377b015-a695-4a5a-8aa5-9b931b023380'}] + + self.mock_object(ssh.HNASSSHBackend, 'check_snapshot', + mock.Mock(return_value=True)) + self.mock_object(self._driver, '_ensure_snapshot', + mock.Mock(return_value=[], side_effect=exc)) + self.mock_object(self._driver, '_get_export_locations', + mock.Mock(return_value=export_locations)) + if exc: + self.mock_object(self._driver, '_create_export') + + path_info = snapshot_mount_support_nfs['provider_location'].split('/') + hnas_snapshot_id = path_info[3] + + out = self._driver.manage_existing_snapshot( + snapshot_mount_support_nfs, + {'size': 20, 'export_locations': export_locations}) + + ssh.HNASSSHBackend.check_snapshot.assert_called_with( + '/snapshots/62125744-fcdd-4f55-a8c1-d1498102f634' + '/3377b015-a695-4a5a-8aa5-9b931b023380') + self._driver._ensure_snapshot.assert_called_with( + snapshot_mount_support_nfs, + hnas_snapshot_id) + self._driver._get_export_locations.assert_called_with( + snapshot_mount_support_nfs['share']['share_proto'], + hnas_snapshot_id, + is_snapshot=True) + if exc: + self._driver._create_export.assert_called_with( + snapshot_mount_support_nfs['share_id'], + snapshot_mount_support_nfs['share']['share_proto'], + snapshot_id=hnas_snapshot_id) + self.assertEqual(20, out['size']) + self.assertEqual(export_locations, out['export_locations']) + self.assertTrue(self.mock_log.debug.called) + self.assertTrue(self.mock_log.info.called) + @ddt.data('fake_size', '128GB', '512 GB', {'size': 128}) def test_manage_snapshot_invalid_size_exception(self, size): self.assertRaises(exception.ManageInvalidShareSnapshot, diff --git a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py index 255bc419d7..c27c919e49 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py +++ b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py @@ -668,7 +668,7 @@ class HNASSSHTestCase(test.TestCase): self._driver_ssh._execute.assert_called_with(fake_cifs_del_command) def test_get_nfs_host_list(self): - self.mock_object(ssh.HNASSSHBackend, "_get_share_export", mock.Mock( + self.mock_object(ssh.HNASSSHBackend, "_get_export", mock.Mock( return_value=[ssh.Export(HNAS_RESULT_export)])) host_list = self._driver_ssh.get_nfs_host_list('fake_id') @@ -1067,14 +1067,15 @@ class HNASSSHTestCase(test.TestCase): self._driver_ssh.check_quota, 'vvol') self._driver_ssh._execute.assert_called_with(fake_check_quota_command) - def test_check_export(self): - self.mock_object(ssh.HNASSSHBackend, "_get_share_export", mock.Mock( + @ddt.data(True, False) + def test_check_export(self, is_snapshot): + self.mock_object(ssh.HNASSSHBackend, "_get_export", mock.Mock( return_value=[ssh.Export(HNAS_RESULT_export)])) - self._driver_ssh.check_export("vvol_test") + self._driver_ssh.check_export("vvol_test", is_snapshot) def test_check_export_error(self): - self.mock_object(ssh.HNASSSHBackend, "_get_share_export", mock.Mock( + self.mock_object(ssh.HNASSSHBackend, "_get_export", mock.Mock( return_value=[ssh.Export(HNAS_RESULT_wrong_export)])) self.assertRaises(exception.HNASItemNotFoundException, @@ -1202,12 +1203,16 @@ class HNASSSHTestCase(test.TestCase): self.assertEqual(1024, self._driver_ssh.get_share_usage("vvol_test")) - def test__get_share_export(self): + @ddt.data(True, False) + def test__get_share_export(self, is_snapshot): self.mock_object(ssh.HNASSSHBackend, '_execute', mock.Mock( return_value=[HNAS_RESULT_export_ip, ''])) - export_list = self._driver_ssh._get_share_export(share_id='share_id') - path = '/shares/share_id' + export_list = self._driver_ssh._get_export( + name='fake_name', is_snapshot=is_snapshot) + path = '/shares/fake_name' + if is_snapshot: + path = '/snapshots/fake_name' command = ['nfs-export', 'list ', path] @@ -1225,7 +1230,7 @@ class HNASSSHTestCase(test.TestCase): stderr="NFS Export List: Export 'id' does not exist."))) self.assertRaises(exception.HNASItemNotFoundException, - self._driver_ssh._get_share_export, 'fake_id') + self._driver_ssh._get_export, 'fake_id') def test__get_share_export_exception_error(self): @@ -1234,7 +1239,7 @@ class HNASSSHTestCase(test.TestCase): )) self.assertRaises(putils.ProcessExecutionError, - self._driver_ssh._get_share_export, 'fake_id') + self._driver_ssh._get_export, 'fake_id') def test__execute(self): key = self.ssh_private_key diff --git a/releasenotes/notes/fix-hnas-mount-on-manage-snapshot-91e094c579ddf1a3.yaml b/releasenotes/notes/fix-hnas-mount-on-manage-snapshot-91e094c579ddf1a3.yaml new file mode 100644 index 0000000000..1c5759ff06 --- /dev/null +++ b/releasenotes/notes/fix-hnas-mount-on-manage-snapshot-91e094c579ddf1a3.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixed Hitachi HNAS driver not checking export on backend + when managing a snapshot.