diff --git a/doc/notification_samples/instance-share_detach-end.json b/doc/notification_samples/instance-share_detach-end.json new file mode 100644 index 000000000000..c92411d32336 --- /dev/null +++ b/doc/notification_samples/instance-share_detach-end.json @@ -0,0 +1,8 @@ +{ + "event_type": "instance.share_detach.end", + "payload": { + "$ref": "common_payloads/InstanceActionSharePayload.json#" + }, + "priority": "INFO", + "publisher_id": "nova-api:compute" +} diff --git a/doc/notification_samples/instance-share_detach-start.json b/doc/notification_samples/instance-share_detach-start.json new file mode 100644 index 000000000000..04576c082932 --- /dev/null +++ b/doc/notification_samples/instance-share_detach-start.json @@ -0,0 +1,8 @@ +{ + "event_type": "instance.share_detach.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 66bf9e35c973..b1344eaba6eb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4831,8 +4831,26 @@ class ComputeManager(manager.Manager): # remove from manila, so we can still detach the share. share_mapping.delete() + compute_utils.notify_about_share_attach_detach( + context, + instance, + instance.host, + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping.share_id + ) + _deny_share(context, instance, share_mapping) + compute_utils.notify_about_share_attach_detach( + context, + instance, + instance.host, + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.END, + share_id=share_mapping.share_id + ) + @wrap_exception() def _mount_all_shares(self, context, instance, share_info): for share_mapping in share_info: diff --git a/nova/notifications/objects/instance.py b/nova/notifications/objects/instance.py index 36f28c2c8499..1f8370b4d31e 100644 --- a/nova/notifications/objects/instance.py +++ b/nova/notifications/objects/instance.py @@ -626,6 +626,8 @@ class InstanceActionVolumeNotification(base.NotificationBase): @base.notification_sample('instance-share_attach-start.json') @base.notification_sample('instance-share_attach-end.json') +@base.notification_sample('instance-share_detach-start.json') +@base.notification_sample('instance-share_detach-end.json') @nova_base.NovaObjectRegistry.register_notification class InstanceActionShareNotification(base.NotificationBase): # Version 1.0: Initial version diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index bdaab010eba6..7852b4455f8c 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -390,6 +390,7 @@ class TestInstanceNotificationSample( self._test_lock_unlock_instance, self._test_lock_unlock_instance_with_reason, self._test_share_attach, + self._test_share_detach, ] for action in actions: @@ -1732,6 +1733,38 @@ class TestInstanceNotificationSample( self.api.post_server_action(server['id'], {'os-start': {}}) self._wait_for_state_change(server, expected_status='ACTIVE') + def _test_share_detach(self, server): + self.api.post_server_action(server['id'], {'os-stop': {}}) + self._wait_for_state_change(server, expected_status='SHUTOFF') + self.notifier.reset() + + # Detach share + self._detach_share(server, 'e8debdc0-447a-4376-a10a-4cd9122d7986') + + self.assertEqual(2, len(self.notifier.versioned_notifications), + self.notifier.versioned_notifications) + self._verify_notification( + 'instance-share_detach-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_detach-end', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id'], + 'state': 'stopped', + 'power_state': 'shutdown', + }, + actual=self.notifier.versioned_notifications[1]) + + # Restart 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/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 8b70b71e2aa6..46a3d7a5d715 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2704,12 +2704,17 @@ 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.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( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we can deny the instance share. """ @@ -2735,13 +2740,36 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_deny.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip) mock_db_delete.assert_called_once() + mock_notifications.assert_has_calls([ + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping.share_id + ), + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + 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.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_in_use( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we cannot deny a share used by an instance. """ @@ -2760,13 +2788,36 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.deny_share(self.context, instance, share_mapping) mock_deny.assert_not_called() mock_db_delete.assert_called_once() + mock_notifications.assert_has_calls([ + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping.share_id + ), + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + 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.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_in_error( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we can deny a share in error on the instance detaching the share. @@ -2794,13 +2845,36 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_deny.assert_called_once_with( mock.ANY, share_mapping.share_id, 'ip', compute_ip) mock_db_delete.assert_called_once() + mock_notifications.assert_has_calls([ + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping.share_id + ), + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + 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.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_access_not_found_in_manila( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we can deny a share even if access is not found in manila. """ @@ -2825,12 +2899,17 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock.ANY, share_mapping.share_id, 'ip', compute_ip) mock_db_delete.assert_called_once() + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_not_found_in_manila( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we can deny a share even if the share is not found in manila. """ @@ -2855,6 +2934,10 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock.ANY, share_mapping.share_id, 'ip', compute_ip) mock_db_delete.assert_called_once() + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @mock.patch('nova.objects.share_mapping.ShareMapping.save') @mock.patch('nova.objects.share_mapping.ShareMapping.delete') @mock.patch('nova.share.manila.API.deny') @@ -2862,7 +2945,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @mock.patch('nova.objects.share_mapping.ShareMappingList.get_by_share_id') def test_deny_share_fails_access_removal_error( self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, - mock_db_save + mock_db_save, mock_notifications ): """Ensure we have an exception if the access cannot be removed by manila. @@ -2893,6 +2976,10 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_db_delete.assert_not_called() self.assertEqual(share_mapping.status, 'error') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @mock.patch('nova.objects.share_mapping.ShareMapping.save') @mock.patch('nova.objects.share_mapping.ShareMapping.delete') @mock.patch('nova.share.manila.API.deny') @@ -2900,7 +2987,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @mock.patch('nova.objects.share_mapping.ShareMappingList.get_by_share_id') def test_deny_share_fails_keystone_unauthorized( self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, - mock_db_save + mock_db_save, mock_notifications ): """Ensure we have an exception if the access cannot be removed by manila. @@ -2930,6 +3017,10 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_db_delete.assert_not_called() self.assertEqual(share_mapping.status, 'error') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @mock.patch('nova.objects.share_mapping.ShareMapping.save') @mock.patch('nova.objects.share_mapping.ShareMapping.delete') @mock.patch('nova.share.manila.API.deny') @@ -2937,7 +3028,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @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 + mock_db_save, mock_notifications ): """Ensure we have an exception if the access cannot be removed by manila. @@ -2967,12 +3058,17 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_db_delete.assert_not_called() self.assertEqual(share_mapping.status, 'error') + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_in_use_by_another_instance( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we do not deny a share used by another instance. """ @@ -2993,13 +3089,36 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.deny_share(self.context, instance, share_mapping1) mock_deny.assert_not_called() mock_db_delete.assert_called_once() + mock_notifications.assert_has_calls([ + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping1.share_id + ), + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.END, + share_id=share_mapping1.share_id + ), + ]) + @mock.patch( + 'nova.compute.utils.notify_about_share_attach_detach', + return_value=None + ) @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_in_error_on_another_instance( - self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete + self, mock_db_get_share, mock_get_access, mock_deny, mock_db_delete, + mock_notifications ): """Ensure we cannot deny a share in error state on another instance. If the other instance is hard rebooted, it might need the share. @@ -3022,6 +3141,24 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.deny_share(self.context, instance, share_mapping1) mock_deny.assert_not_called() mock_db_delete.assert_called_once() + mock_notifications.assert_has_calls([ + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.START, + share_id=share_mapping1.share_id + ), + mock.call( + mock.ANY, + instance, + "fake-host", + action=fields.NotificationAction.SHARE_DETACH, + phase=fields.NotificationPhase.END, + share_id=share_mapping1.share_id + ), + ]) @mock.patch('nova.objects.share_mapping.ShareMapping.save') @mock.patch('nova.virt.fake.FakeDriver.mount_share') diff --git a/releasenotes/notes/share-notifications-e9f096aa2a302c57.yaml b/releasenotes/notes/share-notifications-e9f096aa2a302c57.yaml new file mode 100644 index 000000000000..575436c24dc0 --- /dev/null +++ b/releasenotes/notes/share-notifications-e9f096aa2a302c57.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + The following share attach and share detach versioned notifications have + been added to the nova-compute service: + * instance.share_attach.start + * instance.share_attach.end + * instance.share_detach.start + * instance.share_detach.end