Merge "Reports instance events to the DB regarding attaching and detaching a share"

2024-12-13 03:45:20 +00:00 · 2024-12-13 03:45:20 +00:00 · 967dbbd9aa
commit 967dbbd9aa
parent 728337f200 57d96c265b
6 changed files with 119 additions and 18 deletions
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@ -6066,10 +6066,14 @@ class API:
        return host_statuses

    def allow_share(self, context, instance, share_mapping):
+        self._record_action_start(
+            context, instance, instance_actions.ATTACH_SHARE)
        self.compute_rpcapi.allow_share(
            context, instance, share_mapping)

    def deny_share(self, context, instance, share_mapping):
+        self._record_action_start(
+            context, instance, instance_actions.DETACH_SHARE)
        self.compute_rpcapi.deny_share(
            context, instance, share_mapping)

--- a/nova/compute/instance_actions.py
+++ b/nova/compute/instance_actions.py
@ -65,6 +65,8 @@ ATTACH_INTERFACE = 'attach_interface'
 DETACH_INTERFACE = 'detach_interface'
 ATTACH_VOLUME = 'attach_volume'
 DETACH_VOLUME = 'detach_volume'
+ATTACH_SHARE = 'attach_share'
+DETACH_SHARE = 'detach_share'
 SWAP_VOLUME = 'swap_volume'
 LOCK = 'lock'
 UNLOCK = 'unlock'
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@ -4633,6 +4633,8 @@ class ComputeManager(manager.Manager):

    @messaging.expected_exceptions(NotImplementedError)
    @wrap_exception()
+    @wrap_instance_event(prefix='compute')
+    @wrap_instance_fault
    def allow_share(self, context, instance, share_mapping):

        @utils.synchronized(share_mapping.share_id)
@ -4779,6 +4781,8 @@ class ComputeManager(manager.Manager):

    @messaging.expected_exceptions(NotImplementedError)
    @wrap_exception()
+    @wrap_instance_event(prefix='compute')
+    @wrap_instance_fault
    def deny_share(self, context, instance, share_mapping):

        @utils.synchronized(share_mapping.share_id)
--- a/nova/tests/functional/notification_sample_tests/test_instance.py
+++ b/nova/tests/functional/notification_sample_tests/test_instance.py
@ -2058,7 +2058,7 @@ class TestInstanceNotificationSample(
        # 0: instance-share_detach-start
        # 1: instance-share_detach-error
        # 2: compute.exception
-        self.assertEqual(3, len(self.notifier.versioned_notifications),
+        self.assertEqual(2, len(self.notifier.versioned_notifications),
                            self.notifier.versioned_notifications)
        expected_shares[0]["nova_object.data"]["status"] = "detaching"
        self._verify_notification(
--- a/nova/tests/unit/compute/test_api.py
+++ b/nova/tests/unit/compute/test_api.py
@ -8683,8 +8683,9 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
        share_mapping.share_proto = 'NFS'
        return share_mapping

+    @mock.patch('nova.compute.api.API._record_action_start')
    @mock.patch('oslo_messaging.rpc.client._BaseCallContext.cast')
-    def test_allow_share(self, mock_cast):
+    def test_allow_share(self, mock_cast, mock_rec_action):
        instance = self._create_instance_obj(
                params=dict(vm_state=vm_states.STOPPED))
        self.assertEqual(instance.vm_state, vm_states.STOPPED)
@ -8702,8 +8703,12 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
            instance=instance,
            share_mapping=share_mapping)

+        mock_rec_action.assert_called_once_with(
+            self.context, instance, instance_actions.ATTACH_SHARE)
+
+    @mock.patch('nova.compute.api.API._record_action_start')
    @mock.patch('oslo_messaging.rpc.client._BaseCallContext.cast')
-    def test_deny_share(self, mock_cast):
+    def test_deny_share(self, mock_cast, mock_rec_action):
        instance = self._create_instance_obj(
                params=dict(vm_state=vm_states.STOPPED))
        self.assertEqual(instance.vm_state, vm_states.STOPPED)
@ -8720,3 +8725,6 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
            'deny_share',
            instance=instance,
            share_mapping=share_mapping)
+
+        mock_rec_action.assert_called_once_with(
+            self.context, instance, instance_actions.DETACH_SHARE)
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@ -2505,6 +2505,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+    @mock.patch.object(db, 'instance_fault_create')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2513,7 +2514,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @mock.patch('nova.share.manila.API.get_access')
    @mock.patch('nova.objects.share_mapping.ShareMapping.save')
    def test_allow_share_fails_share_grant_failure(
-        self, mock_db, mock_get_access, mock_allow, mock_notifications
+        self, mock_db, mock_get_access, mock_allow, mock_notifications,
+        mock_instance_fault_create
    ):
        self.flags(shutdown_retry_interval=20, group='compute')
        instance = fake_instance.fake_instance_obj(
@ -2522,6 +2524,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
                vm_state=vm_states.ACTIVE,
                task_state=task_states.POWERING_OFF)
        mock_get_access.side_effect = [None, self.get_fake_share_access()]
+        mock_instance_fault_create.return_value = (
+            test_instance_fault.fake_faults['fake-uuid'][0])
        # 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')
@ -2571,6 +2575,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2579,7 +2584,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @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, mock_notifications
+        self, mock_db, mock_get_access, mock_allow, mock_notifications,
+            mock_instance_fault
    ):
        self.flags(shutdown_retry_interval=20, group='compute')
        instance = fake_instance.fake_instance_obj(
@ -2599,7 +2605,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.side_effect = exception.ShareNotFound(
            share_id=share_mapping.share_id
        )
-        self.assertRaises(
+        exc = self.assertRaises(
            exception.ShareNotFound,
            self.compute.allow_share,
            self.context,
@ -2611,6 +2617,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.assert_called_once_with(
            mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw')

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2619,7 +2633,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @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, mock_notifications
+        self, mock_db, mock_get_access, mock_allow, mock_notifications,
+            mock_instance_fault
    ):
        self.flags(shutdown_retry_interval=20, group='compute')
        instance = fake_instance.fake_instance_obj(
@ -2640,7 +2655,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            share_id=share_mapping.share_id,
            reason="fake_reason"
        )
-        self.assertRaises(
+        exc = self.assertRaises(
            exception.ShareAccessGrantError,
            self.compute.allow_share,
            self.context,
@ -2652,6 +2667,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.assert_called_once_with(
            mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw')

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2660,7 +2683,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @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, mock_notifications
+        self, mock_db, mock_get_access, mock_allow, mock_notifications,
+            mock_instance_fault
    ):
        self.flags(shutdown_retry_interval=20, group='compute')
        instance = fake_instance.fake_instance_obj(
@ -2678,7 +2702,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        self.assertEqual(compute_ip, '192.168.0.1')
        share_mapping = self.get_fake_share_mapping()
        mock_allow.side_effect = sdk_exc.BadRequestException()
-        self.assertRaises(
+        exc = self.assertRaises(
            sdk_exc.BadRequestException,
            self.compute.allow_share,
            self.context,
@ -2690,6 +2714,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.assert_called_once_with(
            mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw')

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2698,7 +2730,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @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, mock_notifications
+        self, mock_db, mock_get_access, mock_allow, mock_notifications,
+            mock_instance_fault
    ):
        self.flags(shutdown_retry_interval=20, group='compute')
        instance = fake_instance.fake_instance_obj(
@ -2718,7 +2751,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.side_effect = keystone_exception.http.Unauthorized(
            message="Unauthorized"
        )
-        self.assertRaises(
+        exc = self.assertRaises(
            keystone_exception.http.Unauthorized,
            self.compute.allow_share,
            self.context,
@ -2730,6 +2763,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.assert_called_once_with(
            mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw')

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2738,7 +2779,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @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, mock_notifications
+        self, mock_db, mock_get_access, mock_allow, mock_notifications,
+            mock_instance_fault
    ):
        self.flags(shutdown_retry_interval=20, group='compute')
        instance = fake_instance.fake_instance_obj(
@ -2758,7 +2800,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.side_effect = exception.ShareProtocolNotSupported(
            share_proto=share_mapping.share_proto
        )
-        self.assertRaises(
+        exc = self.assertRaises(
            exception.ShareProtocolNotSupported,
            self.compute.allow_share,
            self.context,
@ -2770,6 +2812,13 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        mock_allow.assert_called_once_with(
            mock.ANY, share_mapping.share_id, 'ip', compute_ip, 'rw')

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2825,6 +2874,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -2836,7 +2886,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
    @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
+        mock_db_delete, mock_notifications, mock_instance_fault
    ):
        """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.
@ -2852,6 +2902,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
        )
        share_mapping = self.get_fake_share_mapping()
        mock_db_get_share.return_value.objects.append(share_mapping)
+        mock_instance_fault.return_value = (
+            test_instance_fault.fake_faults['fake-uuid'][0])
        # 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')
@ -2895,6 +2947,13 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -3106,6 +3165,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -3117,7 +3177,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_notifications
+            mock_db_save, mock_notifications, mock_instance_fault
    ):
        """Ensure we have an exception if the access cannot be removed
        by manila.
@ -3167,6 +3227,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -3178,7 +3246,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_notifications
+            mock_db_save, mock_notifications, mock_instance_fault
    ):
        """Ensure we have an exception if the access cannot be removed
        by manila.
@ -3227,6 +3295,14 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None
@ -3238,7 +3314,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_notifications
+            mock_db_save, mock_notifications, mock_instance_fault
    ):
        """Ensure we have an exception if the access cannot be removed
        by manila.
@ -3287,6 +3363,13 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
            ),
        ])

+        mock_instance_fault.assert_called_once_with(
+            mock.ANY,
+            instance,
+            exc,
+            mock.ANY
+        )
+
    @mock.patch(
        'nova.compute.utils.notify_about_share_attach_detach',
        return_value=None