openstack/nova
Code Issues Proposed changes

Add instance.share_detach notification

Browse Source 
This patch add a notification when a share is detached from 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: I203b71a39878c3929b22b53220f58c12c0a4c0b9
This commit is contained in:
René Ribaud 2022-07-22 16:32:37 +02:00
parent 320dca5391
commit 13d45b6d37
7 changed files with 225 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/notification_samples/instance-share_detach-end.json Normal file
View File

@ -0,0 +1,8 @@
{
"event_type": "instance.share_detach.end",
"payload": {
"$ref": "common_payloads/InstanceActionSharePayload.json#"
},
"priority": "INFO",
"publisher_id": "nova-api:compute"
}

8
doc/notification_samples/instance-share_detach-start.json Normal file
View File

@ -0,0 +1,8 @@
{
"event_type": "instance.share_detach.start",
"payload": {
"$ref": "common_payloads/InstanceActionSharePayload.json#"
},
"priority": "INFO",
"publisher_id": "nova-api:compute"
}

18
nova/compute/manager.py
View File

@ -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:

2
nova/notifications/objects/instance.py
View File

@ -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

33
nova/tests/functional/notification_sample_tests/test_instance.py
View File

@ -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

157
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -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')

9
releasenotes/notes/share-notifications-e9f096aa2a302c57.yaml Normal file
View File

@ -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