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

@@ -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(

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 '\\\\%s' + ('\\%s' % share_name)
-        return ['\\\\%(ip)s\\%(share_name)s' % data]
 
     def _get_share_path_by_name(self, server, share_name,
                                 ignore_missing=False):

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,
+        result = self._win_smb_helper.create_exports(self._FAKE_SERVER,
-                                                    self._FAKE_SHARE_NAME)
+                                                     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,
+        self._win_smb_helper.remove_exports(mock.sentinel.server,
-                                           mock.sentinel.share_name)
+                                            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)

releasenotes/notes/bug-1663300-554e9c78ca2ba992.yaml

@@ -0,0 +1,4 @@
+fixes:
+  - |
+    The Windows driver issues regarding
+    share creation have been fixed.