From cee77d87f32d53981c0763905804567d91a28081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Ribaud?= Date: Mon, 15 May 2023 17:23:52 +0200 Subject: [PATCH] Allow to mount manila share using Cephfs protocol Manila is the OpenStack Shared Filesystems service. These series of patches implement changes required in Nova to allow the shares provided by Manila to be associated with and attached to instances using virtiofs. Implements: blueprint libvirt-virtiofs-attach-manila-shares Change-Id: I8ad03261b65ebe0aeaab100a7c1255ba8a85aa9c --- nova/compute/manager.py | 70 +++++++--- nova/conf/libvirt.py | 29 ++++ nova/exception.py | 4 +- nova/objects/share_mapping.py | 45 +++++- nova/share/manila.py | 16 +++ nova/tests/unit/compute/test_compute_mgr.py | 131 +++++++++++++++++- nova/tests/unit/objects/test_objects.py | 2 +- nova/tests/unit/objects/test_share_mapping.py | 30 +++- nova/tests/unit/virt/libvirt/test_driver.py | 94 ++++++++++--- .../unit/virt/libvirt/volume/test_cephfs.py | 120 ++++++++++++++++ nova/virt/libvirt/driver.py | 21 ++- nova/virt/libvirt/volume/cephfs.py | 52 +++++++ 12 files changed, 559 insertions(+), 55 deletions(-) create mode 100644 nova/tests/unit/virt/libvirt/volume/test_cephfs.py create mode 100644 nova/virt/libvirt/volume/cephfs.py diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9b1a201541..750da82b45 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4637,14 +4637,6 @@ class ComputeManager(manager.Manager): @utils.synchronized(share_mapping.share_id) def _allow_share(context, instance, share_mapping): - def _has_access(): - access = self.manila_api.get_access( - context, - share_mapping.share_id, - access_type, - access_to - ) - return access is not None and access.state == 'active' def _apply_policy(): # self.manila_api.lock(share_mapping.share_id) @@ -4654,8 +4646,8 @@ class ComputeManager(manager.Manager): self.manila_api.allow( context, share_mapping.share_id, - access_type, - access_to, + share_mapping.access_type, + share_mapping.access_to, "rw", ) @@ -4665,7 +4657,12 @@ class ComputeManager(manager.Manager): max_retries = CONF.manila.share_apply_policy_timeout attempt_count = 0 while attempt_count < max_retries: - if _has_access(): + if self.manila_api.has_access( + context, + share_mapping.share_id, + share_mapping.access_type, + share_mapping.access_to, + ): LOG.debug( "Allow policy set on share %s ", share_mapping.share_id, @@ -4687,10 +4684,14 @@ class ComputeManager(manager.Manager): ) try: - access_type = 'ip' - access_to = CONF.my_shared_fs_storage_ip + share_mapping.set_access_according_to_protocol() - if not _has_access(): + if not self.manila_api.has_access( + context, + share_mapping.share_id, + share_mapping.access_type, + share_mapping.access_to, + ): _apply_policy() _wait_policy_to_be_applied() @@ -4701,6 +4702,7 @@ class ComputeManager(manager.Manager): except ( exception.ShareNotFound, + exception.ShareProtocolNotSupported, exception.ShareAccessGrantError, ) as e: self._set_share_mapping_status( @@ -4773,12 +4775,10 @@ class ComputeManager(manager.Manager): ) try: - still_used = check_share_usage( - context, instance.uuid - ) + still_used = check_share_usage(context, instance.uuid) + + share_mapping.set_access_according_to_protocol() - access_type = 'ip' - access_to = CONF.my_shared_fs_storage_ip if not still_used: # self.manila_api.unlock(share_mapping.share_id) # Explicit unlocking the share is not needed as @@ -4787,13 +4787,16 @@ class ComputeManager(manager.Manager): self.manila_api.deny( context, share_mapping.share_id, - access_type, - access_to, + share_mapping.access_type, + share_mapping.access_to, ) share_mapping.delete() - except exception.ShareAccessRemovalError as e: + except ( + exception.ShareAccessRemovalError, + exception.ShareProtocolNotSupported, + ) as e: self._set_share_mapping_status( share_mapping, fields.ShareMappingStatus.ERROR ) @@ -4828,10 +4831,18 @@ class ComputeManager(manager.Manager): @utils.synchronized(share_mapping.share_id) def _mount_share(context, instance, share_mapping): try: + share_mapping.set_access_according_to_protocol() + + if share_mapping.share_proto == ( + fields.ShareMappingProto.CEPHFS): + share_mapping.enhance_with_ceph_credentials(context) + LOG.debug("Mounting share %s", share_mapping.share_id) self.driver.mount_share(context, instance, share_mapping) except ( + exception.ShareNotFound, + exception.ShareProtocolNotSupported, exception.ShareMountError, ) as e: self._set_share_mapping_and_instance_in_error( @@ -4839,6 +4850,13 @@ class ComputeManager(manager.Manager): ) LOG.error(e.format_message()) raise + except (sdk_exc.BadRequestException) as e: + self._set_share_mapping_and_instance_in_error( + instance, share_mapping + ) + LOG.error("%s: %s error from url: %s, %s", e.message, e.source, + e.url, e.details) + raise _mount_share(context, instance, share_mapping) @@ -4848,10 +4866,18 @@ class ComputeManager(manager.Manager): @utils.synchronized(share_mapping.share_id) def _umount_share(context, instance, share_mapping): try: + share_mapping.set_access_according_to_protocol() + + if share_mapping.share_proto == ( + fields.ShareMappingProto.CEPHFS): + share_mapping.enhance_with_ceph_credentials(context) + self.driver.umount_share(context, instance, share_mapping) except ( + exception.ShareNotFound, exception.ShareUmountError, + exception.ShareProtocolNotSupported, ) as e: self._set_share_mapping_and_instance_in_error( instance, share_mapping diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index a380cc1103..4ac488969f 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -1240,6 +1240,34 @@ Possible values: """), ] +libvirt_volume_ceph_opts = [ + cfg.StrOpt('ceph_mount_point_base', + default=paths.state_path_def('mnt'), + help=""" +Directory where the ceph volume for each manila share is mounted on the +compute node. +The default is 'mnt' directory of the location where nova's Python module +is installed. + +Possible values: + +* A string representing absolute path of mount point. +"""), + cfg.ListOpt('ceph_mount_options', + help=""" +Mount options passed to the ceph client. See section of the ceph man page +for details. + +Mount options controls the way the filesystem is mounted and how the +ceph client behaves when accessing files on this mount point. + +Possible values: + +* Any string representing mount options separated by commas. +* Example string: vers=3,lookupcache=pos +"""), +] + libvirt_volume_quobyte_opts = [ cfg.StrOpt('quobyte_mount_point_base', default=paths.state_path_def('mnt'), @@ -1587,6 +1615,7 @@ ALL_OPTS = list(itertools.chain( libvirt_volume_iser_opts, libvirt_volume_net_opts, libvirt_volume_nfs_opts, + libvirt_volume_ceph_opts, libvirt_volume_quobyte_opts, libvirt_volume_smbfs_opts, libvirt_remotefs_opts, diff --git a/nova/exception.py b/nova/exception.py index 330085f5e4..d96be7672e 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -712,8 +712,8 @@ class ShareMappingAlreadyExists(NotFound): msg_fmt = _("Share %(share_id)s already associated to this server.") -class ShareProtocolUnknown(NotFound): - msg_fmt = _("Share protocol %(share_proto)s is unknown.") +class ShareProtocolNotSupported(NotFound): + msg_fmt = _("Share protocol %(share_proto)s is not supported.") class ShareError(NovaException): diff --git a/nova/objects/share_mapping.py b/nova/objects/share_mapping.py index 63f7a855f8..d0d8276094 100644 --- a/nova/objects/share_mapping.py +++ b/nova/objects/share_mapping.py @@ -14,20 +14,31 @@ import logging from oslo_utils import versionutils +import nova.conf from nova.db.main import api as db from nova import exception from nova.objects import base from nova.objects import fields +from nova.share import manila as manila_api +CONF = nova.conf.CONF LOG = logging.getLogger(__name__) +EPHEMERAL_FIELDS = [ + "access_type", + "access_to", + "access_key", +] + @base.NovaObjectRegistry.register class ShareMapping(base.NovaTimestampObject, base.NovaObject): # Version 1.0: Initial version. # Version 1.1: Add "attaching" and "detaching" to possible values # of status field. - VERSION = '1.1' + # Version 1.2: Add ephemeral fields 'access_type', 'access_to', + # 'access_key' to manage CephFS protocol and access. + VERSION = '1.2' fields = { 'id': fields.IntegerField(read_only=True), @@ -37,7 +48,11 @@ class ShareMapping(base.NovaTimestampObject, base.NovaObject): 'status': fields.ShareMappingStatusField(), 'tag': fields.StringField(nullable=False), 'export_location': fields.StringField(nullable=False), - 'share_proto': fields.ShareMappingProtoField() + 'share_proto': fields.ShareMappingProtoField(), + # Next fields are ephemeral + 'access_type': fields.StringField(nullable=True), + 'access_to': fields.StringField(nullable=True), + 'access_key': fields.StringField(nullable=True), } def obj_make_compatible(self, primitive, target_version): @@ -59,7 +74,8 @@ class ShareMapping(base.NovaTimestampObject, base.NovaObject): @staticmethod def _from_db_object(context, share_mapping, db_share_mapping): for field in share_mapping.fields: - setattr(share_mapping, field, db_share_mapping[field]) + if field not in EPHEMERAL_FIELDS: + setattr(share_mapping, field, db_share_mapping[field]) share_mapping._context = context share_mapping.obj_reset_changes() return share_mapping @@ -133,6 +149,29 @@ class ShareMapping(base.NovaTimestampObject, base.NovaObject): raise NotImplementedError() return rhost + def enhance_with_ceph_credentials(self, context): + # Enhance the share_mapping object by adding Ceph + # credential information + access = manila_api.API().get_access( + context, + self.share_id, + self.access_type, + self.access_to + ) + self.access_key = access.access_key + + def set_access_according_to_protocol(self): + if self.share_proto == fields.ShareMappingProto.NFS: + self.access_type = 'ip' + self.access_to = CONF.my_shared_fs_storage_ip + elif self.share_proto == fields.ShareMappingProto.CEPHFS: + self.access_type = 'cephx' + self.access_to = 'nova' + else: + raise exception.ShareProtocolNotSupported( + share_proto=self.share_proto + ) + @base.NovaObjectRegistry.register class ShareMappingList(base.ObjectListBase, base.NovaObject): diff --git a/nova/share/manila.py b/nova/share/manila.py index a2f37fb0f2..ca79105dce 100644 --- a/nova/share/manila.py +++ b/nova/share/manila.py @@ -331,3 +331,19 @@ class API(object): ) else: raise exception.ShareAccessNotFound(share_id=share_id) + + def has_access(self, context, share_id, access_type, access_to): + """Helper method to check if a policy is applied to a share + :param context: The request context. + :param share_id: the id of the share + :param access_type: the type of access ("ip", "cert", "user") + :param access_to: ip:cidr or cert:cn or user:group or user name + :returns: boolean, true means the policy is applied. + """ + access = self.get_access( + context, + share_id, + access_type, + access_to + ) + return access is not None and access.state == 'active' diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index ebe9f1cc10..b41200ddf5 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -139,6 +139,22 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, } return nova.share.manila.Access.from_dict(access) + def get_fake_share_mapping_cephfs(self): + share_mapping = self.get_fake_share_mapping() + share_mapping.share_proto = 'CEPHFS' + return share_mapping + + def get_fake_share_access_cephfs(self): + access = { + "access_level": "rw", + "state": "active", + "id": "507bf114-36f2-4f56-8cf4-857985ca87c1", + "access_type": "cephx", + "access_to": "nova", + "access_key": "mykey", + } + return nova.share.manila.Access.from_dict(access) + def fake_share_info(self): share_mapping = {} share_mapping['id'] = 1 @@ -2610,6 +2626,42 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_allow.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw') + @mock.patch('nova.share.manila.API.allow') + @mock.patch('nova.share.manila.API.get_access') + @mock.patch('nova.objects.share_mapping.ShareMapping.save') + def test_allow_share_fails_protocol_not_supported( + self, mock_db, mock_get_access, mock_allow + ): + self.flags(shutdown_retry_interval=20, group='compute') + instance = fake_instance.fake_instance_obj( + self.context, + uuid=uuids.instance, + vm_state=vm_states.ACTIVE, + task_state=task_states.POWERING_OFF) + mock_get_access.side_effect = [None, self.get_fake_share_access()] + # Ensure CONF.my_shared_fs_storage_ip default is my_ip + self.flags(my_ip="10.0.0.2") + self.assertEqual(CONF.my_shared_fs_storage_ip, '10.0.0.2') + # Set CONF.my_shared_fs_storage_ip to ensure it is used by the code + self.flags(my_shared_fs_storage_ip="192.168.0.1") + compute_ip = CONF.my_shared_fs_storage_ip + self.assertEqual(compute_ip, '192.168.0.1') + share_mapping = self.get_fake_share_mapping() + mock_allow.side_effect = exception.ShareProtocolNotSupported( + share_proto=share_mapping.share_proto + ) + self.assertRaises( + exception.ShareProtocolNotSupported, + self.compute.allow_share, + self.context, + instance, + share_mapping + ) + mock_get_access.assert_called_with( + self.context, share_mapping.share_id, 'ip', compute_ip) + mock_allow.assert_called_once_with( + mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw') + @mock.patch('nova.objects.share_mapping.ShareMapping.delete') @mock.patch('nova.share.manila.API.deny') @mock.patch('nova.share.manila.API.get_access') @@ -2836,6 +2888,43 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_db_delete.assert_not_called() self.assertEqual(share_mapping.status, 'error') + @mock.patch('nova.objects.share_mapping.ShareMapping.save') + @mock.patch('nova.objects.share_mapping.ShareMapping.delete') + @mock.patch('nova.share.manila.API.deny') + @mock.patch('nova.share.manila.API.get_access') + @mock.patch('nova.objects.share_mapping.ShareMappingList.get_by_share_id') + def test_deny_share_fails_protocol_not_supported( + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_db_save + ): + """Ensure we have an exception if the access cannot be removed + by manila. + """ + self.flags(shutdown_retry_interval=20, group='compute') + instance = fake_instance.fake_instance_obj( + self.context, + uuid=uuids.instance, + vm_state=vm_states.ACTIVE, + task_state=task_states.POWERING_OFF) + mock_db_get_share.return_value = ( + objects.share_mapping.ShareMappingList() + ) + share_mapping = self.get_fake_share_mapping() + share_mapping.status = 'detaching' + mock_db_get_share.return_value.objects.append(share_mapping) + mock_deny.side_effect = exception.ShareProtocolNotSupported( + share_proto=share_mapping.share_proto + ) + self.assertRaises( + exception.ShareProtocolNotSupported, + self.compute.deny_share, + self.context, + instance, + share_mapping + ) + mock_db_delete.assert_not_called() + self.assertEqual(share_mapping.status, 'error') + @mock.patch('nova.objects.share_mapping.ShareMapping.delete') @mock.patch('nova.share.manila.API.deny') @mock.patch('nova.share.manila.API.get_access') @@ -2907,9 +2996,32 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute._mount_share(self.context, instance, share_mapping) mock_drv.assert_called_once_with(self.context, instance, share_mapping) + @mock.patch('nova.objects.share_mapping.ShareMapping.save') + @mock.patch('nova.virt.fake.FakeDriver.mount_share') + @mock.patch('nova.share.manila.API.get_access') + def test_mount_cephfs_share( + self, mock_get_access, mock_drv, mock_db + ): + self.flags(shutdown_retry_interval=20, group='compute') + instance = fake_instance.fake_instance_obj( + self.context, + uuid=uuids.instance, + vm_state=vm_states.ACTIVE, + task_state=task_states.POWERING_OFF) + share_mapping = self.get_fake_share_mapping_cephfs() + mock_get_access.side_effect = [ + self.get_fake_share_access_cephfs(), + ] + self.compute._mount_share(self.context, instance, share_mapping) + mock_get_access.assert_called_with( + self.context, share_mapping.share_id, 'cephx', 'nova') + mock_drv.assert_called_once_with(self.context, instance, share_mapping) + self.assertEqual(share_mapping.access_to, 'nova') + self.assertEqual(share_mapping.access_key, 'mykey') + @mock.patch('nova.share.manila.API.deny') @mock.patch('nova.virt.fake.FakeDriver.umount_share', return_value=False) - def test_umount_share( + def test_umount_nfs_share( self, mock_drv, mock_deny): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( @@ -2921,6 +3033,23 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute._umount_share(self.context, instance, share_mapping) mock_drv.assert_called_once_with(self.context, instance, share_mapping) + @mock.patch('nova.share.manila.API.get_access') + @mock.patch('nova.virt.fake.FakeDriver.umount_share', return_value=False) + def test_umount_cephfs_share( + self, mock_drv, mock_get_access): + self.flags(shutdown_retry_interval=20, group='compute') + instance = fake_instance.fake_instance_obj( + self.context, + uuid=uuids.instance, + vm_state=vm_states.ACTIVE, + task_state=task_states.POWERING_OFF) + share_mapping = self.get_fake_share_mapping_cephfs() + mock_get_access.return_value = self.get_fake_share_access_cephfs() + self.compute._umount_share(self.context, instance, share_mapping) + mock_get_access.assert_called_once() + mock_drv.assert_called_once_with( + self.context, instance, share_mapping) + @mock.patch('nova.compute.manager.ComputeManager._umount_share') @mock.patch('nova.compute.manager.ComputeManager.deny_share') @mock.patch('nova.compute.manager.ComputeManager._get_share_info') diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 8a24773c7b..7a12cec521 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1168,7 +1168,7 @@ object_data = { 'Selection': '1.1-548e3c2f04da2a61ceaf9c4e1589f264', 'Service': '1.22-8a740459ab9bf258a19c8fcb875c2d9a', 'ServiceList': '1.19-5325bce13eebcbf22edc9678285270cc', - 'ShareMapping': '1.1-3f1f4e053d37c1ddcdd0040cee569c1f', + 'ShareMapping': '1.2-ae6ba712dc8022d08c4de34fb8b6e015', 'ShareMappingList': '1.0-634980d5efdf3656e28c8dec3d862ab9', 'ShareMetadata': '1.0-09f69ac0bd47371417b5477a277e43af', 'Tag': '1.1-8b8d7d5b48887651a0e01241672e2963', diff --git a/nova/tests/unit/objects/test_share_mapping.py b/nova/tests/unit/objects/test_share_mapping.py index 36d8cd06ea..dc8b27f5d4 100644 --- a/nova/tests/unit/objects/test_share_mapping.py +++ b/nova/tests/unit/objects/test_share_mapping.py @@ -35,6 +35,9 @@ fake_share_mapping = { 'tag': 'fake_tag', 'export_location': '192.168.122.152:/manila/share', 'share_proto': 'NFS', + 'access_to': '', + 'access_type': '', + 'access_key': '', } fake_share_mapping2 = { @@ -48,6 +51,9 @@ fake_share_mapping2 = { 'tag': 'fake_tag2', 'export_location': '192.168.122.152:/manila/share2', 'share_proto': 'NFS', + 'access_to': '', + 'access_type': '', + 'access_key': '', } fake_share_mapping_attached = deepcopy(fake_share_mapping) @@ -59,7 +65,8 @@ class _TestShareMapping(object): self.compare_obj( share_mapping, fake_share_mapping, - allow_missing=['deleted', 'deleted_at']) + allow_missing=["deleted", "deleted_at", "access_type", + "access_to", "access_key"]) def test_obj_make_compatible_attaching_status(self): obj = objects.ShareMapping() @@ -87,6 +94,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_mapping.save() mock_upd.assert_called_once_with( self.context, @@ -109,6 +119,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_host_provider = share_mapping.get_share_host_provider() self.assertEqual(share_host_provider, '192.168.122.152') @@ -121,6 +134,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_host_provider = share_mapping.get_share_host_provider() self.assertIsNone(share_host_provider) @@ -136,6 +152,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_mapping.create() mock_upd.assert_called_once_with( self.context, @@ -161,6 +180,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_mapping.activate() mock_upd.assert_called_once_with( self.context, @@ -187,6 +209,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_mapping.deactivate() mock_upd.assert_called_once_with( self.context, @@ -212,6 +237,9 @@ class _TestShareMapping(object): share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' share_mapping.share_proto = 'NFS' + share_mapping.access_to = '' + share_mapping.access_type = '' + share_mapping.access_key = '' share_mapping.delete() mock_del.assert_called_once_with( self.context, uuids.instance, uuids.share) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 3fe5dae584..aa6b15512d 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -17818,10 +17818,16 @@ class LibvirtConnTestCase(test.NoDBTestCase, external_events=[]) mock_metadata.assert_called_once_with(self.context, instance) - def _mount_or_umount_share(self, func, side_effect=False): + def _mount_or_umount_share(self, protocol, func, side_effect=False): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) class_object = getattr(drvr, func) - base_class = 'nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver' + if protocol == fields.ShareMappingProto.NFS: + base_class = "nova.virt.libvirt.volume.nfs.LibvirtNFSVolumeDriver" + elif protocol == fields.ShareMappingProto.CEPHFS: + base_class = ( + "nova.virt.libvirt.volume.cephfs.LibvirtCEPHFSVolumeDriver" + ) + if func == 'umount_share': mock_class_object = base_class + '.disconnect_volume' exc = exception.ShareUmountError @@ -17838,20 +17844,32 @@ class LibvirtConnTestCase(test.NoDBTestCase, share_mapping.status = 'inactive' share_mapping.tag = 'fake_tag' share_mapping.export_location = '192.168.122.152:/manila/share' - share_mapping.share_proto = 'NFS' + share_mapping.share_proto = protocol + share_mapping.access_to = 'myname' + share_mapping.access_key = 'mysecret' - with mock.patch(mock_class_object) as mock_nfsdrv, mock.patch( + with mock.patch(mock_class_object) as mock_drv, mock.patch( 'nova.objects.share_mapping.ShareMapping.save'): if not side_effect: class_object(self.context, instance, share_mapping) - mock_nfsdrv.assert_called_once_with( - {'data': { - 'export': share_mapping.export_location, - 'name': share_mapping.share_id}, - }, - instance) + if protocol == fields.ShareMappingProto.NFS: + mock_drv.assert_called_once_with( + {'data': { + 'export': share_mapping.export_location, + 'name': share_mapping.share_id}, + }, + instance) + elif protocol == fields.ShareMappingProto.CEPHFS: + mock_drv.assert_called_once_with( + {'data': { + 'export': share_mapping.export_location, + 'name': share_mapping.share_id, + "options": ["name=myname", "secret=mysecret"]}, + }, + instance) + else: - mock_nfsdrv.side_effect = side_effect + mock_drv.side_effect = side_effect self.assertRaises( exc, class_object, @@ -17860,19 +17878,53 @@ class LibvirtConnTestCase(test.NoDBTestCase, share_mapping ) - def test_mount_share(self): - self._mount_or_umount_share('mount_share') - - def test_mount_share_fails(self): + def test_mount_nfs_share(self): self._mount_or_umount_share( - 'mount_share', processutils.ProcessExecutionError) + fields.ShareMappingProto.NFS, "mount_share" + ) - def test_umount_share(self): - self._mount_or_umount_share('umount_share') - - def test_umount_share_fails(self): + def test_mount_nfs_share_fails(self): self._mount_or_umount_share( - 'umount_share', processutils.ProcessExecutionError) + fields.ShareMappingProto.NFS, + "mount_share", + processutils.ProcessExecutionError, + ) + + def test_umount_nfs_share(self): + self._mount_or_umount_share( + fields.ShareMappingProto.NFS, "umount_share" + ) + + def test_umount_nfs_share_fails(self): + self._mount_or_umount_share( + fields.ShareMappingProto.NFS, + "umount_share", + processutils.ProcessExecutionError, + ) + + def test_mount_cephfs_share(self): + self._mount_or_umount_share( + fields.ShareMappingProto.CEPHFS, "mount_share" + ) + + def test_mount_cephfs_share_fails(self): + self._mount_or_umount_share( + fields.ShareMappingProto.CEPHFS, + "mount_share", + processutils.ProcessExecutionError, + ) + + def test_umount_cephfs_share(self): + self._mount_or_umount_share( + fields.ShareMappingProto.CEPHFS, "umount_share" + ) + + def test_umount_cephfs_share_fails(self): + self._mount_or_umount_share( + fields.ShareMappingProto.CEPHFS, + "umount_share", + processutils.ProcessExecutionError, + ) @mock.patch('nova.objects.instance.Instance.save', return_value=None) diff --git a/nova/tests/unit/virt/libvirt/volume/test_cephfs.py b/nova/tests/unit/virt/libvirt/volume/test_cephfs.py new file mode 100644 index 0000000000..e03ce9bf62 --- /dev/null +++ b/nova/tests/unit/virt/libvirt/volume/test_cephfs.py @@ -0,0 +1,120 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os +from unittest import mock + +from oslo_utils.fixture import uuidsentinel as uuids + +from nova.tests.unit.virt.libvirt.volume import test_mount +from nova.tests.unit.virt.libvirt.volume import test_volume +from nova import utils +from nova.virt.libvirt.volume import cephfs +from nova.virt.libvirt.volume import mount + + +class LibvirtCEPHFSVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): + """Tests the libvirt CEPHFS volume driver.""" + + def setUp(self): + super(LibvirtCEPHFSVolumeDriverTestCase, self).setUp() + + self.useFixture(test_mount.MountFixture(self)) + + m = mount.get_manager() + m._reset_state() + + self.mnt_base = '/mnt' + m.host_up(self.fake_host) + self.flags(ceph_mount_point_base=self.mnt_base, group='libvirt') + + def test_libvirt_ceph_driver(self): + libvirt_driver = cephfs.LibvirtCEPHFSVolumeDriver(self.fake_host) + + export_string = '192.168.1.1:/ceph/share1' + export_mnt_base = os.path.join(self.mnt_base, + utils.get_hash_str(export_string)) + + connection_info = { + "data": { + "export": export_string, + "name": self.name, + "options": ["name=myname", "secret=mysecret"], + } + } + instance = mock.sentinel.instance + instance.uuid = uuids.instance + libvirt_driver.connect_volume(connection_info, instance) + libvirt_driver.disconnect_volume(connection_info, + mock.sentinel.instance) + + device_path = os.path.join(export_mnt_base, + connection_info['data']['name']) + self.assertEqual(connection_info['data']['device_path'], device_path) + self.mock_ensure_tree.assert_has_calls([mock.call(export_mnt_base)]) + self.mock_mount.assert_has_calls( + [ + mock.call( + "ceph", + export_string, + export_mnt_base, + ["-o", "name=myname,secret=mysecret"], + ) + ] + ) + self.mock_umount.assert_has_calls([mock.call(export_mnt_base)]) + self.mock_rmdir.assert_has_calls([mock.call(export_mnt_base)]) + + def test_libvirt_ceph_driver_with_default_options(self): + libvirt_driver = cephfs.LibvirtCEPHFSVolumeDriver(self.fake_host) + self.flags( + ceph_mount_options=["default_opt1=true", "default_opt2=true"], + group="libvirt", + ) + + export_string = '192.168.1.1:/ceph/share1' + export_mnt_base = os.path.join(self.mnt_base, + utils.get_hash_str(export_string)) + + connection_info = { + "data": { + "export": export_string, + "name": self.name, + "options": ["name=myname", "secret=mysecret"], + } + } + instance = mock.sentinel.instance + instance.uuid = uuids.instance + libvirt_driver.connect_volume(connection_info, instance) + libvirt_driver.disconnect_volume(connection_info, + mock.sentinel.instance) + + device_path = os.path.join(export_mnt_base, + connection_info['data']['name']) + self.assertEqual(connection_info['data']['device_path'], device_path) + self.mock_ensure_tree.assert_has_calls([mock.call(export_mnt_base)]) + self.mock_mount.assert_has_calls( + [ + mock.call( + "ceph", + export_string, + export_mnt_base, + [ + "-o", + "default_opt1=true,default_opt2=true,name=myname," + "secret=mysecret", + ], + ) + ] + ) + self.mock_umount.assert_has_calls([mock.call(export_mnt_base)]) + self.mock_rmdir.assert_has_calls([mock.call(export_mnt_base)]) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 2ab419cdbe..74472b247a 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -127,6 +127,7 @@ from nova.virt.libvirt.storage import dmcrypt from nova.virt.libvirt.storage import lvm from nova.virt.libvirt import utils as libvirt_utils from nova.virt.libvirt import vif as libvirt_vif +from nova.virt.libvirt.volume import cephfs from nova.virt.libvirt.volume import fs from nova.virt.libvirt.volume import mount from nova.virt.libvirt.volume import nfs @@ -4319,13 +4320,25 @@ class LibvirtDriver(driver.ComputeDriver): if protocol == fields.ShareMappingProto.NFS: return nfs.LibvirtNFSVolumeDriver(host) elif protocol == fields.ShareMappingProto.CEPHFS: - raise NotImplementedError() + return cephfs.LibvirtCEPHFSVolumeDriver(host) else: - raise exception.ShareProtocolUnknown(share_proto=protocol) + raise exception.ShareProtocolNotSupported(share_proto=protocol) def _get_share_connection_info(self, share_mapping): - connection_info = {'data': {'export': share_mapping.export_location, - 'name': share_mapping.share_id}} + connection_info = { + "data": { + "export": share_mapping.export_location, + "name": share_mapping.share_id, + } + } + if share_mapping.share_proto == fields.ShareMappingProto.CEPHFS: + if ( + "access_to" in share_mapping and + share_mapping.access_to is not None + ): + name_opt = "name=" + share_mapping.access_to + secret_opt = "secret=" + share_mapping.access_key + connection_info["data"]["options"] = [name_opt, secret_opt] return connection_info def _get_share_mount_path(self, instance, share_mapping): diff --git a/nova/virt/libvirt/volume/cephfs.py b/nova/virt/libvirt/volume/cephfs.py new file mode 100644 index 0000000000..12aa4f25c3 --- /dev/null +++ b/nova/virt/libvirt/volume/cephfs.py @@ -0,0 +1,52 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +# The driver is not designed to function as a volume for libvirt. Rather, +# its purpose is to facilitate the mounting of cephfs shares exposed by manila +# on compute, enabling the shares to be accessed and shared using virtiofs. +# This is the reason why get_config and extend_volume are not implemented. + +import nova.conf +from nova.virt.libvirt.volume import fs + +CONF = nova.conf.CONF + + +class LibvirtCEPHFSVolumeDriver(fs.LibvirtMountedFileSystemVolumeDriver): + """Class implements libvirt part of volume driver for CEPHFS.""" + + def __init__(self, connection): + super(LibvirtCEPHFSVolumeDriver, self).__init__(connection, 'ceph') + + def _get_mount_point_base(self): + return CONF.libvirt.ceph_mount_point_base + + def get_config(self, connection_info, disk_info): + raise NotImplementedError() + + def _mount_options(self, connection_info): + options = [] + conn_options = connection_info['data'].get('options') + if CONF.libvirt.ceph_mount_options is not None: + options.extend(CONF.libvirt.ceph_mount_options) + if conn_options: + options.extend(conn_options) + options = [','.join(options)] + options.insert(0, '-o') + else: + if conn_options: + options.extend(['-o', ','.join(conn_options)]) + + return options + + def extend_volume(self, connection_info, instance, requested_size): + raise NotImplementedError()