From 721bb70a81b80bf065705e8790bd117eae806e7d Mon Sep 17 00:00:00 2001 From: Tom Barron Date: Tue, 30 Oct 2018 10:25:01 -0400 Subject: [PATCH] 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/630221/ Change-Id: Iad78b8cc25c34d3675cd38ce9c757ad0e411b613 --- manila/share/drivers/cephfs/driver.py | 27 +++++- .../tests/share/drivers/cephfs/test_driver.py | 82 +++++++++++++++++-- .../cephfs-set-mode-b7fb3ec51300c220.yaml | 8 ++ 3 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/cephfs-set-mode-b7fb3ec51300c220.yaml diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index cda4cd46bc..fc0b081b7e 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/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, [] diff --git a/manila/tests/share/drivers/cephfs/test_driver.py b/manila/tests/share/drivers/cephfs/test_driver.py index 81aca12130..06a448e031 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/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) diff --git a/releasenotes/notes/cephfs-set-mode-b7fb3ec51300c220.yaml b/releasenotes/notes/cephfs-set-mode-b7fb3ec51300c220.yaml new file mode 100644 index 0000000000..bac9171762 --- /dev/null +++ b/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.