From 4e6be9ab3c899620440d1bfff3e37606ea60cd2b Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 23 Jan 2017 15:22:22 +0200 Subject: [PATCH] Fix Windows SMB helper The Windows SMB driver inherits the Generic driver, reusing most of its logic. The following change broke the Windows SMB driver and helpers, changing a few method signatures and return values: I6e7eca54725d1aef994d617f253e73909a9d8303 This patch fixes the Windows SMB helper based on the above mentioned change. Closes-Bug: #1663300 Change-Id: I1ce56ba8abaddda1bf3f0b885f72ad713f36de56 --- manila/share/drivers/helpers.py | 38 ++++++++++--------- .../drivers/windows/windows_smb_helper.py | 13 +++---- .../windows/test_windows_smb_helper.py | 27 +++++++------ .../notes/bug-1663300-554e9c78ca2ba992.yaml | 4 ++ 4 files changed, 45 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/bug-1663300-554e9c78ca2ba992.yaml diff --git a/manila/share/drivers/helpers.py b/manila/share/drivers/helpers.py index 769ffe804f..63f8e54a75 100644 --- a/manila/share/drivers/helpers.py +++ b/manila/share/drivers/helpers.py @@ -346,7 +346,26 @@ class NFSHelper(NASHelperBase): self._ssh_exec(server, restore_exports) -class CIFSHelperIPAccess(NASHelperBase): +class CIFSHelperBase(NASHelperBase): + @staticmethod + def _get_share_group_name_from_export_location(export_location): + if '/' in export_location and '\\' in export_location: + pass + elif export_location.startswith('\\\\'): + return export_location.split('\\')[-1] + elif export_location.startswith('//'): + return export_location.split('/')[-1] + + msg = _("Got incorrect CIFS export location '%s'.") % export_location + raise exception.InvalidShare(reason=msg) + + def _get_export_location_template(self, export_location_or_path): + group_name = self._get_share_group_name_from_export_location( + export_location_or_path) + return ('\\\\%s' + ('\\%s' % group_name)) + + +class CIFSHelperIPAccess(CIFSHelperBase): """Manage shares in samba server by net conf tool. Class provides functionality to operate with CIFS shares. @@ -446,23 +465,6 @@ class CIFSHelperIPAccess(NASHelperBase): self._ssh_exec(server, ['sudo', 'net', 'conf', 'setparm', share_name, 'hosts allow', value]) - @staticmethod - def _get_share_group_name_from_export_location(export_location): - if '/' in export_location and '\\' in export_location: - pass - elif export_location.startswith('\\\\'): - return export_location.split('\\')[-1] - elif export_location.startswith('//'): - return export_location.split('/')[-1] - - msg = _("Got incorrect CIFS export location '%s'.") % export_location - raise exception.InvalidShare(reason=msg) - - def _get_export_location_template(self, export_location_or_path): - group_name = self._get_share_group_name_from_export_location( - export_location_or_path) - return ('\\\\%s' + ('\\%s' % group_name)) - def get_share_path_by_export_location(self, server, export_location): # Get name of group that contains share data on CIFS server group_name = self._get_share_group_name_from_export_location( diff --git a/manila/share/drivers/windows/windows_smb_helper.py b/manila/share/drivers/windows/windows_smb_helper.py index fd3b1e8234..98860d504b 100644 --- a/manila/share/drivers/windows/windows_smb_helper.py +++ b/manila/share/drivers/windows/windows_smb_helper.py @@ -27,7 +27,7 @@ from manila.share.drivers.windows import windows_utils LOG = log.getLogger(__name__) -class WindowsSMBHelper(helpers.NASHelperBase): +class WindowsSMBHelper(helpers.CIFSHelperBase): _SHARE_ACCESS_RIGHT_MAP = { constants.ACCESS_LEVEL_RW: "Change", constants.ACCESS_LEVEL_RO: "Read"} @@ -64,7 +64,7 @@ class WindowsSMBHelper(helpers.NASHelperBase): def init_helper(self, server): self._remote_exec(server, "Get-SmbShare") - def create_export(self, server, share_name, recreate=False): + def create_exports(self, server, share_name, recreate=False): export_location = '\\\\%s\\%s' % (server['public_address'], share_name) if not self._share_exists(server, share_name): @@ -80,9 +80,9 @@ class WindowsSMBHelper(helpers.NASHelperBase): else: LOG.info(_LI("Skipping creating export %s as it already exists."), share_name) - return export_location + return self.get_exports_for_share(server, export_location) - def remove_export(self, server, share_name): + def remove_exports(self, server, share_name): if self._share_exists(server, share_name): cmd = ['Remove-SmbShare', '-Name', share_name, "-Force"] self._remote_exec(server, cmd) @@ -244,10 +244,9 @@ class WindowsSMBHelper(helpers.NASHelperBase): return self._windows_utils.normalize_path( export_location).split('\\')[-1] - def get_exports_for_share(self, server, old_export_location): + def _get_export_location_template(self, old_export_location): share_name = self._get_share_name(old_export_location) - data = dict(ip=server['public_address'], share_name=share_name) - return ['\\\\%(ip)s\\%(share_name)s' % data] + return '\\\\%s' + ('\\%s' % share_name) def _get_share_path_by_name(self, server, share_name, ignore_missing=False): diff --git a/manila/tests/share/drivers/windows/test_windows_smb_helper.py b/manila/tests/share/drivers/windows/test_windows_smb_helper.py index 5c0d4d2286..c13d737081 100644 --- a/manila/tests/share/drivers/windows/test_windows_smb_helper.py +++ b/manila/tests/share/drivers/windows/test_windows_smb_helper.py @@ -64,11 +64,11 @@ class WindowsSMBHelperTestCase(test.TestCase): @ddt.data(True, False) @mock.patch.object(windows_smb_helper.WindowsSMBHelper, '_share_exists') - def test_create_export(self, share_exists, mock_share_exists): + def test_create_exports(self, share_exists, mock_share_exists): mock_share_exists.return_value = share_exists - result = self._win_smb_helper.create_export(self._FAKE_SERVER, - self._FAKE_SHARE_NAME) + result = self._win_smb_helper.create_exports(self._FAKE_SERVER, + self._FAKE_SHARE_NAME) if not share_exists: cmd = ['New-SmbShare', '-Name', self._FAKE_SHARE_NAME, '-Path', @@ -79,14 +79,22 @@ class WindowsSMBHelperTestCase(test.TestCase): else: self.assertFalse(self._remote_exec.called) - self.assertEqual(self._FAKE_SHARE, result) + expected_exports = [ + { + 'is_admin_only': False, + 'metadata': {'export_location_metadata_example': 'example'}, + 'path': self._FAKE_SHARE + }, + ] + + self.assertEqual(expected_exports, result) @mock.patch.object(windows_smb_helper.WindowsSMBHelper, '_share_exists') - def test_remove_export(self, mock_share_exists): + def test_remove_exports(self, mock_share_exists): mock_share_exists.return_value = True - self._win_smb_helper.remove_export(mock.sentinel.server, - mock.sentinel.share_name) + self._win_smb_helper.remove_exports(mock.sentinel.server, + mock.sentinel.share_name) cmd = ['Remove-SmbShare', '-Name', mock.sentinel.share_name, "-Force"] self._remote_exec.assert_called_once_with(mock.sentinel.server, cmd) @@ -330,11 +338,6 @@ class WindowsSMBHelperTestCase(test.TestCase): result = self._win_smb_helper._get_share_name(self._FAKE_SHARE) self.assertEqual(self._FAKE_SHARE_NAME, result) - def test_exports_for_share(self): - result = self._win_smb_helper.get_exports_for_share( - self._FAKE_SERVER, self._FAKE_SHARE_LOCATION) - self.assertEqual([self._FAKE_SHARE], result) - def test_get_share_path_by_name(self): self._remote_exec.return_value = (self._FAKE_SHARE_LOCATION, mock.sentinel.std_err) diff --git a/releasenotes/notes/bug-1663300-554e9c78ca2ba992.yaml b/releasenotes/notes/bug-1663300-554e9c78ca2ba992.yaml new file mode 100644 index 0000000000..4da7737da5 --- /dev/null +++ b/releasenotes/notes/bug-1663300-554e9c78ca2ba992.yaml @@ -0,0 +1,4 @@ +fixes: + - | + The Windows driver issues regarding + share creation have been fixed.