From 08b1ad0c9d3143165a8500ad8b473be56b0c5ae3 Mon Sep 17 00:00:00 2001 From: Jordan Pittier Date: Fri, 26 Feb 2016 14:26:07 +0100 Subject: [PATCH] Fix Scality SOFS support The "Scaling backup service" patch [1] in Cinder (merged a couple of days ago) uses os-brick to perform a local attach volume. It revealed that the support for Scality SOFS in os-brick was broken. (we did have a CI in Cinder but the os-brick code path was not fully exercised until [1] merged). This patch addresses this issue. The patch introduces a new class called `ScalityRemoteFsClient` which inherits from `RemoteFsClient`. We can't strictly use the `RemoteFsClient` because how to mount Scality FS is a bit different from other FS. Note that without this patch, the Scality Cinder driver is broken, more precisely cinder backup is broken. So I would appreciate if we can merge that for Mitaka (which needs another minor/micro(?) release of os-brick) [1] https://review.openstack.org/#/c/262395/ Change-Id: Icfa09b124d252d1d6b07d9cff8c63c7c0d65cc30 --- os_brick/initiator/connector.py | 13 ++++++--- os_brick/remotefs/remotefs.py | 34 +++++++++++++++++++++- os_brick/tests/initiator/test_connector.py | 7 ++++- os_brick/tests/remotefs/test_remotefs.py | 32 ++++++++++++++++++-- 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/os_brick/initiator/connector.py b/os_brick/initiator/connector.py index f6ef172f4..fc298ca85 100644 --- a/os_brick/initiator/connector.py +++ b/os_brick/initiator/connector.py @@ -1762,9 +1762,9 @@ class RemoteFsConnector(InitiatorConnector): *args, **kwargs): kwargs = kwargs or {} conn = kwargs.get('conn') + mount_type_lower = mount_type.lower() if conn: mount_point_base = conn.get('mount_point_base') - mount_type_lower = mount_type.lower() if mount_type_lower in ('nfs', 'glusterfs', 'scality', 'quobyte', 'vzstorage'): kwargs[mount_type_lower + '_mount_point_base'] = ( @@ -1773,9 +1773,14 @@ class RemoteFsConnector(InitiatorConnector): else: LOG.warning(_LW("Connection details not present." " RemoteFsClient may not initialize properly.")) - self._remotefsclient = remotefs.RemoteFsClient(mount_type, root_helper, - execute=execute, - *args, **kwargs) + + if mount_type_lower == 'scality': + cls = remotefs.ScalityRemoteFsClient + else: + cls = remotefs.RemoteFsClient + self._remotefsclient = cls(mount_type, root_helper, execute=execute, + *args, **kwargs) + super(RemoteFsConnector, self).__init__( root_helper, driver=driver, execute=execute, diff --git a/os_brick/remotefs/remotefs.py b/os_brick/remotefs/remotefs.py index 4f6b5e226..16b891beb 100644 --- a/os_brick/remotefs/remotefs.py +++ b/os_brick/remotefs/remotefs.py @@ -40,7 +40,6 @@ class RemoteFsClient(object): 'cifs': 'smbfs', 'glusterfs': 'glusterfs', 'vzstorage': 'vzstorage', - 'scality': 'scality_sofs', 'quobyte': 'quobyte' } @@ -230,3 +229,36 @@ class RemoteFsClient(object): opt = '%s=%s' % (option, value) if value else option opts.append(opt) return ",".join(opts) if len(opts) > 1 else opts[0] + + +class ScalityRemoteFsClient(RemoteFsClient): + def __init__(self, mount_type, root_helper, + execute=None, *args, **kwargs): + self._mount_type = mount_type + self._mount_base = kwargs.get( + 'scality_mount_point_base', "").rstrip('/') + if not self._mount_base: + raise exception.InvalidParameterValue( + err=_('scality_mount_point_base required')) + self.root_helper = root_helper + self.set_execute(execute or putils.execute) + self._mount_options = None + + def get_mount_point(self, device_name): + return os.path.join(self._mount_base, + device_name, + "00") + + def mount(self, share, flags=None): + """Mount the Scality ScaleOut FS. + + The `share` argument is ignored because you can't mount several + SOFS at the same type on a single server. But we want to keep the + same method signature for class inheritance purpose. + """ + if self._mount_base in self._read_mounts(): + LOG.info(_LI('Already mounted: %s'), self._mount_base) + return + self._execute('mkdir', '-p', self._mount_base, check_exit_code=0) + super(ScalityRemoteFsClient, self)._do_mount( + 'sofs', '/etc/sfused.conf', self._mount_base) diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index 9c33463af..f117abdba 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/os_brick/tests/initiator/test_connector.py @@ -159,7 +159,7 @@ class ConnectorTestCase(base.TestCase): self.assertEqual(obj.__class__.__name__, "RemoteFsConnector") obj = connector.InitiatorConnector.factory( - 'scality', None, scality_sofs_mount_point_base='/mnt/test') + 'scality', None, scality_mount_point_base='/mnt/test') self.assertEqual(obj.__class__.__name__, "RemoteFsConnector") obj = connector.InitiatorConnector.factory('local', None) @@ -1665,6 +1665,11 @@ class RemoteFsConnectorTestCase(ConnectorTestCase): nfs_mount_point_base=self.TEST_BASE, nfs_mount_options='vers=3') + @mock.patch('os_brick.remotefs.remotefs.ScalityRemoteFsClient') + def test_init_with_scality(self, mock_scality_remotefs_client): + connector.RemoteFsConnector('scality', root_helper='sudo') + self.assertEqual(1, mock_scality_remotefs_client.call_count) + def test_get_search_path(self): expected = self.TEST_BASE actual = self.connector.get_search_path() diff --git a/os_brick/tests/remotefs/test_remotefs.py b/os_brick/tests/remotefs/test_remotefs.py index b8d0dd65e..c70a7c2ae 100644 --- a/os_brick/tests/remotefs/test_remotefs.py +++ b/os_brick/tests/remotefs/test_remotefs.py @@ -174,9 +174,6 @@ class RemoteFsClientTestCase(base.TestCase): def test_no_mount_point_vzstorage(self): self._test_no_mount_point('vzstorage') - def test_no_mount_point_scality(self): - self._test_no_mount_point('scality') - def test_no_mount_point_quobyte(self): self._test_no_mount_point('quobyte') @@ -198,3 +195,32 @@ class RemoteFsClientTestCase(base.TestCase): remotefs.RemoteFsClient("nfs", root_helper='true', nfs_mount_point_base='/fake') mock_check_nfs_options.assert_called_once_with() + + +class ScalityRemoteFsClientTestCase(base.TestCase): + + def setUp(self): + super(ScalityRemoteFsClientTestCase, self).setUp() + + def test_no_mount_point_scality(self): + self.assertRaises(exception.InvalidParameterValue, + remotefs.ScalityRemoteFsClient, + 'scality', root_helper='true') + + def test_get_mount_point(self): + fsclient = remotefs.ScalityRemoteFsClient( + 'scality', root_helper='true', scality_mount_point_base='/fake') + self.assertEqual('/fake/path/00', fsclient.get_mount_point('path')) + + @mock.patch('oslo_concurrency.processutils.execute') + @mock.patch('os_brick.remotefs.remotefs.RemoteFsClient._do_mount') + def test_mount(self, mock_do_mount, mock_execute): + fsclient = remotefs.ScalityRemoteFsClient( + 'scality', root_helper='true', scality_mount_point_base='/fake') + with mock.patch.object(fsclient, '_read_mounts', return_value={}): + fsclient.mount('fake') + + mock_execute.assert_called_once_with( + 'mkdir', '-p', '/fake', check_exit_code=0) + mock_do_mount.assert_called_once_with( + 'sofs', '/etc/sfused.conf', '/fake')