From 320dca539147a607b126eeb37192603d279918df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Ribaud?= Date: Fri, 8 Jul 2022 10:46:35 +0200 Subject: [PATCH] Add instance.share_attach notification This patch add a notification when a share is attached to an instance. 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: I6fe3807164bd2ca6439342abd32f8e4ce9057c8c --- .../InstanceActionSharePayload.json | 9 ++++ .../instance-share_attach-end.json | 8 +++ .../instance-share_attach-start.json | 8 +++ nova/compute/manager.py | 18 +++++++ nova/compute/utils.py | 31 +++++++++++ nova/notifications/objects/base.py | 4 +- nova/notifications/objects/instance.py | 28 ++++++++++ nova/objects/fields.py | 8 +-- .../test_instance.py | 32 +++++++++++ .../openstack/compute/test_server_shares.py | 21 ++++---- nova/tests/unit/compute/test_compute_mgr.py | 54 ++++++++++++++++--- .../objects/test_notification.py | 4 +- 12 files changed, 203 insertions(+), 22 deletions(-) create mode 100644 doc/notification_samples/common_payloads/InstanceActionSharePayload.json create mode 100644 doc/notification_samples/instance-share_attach-end.json create mode 100644 doc/notification_samples/instance-share_attach-start.json diff --git a/doc/notification_samples/common_payloads/InstanceActionSharePayload.json b/doc/notification_samples/common_payloads/InstanceActionSharePayload.json new file mode 100644 index 0000000000..63bea1eb48 --- /dev/null +++ b/doc/notification_samples/common_payloads/InstanceActionSharePayload.json @@ -0,0 +1,9 @@ +{ + "$ref": "InstanceActionPayload.json", + "nova_object.data":{ + "share_id": "e8debdc0-447a-4376-a10a-4cd9122d7986" + }, + "nova_object.name": "InstanceActionSharePayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.0" +} diff --git a/doc/notification_samples/instance-share_attach-end.json b/doc/notification_samples/instance-share_attach-end.json new file mode 100644 index 0000000000..0e0bfd8c8d --- /dev/null +++ b/doc/notification_samples/instance-share_attach-end.json @@ -0,0 +1,8 @@ +{ + "event_type": "instance.share_attach.end", + "payload": { + "$ref": "common_payloads/InstanceActionSharePayload.json#" + }, + "priority": "INFO", + "publisher_id": "nova-api:compute" +} diff --git a/doc/notification_samples/instance-share_attach-start.json b/doc/notification_samples/instance-share_attach-start.json new file mode 100644 index 0000000000..e3358e9526 --- /dev/null +++ b/doc/notification_samples/instance-share_attach-start.json @@ -0,0 +1,8 @@ +{ + "event_type": "instance.share_attach.start", + "payload": { + "$ref": "common_payloads/InstanceActionSharePayload.json#" + }, + "priority": "INFO", + "publisher_id": "nova-api:compute" +} diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 750da82b45..66bf9e35c9 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4731,8 +4731,26 @@ class ComputeManager(manager.Manager): LOG.error(e) raise + compute_utils.notify_about_share_attach_detach( + context, + instance, + instance.host, + action=fields.NotificationAction.SHARE_ATTACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping.share_id + ) + _allow_share(context, instance, share_mapping) + compute_utils.notify_about_share_attach_detach( + context, + instance, + instance.host, + action=fields.NotificationAction.SHARE_ATTACH, + phase=fields.NotificationPhase.END, + share_id=share_mapping.share_id + ) + @messaging.expected_exceptions(NotImplementedError) @wrap_exception() def deny_share(self, context, instance, share_mapping): diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 1faeb71489..982101344e 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -585,6 +585,37 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase, notification.emit(context) +@rpc.if_notifications_enabled +def notify_about_share_attach_detach(context, instance, host, action, phase, + share_id=None, exception=None): + """Send versioned notification about the action made on the instance + :param instance: the instance which the action performed on + :param host: the host emitting the notification + :param action: the name of the action + :param phase: the phase of the action + :param share_info: share related information + :param exception: the thrown exception (used in error notifications) + """ + fault, priority = _get_fault_and_priority_from_exception(exception) + payload = instance_notification.InstanceActionSharePayload( + context=context, + instance=instance, + fault=fault, + share_id=share_id + ) + notification = instance_notification.InstanceActionShareNotification( + context=context, + priority=priority, + publisher=notification_base.NotificationPublisher( + host=host, source=fields.NotificationSource.API), + event_type=notification_base.EventType( + object='instance', + action=action, + phase=phase), + payload=payload) + notification.emit(context) + + @rpc.if_notifications_enabled def notify_about_instance_rescue_action(context, instance, host, rescue_image_ref, phase=None, diff --git a/nova/notifications/objects/base.py b/nova/notifications/objects/base.py index 5fc3ec2c5f..3ba84a60cc 100644 --- a/nova/notifications/objects/base.py +++ b/nova/notifications/objects/base.py @@ -73,7 +73,9 @@ class EventType(NotificationObject): # enum # Version 1.20: IMAGE_CACHE is added to the NotificationActionField enum # Version 1.21: PROGRESS added to NotificationPhase enum - VERSION = '1.21' + # Version 1.22: SHARE_ATTACH SHARE_DETACH are added to the + # NotificationActionField enum + VERSION = '1.22' fields = { 'object': fields.StringField(nullable=False), diff --git a/nova/notifications/objects/instance.py b/nova/notifications/objects/instance.py index d1c47c5e35..36f28c2c84 100644 --- a/nova/notifications/objects/instance.py +++ b/nova/notifications/objects/instance.py @@ -187,6 +187,22 @@ class InstanceActionVolumePayload(InstanceActionPayload): self.volume_id = volume_id +@nova_base.NovaObjectRegistry.register_notification +class InstanceActionSharePayload(InstanceActionPayload): + # Version 1.0: Initial version + VERSION = '1.0' + fields = { + 'share_id': fields.UUIDField(), + } + + def __init__(self, context, instance, fault, share_id): + super(InstanceActionSharePayload, self).__init__( + context=context, + instance=instance, + fault=fault) + self.share_id = share_id + + @nova_base.NovaObjectRegistry.register_notification class InstanceActionVolumeSwapPayload(InstanceActionPayload): # No SCHEMA as all the additional fields are calculated @@ -608,6 +624,18 @@ class InstanceActionVolumeNotification(base.NotificationBase): } +@base.notification_sample('instance-share_attach-start.json') +@base.notification_sample('instance-share_attach-end.json') +@nova_base.NovaObjectRegistry.register_notification +class InstanceActionShareNotification(base.NotificationBase): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'payload': fields.ObjectField('InstanceActionSharePayload') + } + + @base.notification_sample('instance-create-start.json') @base.notification_sample('instance-create-end.json') @base.notification_sample('instance-create-error.json') diff --git a/nova/objects/fields.py b/nova/objects/fields.py index b476f40209..d64234f00c 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -937,6 +937,8 @@ class NotificationAction(BaseNovaEnum): RESCUE = 'rescue' VOLUME_ATTACH = 'volume_attach' VOLUME_DETACH = 'volume_detach' + SHARE_ATTACH = 'share_attach' + SHARE_DETACH = 'share_detach' CREATE = 'create' IMPORT = 'import' EVACUATE = 'evacuate' @@ -977,9 +979,9 @@ class NotificationAction(BaseNovaEnum): ALL = (UPDATE, EXCEPTION, DELETE, PAUSE, UNPAUSE, RESIZE, VOLUME_SWAP, SUSPEND, POWER_ON, REBOOT, SHUTDOWN, SNAPSHOT, INTERFACE_ATTACH, POWER_OFF, SHELVE, RESUME, RESTORE, EXISTS, RESCUE, VOLUME_ATTACH, - VOLUME_DETACH, CREATE, IMPORT, EVACUATE, RESIZE_FINISH, - LIVE_MIGRATION_ABORT, LIVE_MIGRATION_POST_DEST, LIVE_MIGRATION_POST, - LIVE_MIGRATION_PRE, LIVE_MIGRATION_ROLLBACK, + VOLUME_DETACH, SHARE_ATTACH, SHARE_DETACH, CREATE, IMPORT, EVACUATE, + RESIZE_FINISH, LIVE_MIGRATION_ABORT, LIVE_MIGRATION_POST_DEST, + LIVE_MIGRATION_POST, LIVE_MIGRATION_PRE, LIVE_MIGRATION_ROLLBACK, LIVE_MIGRATION_ROLLBACK_DEST, REBUILD, INTERFACE_DETACH, RESIZE_CONFIRM, RESIZE_PREP, RESIZE_REVERT, SHELVE_OFFLOAD, SOFT_DELETE, TRIGGER_CRASH_DUMP, UNRESCUE, UNSHELVE, ADD_HOST, diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index bf04c2b4b1..bdaab010eb 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -342,6 +342,7 @@ class TestInstanceNotificationSample( self.useFixture(self.neutron) self.cinder = fixtures.CinderFixture(self) self.useFixture(self.cinder) + self.useFixture(fixtures.ManilaFixture()) def _wait_until_swap_volume(self, server, volume_id): for i in range(50): @@ -388,6 +389,7 @@ class TestInstanceNotificationSample( self._test_interface_attach_error, self._test_lock_unlock_instance, self._test_lock_unlock_instance_with_reason, + self._test_share_attach, ] for action in actions: @@ -1700,6 +1702,36 @@ class TestInstanceNotificationSample( 'uuid': server['id']}, actual=self.notifier.versioned_notifications[1]) + def _test_share_attach(self, server): + self.api.post_server_action(server['id'], {'os-stop': {}}) + self._wait_for_state_change(server, expected_status='SHUTOFF') + self.notifier.reset() + + self._attach_share(server, "e8debdc0-447a-4376-a10a-4cd9122d7986") + + self.assertEqual(2, len(self.notifier.versioned_notifications), + self.notifier.versioned_notifications) + self._verify_notification( + 'instance-share_attach-start', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id'], + 'state': 'stopped', + 'power_state': 'shutdown'}, + actual=self.notifier.versioned_notifications[0]) + self._verify_notification( + 'instance-share_attach-end', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id'], + 'state': 'stopped', + 'power_state': 'shutdown'}, + actual=self.notifier.versioned_notifications[1]) + + # Start server + self.api.post_server_action(server['id'], {'os-start': {}}) + self._wait_for_state_change(server, expected_status='ACTIVE') + def _test_rescue_unrescue_server(self, server): # Both "rescue" and "unrescue" notification asserts are made here # rescue notification asserts diff --git a/nova/tests/unit/api/openstack/compute/test_server_shares.py b/nova/tests/unit/api/openstack/compute/test_server_shares.py index ab154d41b0..a7327b9d8e 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_shares.py +++ b/nova/tests/unit/api/openstack/compute/test_server_shares.py @@ -142,10 +142,13 @@ class ServerSharesTest(BaseTestCase): mock_db_get_shares.assert_called_once_with(mock.ANY, instance.uuid) self.assertEqual(output, fake_shares) - @mock.patch('nova.compute.api.API.allow_share') @mock.patch( 'nova.virt.hardware.check_shares_supported', return_value=None ) + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @mock.patch( 'nova.db.main.api.share_mapping_get_by_instance_uuid_and_share_id' ) @@ -156,8 +159,8 @@ class ServerSharesTest(BaseTestCase): mock_get_instance, mock_db_update_share, mock_db_get_share, + mock_notifications, mock_shares_support, - mock_allow ): instance = self.fake_get_instance() @@ -187,15 +190,6 @@ class ServerSharesTest(BaseTestCase): mock_db_get_share.side_effect = [None, fake_db_share] self.controller.create(self.req, instance.uuid, body=body) - mock_allow.assert_called_once() - self.assertIsInstance( - mock_allow.call_args.args[1], objects.instance.Instance) - self.assertEqual(mock_allow.call_args.args[1].uuid, instance.uuid) - self.assertIsInstance( - mock_allow.call_args.args[2], objects.share_mapping.ShareMapping) - self.assertEqual( - mock_allow.call_args.args[2].share_id, fake_db_share['share_id']) - mock_db_update_share.assert_called_once_with( mock.ANY, mock.ANY, @@ -366,6 +360,10 @@ class ServerSharesTest(BaseTestCase): @mock.patch( 'nova.virt.hardware.check_shares_supported', return_value=None ) + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @mock.patch('nova.db.main.api.' 'share_mapping_delete_by_instance_uuid_and_share_id') @mock.patch('nova.db.main.api.' @@ -376,6 +374,7 @@ class ServerSharesTest(BaseTestCase): mock_get_instance, mock_db_get_shares, mock_db_delete_share, + mock_notifications, mock_shares_support, mock_deny ): diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index b41200ddf5..8b70b71e2a 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2456,11 +2456,15 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, "detaching", ) + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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( - self, mock_db, mock_get_access, mock_allow + self, mock_db, mock_get_access, mock_allow, mock_notifications ): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( @@ -2482,12 +2486,34 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, 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_notifications.assert_has_calls([ + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_ATTACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping.share_id + ), + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_ATTACH, + phase=fields.NotificationPhase.END, + share_id=share_mapping.share_id + ), + ]) + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_share_not_found( - self, mock_db, mock_get_access, mock_allow + self, mock_db, mock_get_access, mock_allow, mock_notifications ): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( @@ -2519,11 +2545,15 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_allow.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_share_access_grant_error( - self, mock_db, mock_get_access, mock_allow + self, mock_db, mock_get_access, mock_allow, mock_notifications ): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( @@ -2556,11 +2586,15 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_allow.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_bad_request_exception( - self, mock_db, mock_get_access, mock_allow + self, mock_db, mock_get_access, mock_allow, mock_notifications ): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( @@ -2590,11 +2624,15 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_allow.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_keystone_exception( - self, mock_db, mock_get_access, mock_allow + self, mock_db, mock_get_access, mock_allow, mock_notifications ): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( @@ -2626,11 +2664,15 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_allow.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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, mock_db, mock_get_access, mock_allow, mock_notifications ): self.flags(shutdown_retry_interval=20, group='compute') instance = fake_instance.fake_instance_obj( diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index 521fcbe2b9..01eada6656 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -375,7 +375,7 @@ notification_object_data = { 'ComputeTaskNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'ComputeTaskPayload': '1.0-59a9b78b6199470a83c8d07bffd13f5e', 'DestinationPayload': '1.0-d8faf610201bf5f460892243f6632a37', - 'EventType': '1.21-6a5f57fafe478f354f66b81b4cb537ea', + 'EventType': '1.22-fab37f98c6e7513edece32ad5420f549', 'ExceptionNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'ExceptionPayload': '1.1-34f006107693b8c9eaf4b104157d21b4', 'FlavorNotification': '1.0-a73147b93b520ff0061865849d3dfa56', @@ -396,6 +396,8 @@ notification_object_data = { 'InstanceActionResizePrepNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'InstanceActionResizePrepPayload': '1.3-8969438a48c496569c2add19b170dca1', + 'InstanceActionShareNotification': '1.0-a73147b93b520ff0061865849d3dfa56', + 'InstanceActionSharePayload': '1.0-024003b187136ba4db3b958e8d22d540', 'InstanceActionVolumeNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'InstanceActionVolumePayload': '1.6-b5fd2b23dbafe33b72cdbbb7b937bf18', 'InstanceActionVolumeSwapNotification':