openstack/nova
Code Issues Proposed changes

Merge "Add instance.share_detach_error notification"

Browse Source
This commit is contained in:
Zuul 2024-12-12 21:36:13 +00:00 committed by Gerrit Code Review
commit 728337f200
6 changed files with 343 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
doc/notification_samples/instance-share_detach-error.json Normal file
View File

@ -0,0 +1,22 @@
{
"event_type": "instance.share_detach.error",
"payload": {
"$ref": "common_payloads/InstanceActionSharePayload.json#",
"nova_object.data": {
"fault": {
"nova_object.data": {
"exception": "ShareAccessRemovalError",
"exception_message": "Share access could not be removed from share id 8db0037b-e98f-4bde-ae71-f96a077c19a4.\nReason: Connection timed out.",
"function_name": "_execute_mock_call",
"module_name": "unittest.mock",
"traceback": "Traceback (most recent call last):\n File \"nova/compute/manager.py\", line ..."
},
"nova_object.name": "ExceptionPayload",
"nova_object.namespace": "nova",
"nova_object.version": "1.1"
}
}
},
"priority": "ERROR",
"publisher_id": "nova-api:compute"
}

63
nova/compute/manager.py
View File

@ -4783,6 +4783,7 @@ class ComputeManager(manager.Manager):
@utils.synchronized(share_mapping.share_id)
def _deny_share(context, instance, share_mapping):
def check_share_usage(context, instance_uuid):
share_mappings_used_by_share = (
objects.share_mapping.ShareMappingList.get_by_share_id(
@ -4819,6 +4820,15 @@ class ComputeManager(manager.Manager):
)
try:
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,
)
still_used = check_share_usage(context, instance.uuid)
share_mapping.set_access_according_to_protocol()
@ -4837,6 +4847,15 @@ class ComputeManager(manager.Manager):
share_mapping.delete()
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,
)
except (
exception.ShareAccessRemovalError,
exception.ShareProtocolNotSupported,
@ -4844,39 +4863,47 @@ class ComputeManager(manager.Manager):
self._set_share_mapping_status(
share_mapping, fields.ShareMappingStatus.ERROR
)
compute_utils.notify_about_share_attach_detach(
context,
instance,
instance.host,
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.ERROR,
share_id=share_mapping.share_id,
exception=e
)
LOG.error(e.format_message())
raise
except keystone_exception.http.Unauthorized as e:
self._set_share_mapping_status(
share_mapping, fields.ShareMappingStatus.ERROR
)
compute_utils.notify_about_share_attach_detach(
context,
instance,
instance.host,
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.ERROR,
share_id=share_mapping.share_id,
exception=e
)
LOG.error(e)
raise
except (exception.ShareNotFound, exception.ShareAccessNotFound):
# Ignore the error if for any reason there is nothing to
# 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
)
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,
)
_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:

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

@ -686,6 +686,7 @@ class InstanceActionVolumeNotification(base.NotificationBase):
@base.notification_sample('instance-share_attach-error.json')
@base.notification_sample('instance-share_attach-end.json')
@base.notification_sample('instance-share_detach-start.json')
@base.notification_sample('instance-share_detach-error.json')
@base.notification_sample('instance-share_detach-end.json')
@nova_base.NovaObjectRegistry.register_notification
class InstanceActionShareNotification(base.NotificationBase):

5
nova/tests/functional/integrated_helpers.py
View File

@ -576,6 +576,11 @@ class InstanceHelperMixin:
self.notifier.wait_for_versioned_notifications(
'instance.share_detach.end')
def _detach_share_with_error(self, server, share_id):
self.api.delete_server_share(server['id'], share_id)
self.notifier.wait_for_versioned_notifications(
'instance.share_detach.error')
def _rebuild_server(self, server, image_uuid, expected_state='ACTIVE'):
"""Rebuild a server."""
self.api.post_server_action(

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

@ -393,7 +393,7 @@ class TestInstanceNotificationSample(
self._test_lock_unlock_instance,
self._test_lock_unlock_instance_with_reason,
self._test_share_attach_detach,
self._test_share_attach_error,
self._test_share_attach_detach_error,
]
for action in actions:
@ -1844,7 +1844,7 @@ class TestInstanceNotificationSample(
},
actual=self.notifier.versioned_notifications[1])
# Restart server
# Start server
self.notifier.reset()
self.api.post_server_action(server['id'], {'os-start': {}})
self._wait_for_state_change(server, expected_status='ACTIVE')
@ -1869,7 +1869,7 @@ class TestInstanceNotificationSample(
},
actual=self.notifier.versioned_notifications[1])
def _test_share_attach_error(self, server):
def _test_share_attach_detach_error(self, server):
expected_shares = [
{
@ -1906,6 +1906,7 @@ class TestInstanceNotificationSample(
):
# Simulate an error attaching the share.
original_side_effect = self.manila_fixture.mock_allow.side_effect
self.manila_fixture.mock_allow.side_effect = (
exception.ShareAccessGrantError(
share_id="8db0037b-e98f-4bde-ae71-f96a077c19a4",
@ -2004,6 +2005,95 @@ class TestInstanceNotificationSample(
self._wait_for_notification('instance.reboot.start')
self._wait_for_notification('instance.reboot.end')
# Attach a share again in order to simulate a detach error
self.manila_fixture.mock_allow.side_effect = original_side_effect
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")
share_info = self._get_share(
server, "e8debdc0-447a-4376-a10a-4cd9122d7986", admin=True
)
expected_shares[0]["nova_object.data"][
"share_mapping_uuid"
] = share_info["uuid"]
self.assertEqual(2, len(self.notifier.versioned_notifications),
self.notifier.versioned_notifications)
expected_shares[0]['nova_object.data']['status'] = 'attaching'
self._verify_notification(
'instance-share_attach-start',
replacements={
'reservation_id': server['reservation_id'],
'uuid': server['id'],
'state': 'stopped',
'power_state': 'shutdown',
'shares': expected_shares
},
actual=self.notifier.versioned_notifications[0])
expected_shares[0]['nova_object.data']['status'] = 'inactive'
self._verify_notification(
'instance-share_attach-end',
replacements={
'reservation_id': server['reservation_id'],
'uuid': server['id'],
'state': 'stopped',
'power_state': 'shutdown',
'shares': expected_shares
},
actual=self.notifier.versioned_notifications[1])
# Simulate an error detaching the share.
self.manila_fixture.mock_deny.side_effect = (
exception.ShareAccessRemovalError(
share_id="8db0037b-e98f-4bde-ae71-f96a077c19a4",
reason="Connection timed out"
)
)
self.notifier.reset()
self._detach_share_with_error(
server, "e8debdc0-447a-4376-a10a-4cd9122d7986")
# 0: instance-share_detach-start
# 1: instance-share_detach-error
# 2: compute.exception
self.assertEqual(3, len(self.notifier.versioned_notifications),
self.notifier.versioned_notifications)
expected_shares[0]["nova_object.data"]["status"] = "detaching"
self._verify_notification(
'instance-share_detach-start',
replacements={
'reservation_id': server['reservation_id'],
'uuid': server['id'],
'state': 'stopped',
'power_state': 'shutdown',
'shares': expected_shares,
},
actual=self.notifier.versioned_notifications[0])
expected_shares[0]["nova_object.data"]["status"] = "error"
self._verify_notification(
'instance-share_detach-error',
replacements={
'reservation_id': server['reservation_id'],
'uuid': server['id'],
'state': 'stopped',
'power_state': 'shutdown',
'shares': expected_shares,
'fault.traceback': self.ANY
},
actual=self.notifier.versioned_notifications[1])
self._wait_for_notification('compute.exception')
# Reboot server
self.notifier.reset()
post = {'reboot': {'type': 'HARD'}}
self.api.post_server_action(server['id'], post)
self._wait_for_notification('instance.reboot.start')
self._wait_for_notification('instance.reboot.end')
def _test_rescue_unrescue_server(self, server):
# Both "rescue" and "unrescue" notification asserts are made here
# rescue notification asserts

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

@ -2813,7 +2813,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.START,
share_id=share_mapping.share_id
share_id=share_mapping.share_id,
),
mock.call(
mock.ANY,
@ -2821,7 +2821,77 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.END,
share_id=share_mapping.share_id
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')
@mock.patch('nova.objects.share_mapping.ShareMapping.save')
def test_deny_share_fails_access_removal(
self, mock_db, mock_db_get_share, mock_get_access, mock_deny,
mock_db_delete, mock_notifications
):
"""Make sure we can remove a share even if we have an error with
the access or the access is not existing anymore for any reason.
"""
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()
mock_db_get_share.return_value.objects.append(share_mapping)
# 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')
exc = exception.ShareAccessRemovalError(
share_id=share_mapping.share_id,
reason="fake_reason"
)
mock_deny.side_effect = exc
self.assertRaises(
exception.ShareAccessRemovalError,
self.compute.deny_share,
self.context,
instance,
share_mapping,
)
mock_deny.assert_called_once_with(
mock.ANY, share_mapping.share_id, 'ip', compute_ip)
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.ERROR,
share_id=share_mapping.share_id,
exception=exc
),
])
@ -2861,7 +2931,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.START,
share_id=share_mapping.share_id
share_id=share_mapping.share_id,
),
mock.call(
mock.ANY,
@ -2869,7 +2939,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.END,
share_id=share_mapping.share_id
share_id=share_mapping.share_id,
),
])
@ -2918,7 +2988,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.START,
share_id=share_mapping.share_id
share_id=share_mapping.share_id,
),
mock.call(
mock.ANY,
@ -2926,7 +2996,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.END,
share_id=share_mapping.share_id
share_id=share_mapping.share_id,
),
])
@ -2964,6 +3034,24 @@ 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',
@ -2999,6 +3087,24 @@ 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',
@ -3032,15 +3138,34 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
share_id=share_mapping.share_id,
reason="fake_reason"
)
self.assertRaises(
exc = self.assertRaises(
exception.ShareAccessRemovalError,
self.compute.deny_share,
self.context,
instance,
share_mapping
share_mapping,
)
mock_db_delete.assert_not_called()
self.assertEqual(share_mapping.status, 'error')
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.ERROR,
share_id=share_mapping.share_id,
exception=exc,
),
])
@mock.patch(
'nova.compute.utils.notify_about_share_attach_detach',
@ -3073,7 +3198,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
mock_deny.side_effect = keystone_exception.http.Unauthorized(
message="Unauthorized"
)
self.assertRaises(
exc = self.assertRaises(
keystone_exception.http.Unauthorized,
self.compute.deny_share,
self.context,
@ -3082,6 +3207,25 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
)
mock_db_delete.assert_not_called()
self.assertEqual(share_mapping.status, 'error')
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.ERROR,
share_id=share_mapping.share_id,
exception=exc,
),
])
@mock.patch(
'nova.compute.utils.notify_about_share_attach_detach',
@ -3114,7 +3258,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
mock_deny.side_effect = exception.ShareProtocolNotSupported(
share_proto=share_mapping.share_proto
)
self.assertRaises(
exc = self.assertRaises(
exception.ShareProtocolNotSupported,
self.compute.deny_share,
self.context,
@ -3123,6 +3267,25 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
)
mock_db_delete.assert_not_called()
self.assertEqual(share_mapping.status, 'error')
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.ERROR,
share_id=share_mapping.share_id,
exception=exc,
),
])
@mock.patch(
'nova.compute.utils.notify_about_share_attach_detach',
@ -3162,7 +3325,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.START,
share_id=share_mapping1.share_id
share_id=share_mapping1.share_id,
),
mock.call(
mock.ANY,
@ -3170,7 +3333,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.END,
share_id=share_mapping1.share_id
share_id=share_mapping1.share_id,
),
])
@ -3214,7 +3377,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.START,
share_id=share_mapping1.share_id
share_id=share_mapping1.share_id,
),
mock.call(
mock.ANY,
@ -3222,7 +3385,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
"fake-host",
action=fields.NotificationAction.SHARE_DETACH,
phase=fields.NotificationPhase.END,
share_id=share_mapping1.share_id
share_id=share_mapping1.share_id,
),
])