Set mode for CephFS volumes and snapshots

Corresponding ceph_volume_client PR [1]


[1] https://github.com/ceph/ceph/pull/24839

Depends-on: https://review.openstack.org/#/c/634028/
Change-Id: Iad78b8cc25c34d3675cd38ce9c757ad0e411b613
(cherry picked from commit 721bb70a81)

manila/share/drivers/cephfs/driver.py

@@ -42,6 +42,8 @@ CEPHX_ACCESS_TYPE = "cephx"
 # The default Ceph administrative identity
 CEPH_DEFAULT_AUTH_ID = "admin"
 
+DEFAULT_VOLUME_MODE = '755'
+
 
 LOG = log.getLogger(__name__)
 
@@ -89,6 +91,11 @@ cephfs_opts = [
                help="The password to authenticate as the user in the remote "
                     "Ganesha server host. This is not required if "
                     "'cephfs_ganesha_path_to_private_key' is configured."),
+    cfg.StrOpt('cephfs_volume_mode',
+               default=DEFAULT_VOLUME_MODE,
+               help="The read/write/execute permissions mode for CephFS "
+                    "volumes, snapshots, and snapshot groups expressed in "
+                    "Octal as with linux 'chmod' or 'umask' commands."),
 ]
 
 
@@ -115,6 +122,14 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
 
         self.configuration.append_config_values(cephfs_opts)
 
+        try:
+            self._cephfs_volume_mode = int(
+                self.configuration.cephfs_volume_mode, 8)
+        except ValueError:
+            msg = _("Invalid CephFS volume mode %s")
+            raise exception.BadConfigurationException(
+                msg % self.configuration.cephfs_volume_mode)
+
     def do_setup(self, context):
         if self.configuration.cephfs_protocol_helper_type.upper() == "CEPHFS":
             protocol_helper_class = getattr(
@@ -237,7 +252,8 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
 
         # Create the CephFS volume
         cephfs_volume = self.volume_client.create_volume(
-            cephfs_share_path(share), size=size, data_isolated=data_isolated)
+            cephfs_share_path(share), size=size, data_isolated=data_isolated,
+            mode=self._cephfs_volume_mode)
 
         return self.protocol_helper.get_export_locations(share, cephfs_volume)
 
@@ -284,7 +300,8 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
     def create_snapshot(self, context, snapshot, share_server=None):
         self.volume_client.create_snapshot_volume(
             cephfs_share_path(snapshot['share']),
-            '_'.join([snapshot['snapshot_id'], snapshot['id']]))
+            '_'.join([snapshot['snapshot_id'], snapshot['id']]),
+            mode=self._cephfs_volume_mode)
 
     def delete_snapshot(self, context, snapshot, share_server=None):
         self.volume_client.destroy_snapshot_volume(
@@ -292,7 +309,8 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
             '_'.join([snapshot['snapshot_id'], snapshot['id']]))
 
     def create_share_group(self, context, sg_dict, share_server=None):
-        self.volume_client.create_group(sg_dict['id'])
+        self.volume_client.create_group(sg_dict['id'],
+                                        mode=self._cephfs_volume_mode)
 
     def delete_share_group(self, context, sg_dict, share_server=None):
         self.volume_client.destroy_group(sg_dict['id'])
@@ -309,7 +327,8 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
                                     share_server=None):
         self.volume_client.create_snapshot_group(
             snap_dict['share_group_id'],
-            snap_dict['id'])
+            snap_dict['id'],
+            mode=self._cephfs_volume_mode)
 
         return None, []
 

manila/tests/share/drivers/cephfs/test_driver.py

@@ -29,6 +29,11 @@ from manila import test
 from manila.tests import fake_share
 
 
+DEFAULT_VOLUME_MODE = 0o755
+ALT_VOLUME_MODE_CFG = '775'
+ALT_VOLUME_MODE = 0o775
+
+
 class MockVolumeClientModule(object):
     """Mocked up version of ceph's VolumeClient interface."""
 
@@ -129,6 +134,8 @@ class CephFSDriverTestCase(test.TestCase):
 
         self._driver.protocol_helper.init_helper.assert_called_once_with()
 
+        self.assertEqual(DEFAULT_VOLUME_MODE, self._driver._cephfs_volume_mode)
+
     def test_create_share(self):
         cephfs_volume = {"mount_path": "/foo/bar"}
 
@@ -137,7 +144,7 @@ class CephFSDriverTestCase(test.TestCase):
         self._driver._volume_client.create_volume.assert_called_once_with(
             driver.cephfs_share_path(self._share),
             size=self._share['size'] * units.Gi,
-            data_isolated=False)
+            data_isolated=False, mode=DEFAULT_VOLUME_MODE)
         (self._driver.protocol_helper.get_export_locations.
             assert_called_once_with(self._share, cephfs_volume))
 
@@ -175,7 +182,8 @@ class CephFSDriverTestCase(test.TestCase):
         self._driver._volume_client.create_volume.assert_called_once_with(
             driver.cephfs_share_path(self._share),
             size=self._share['size'] * units.Gi,
-            data_isolated=False)
+            data_isolated=False,
+            mode=DEFAULT_VOLUME_MODE)
 
     def test_create_data_isolated(self):
         self.mock_object(share_types, 'get_share_type_extra_specs',
@@ -187,7 +195,8 @@ class CephFSDriverTestCase(test.TestCase):
         self._driver._volume_client.create_volume.assert_called_once_with(
             driver.cephfs_share_path(self._share),
             size=self._share['size'] * units.Gi,
-            data_isolated=True)
+            data_isolated=True,
+            mode=DEFAULT_VOLUME_MODE)
 
     def test_delete_share(self):
         self._driver.delete_share(self._context, self._share)
@@ -260,7 +269,8 @@ class CephFSDriverTestCase(test.TestCase):
         (self._driver._volume_client.create_snapshot_volume
             .assert_called_once_with(
                 driver.cephfs_share_path(self._share),
-                "snappy1_instance1"))
+                "snappy1_instance1",
+                mode=DEFAULT_VOLUME_MODE))
 
     def test_delete_snapshot(self):
         self._driver.delete_snapshot(self._context,
@@ -280,7 +290,7 @@ class CephFSDriverTestCase(test.TestCase):
         self._driver.create_share_group(self._context, {"id": "grp1"}, None)
 
         self._driver._volume_client.create_group.assert_called_once_with(
-            "grp1")
+            "grp1", mode=DEFAULT_VOLUME_MODE)
 
     def test_delete_share_group(self):
         self._driver.delete_share_group(self._context, {"id": "grp1"}, None)
@@ -291,16 +301,16 @@ class CephFSDriverTestCase(test.TestCase):
     def test_create_share_snapshot(self):
         self._driver.create_share_group_snapshot(self._context, {
             'share_group_id': 'sgid',
-            'id': 'snapid'
+            'id': 'snapid',
         })
 
         (self._driver._volume_client.create_snapshot_group.
-         assert_called_once_with("sgid", "snapid"))
+         assert_called_once_with("sgid", "snapid", mode=DEFAULT_VOLUME_MODE))
 
     def test_delete_share_group_snapshot(self):
         self._driver.delete_share_group_snapshot(self._context, {
             'share_group_id': 'sgid',
-            'id': 'snapid'
+            'id': 'snapid',
         })
 
         (self._driver._volume_client.destroy_snapshot_group.
@@ -676,3 +686,59 @@ class NFSProtocolHelperTestCase(test.TestCase):
         self._volume_client._get_path.assert_called_once_with(
             'fakevolumepath')
         self.assertEqual('/foo/bar', ret)
+
+
+@ddt.ddt
+class CephFSDriverAltConfigTestCase(test.TestCase):
+    """Test the CephFS driver with non-default config values."""
+
+    def setUp(self):
+        super(CephFSDriverAltConfigTestCase, self).setUp()
+        self._execute = mock.Mock()
+        self.fake_conf = configuration.Configuration(None)
+        self._context = context.get_admin_context()
+        self._share = fake_share.fake_share(share_proto='CEPHFS')
+
+        self.fake_conf.set_default('driver_handles_share_servers', False)
+        self.fake_conf.set_default('cephfs_auth_id', 'manila')
+
+        self.mock_object(driver, "ceph_volume_client",
+                         MockVolumeClientModule)
+        self.mock_object(driver, "ceph_module_found", True)
+        self.mock_object(driver, "cephfs_share_path")
+        self.mock_object(driver, 'NativeProtocolHelper')
+        self.mock_object(driver, 'NFSProtocolHelper')
+
+    @ddt.data('cephfs', 'nfs')
+    def test_do_setup_alt_volume_mode(self, protocol_helper):
+
+        self.fake_conf.set_default('cephfs_volume_mode', ALT_VOLUME_MODE_CFG)
+        self._driver = driver.CephFSDriver(execute=self._execute,
+                                           configuration=self.fake_conf)
+
+        self._driver.configuration.cephfs_protocol_helper_type = (
+            protocol_helper)
+
+        self._driver.do_setup(self._context)
+
+        if protocol_helper == 'cephfs':
+            driver.NativeProtocolHelper.assert_called_once_with(
+                self._execute, self._driver.configuration,
+                ceph_vol_client=self._driver._volume_client)
+        else:
+            driver.NFSProtocolHelper.assert_called_once_with(
+                self._execute, self._driver.configuration,
+                ceph_vol_client=self._driver._volume_client)
+
+        self._driver.protocol_helper.init_helper.assert_called_once_with()
+
+        self.assertEqual(ALT_VOLUME_MODE, self._driver._cephfs_volume_mode)
+
+    @ddt.data('0o759', '0x755', '12a3')
+    def test_volume_mode_exception(self, volume_mode):
+        # cephfs_volume_mode must be a string representing an int as octal
+        self.fake_conf.set_default('cephfs_volume_mode', volume_mode)
+
+        self.assertRaises(exception.BadConfigurationException,
+                          driver.CephFSDriver, execute=self._execute,
+                          configuration=self.fake_conf)

releasenotes/notes/cephfs-set-mode-b7fb3ec51300c220.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Shares backed by CephFS no longer have hard-coded mode 755.  Use
+    the ``cephfs_volume_mode`` configuration option to set another
+    mode, such as 775 when using manila dynamic external storage
+    provider with OpenShift.  The default value remains 755 for
+    backwards compatibility.