diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 814c51152d93..8ae115ae3e52 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -7525,6 +7525,19 @@ class ComputeManager(manager.Manager): def attach_interface(self, context, instance, network_id, port_id, requested_ip, tag): """Use hotplug to add an network adapter to an instance.""" + lockname = 'interface-%s-%s' % (instance.uuid, port_id) + + @utils.synchronized(lockname) + def do_attach_interface(context, instance, network_id, port_id, + requested_ip, tag): + return self._attach_interface(context, instance, network_id, + port_id, requested_ip, tag) + + return do_attach_interface(context, instance, network_id, port_id, + requested_ip, tag) + + def _attach_interface(self, context, instance, network_id, port_id, + requested_ip, tag): if not self.driver.capabilities.get('supports_attach_interface', False): raise exception.AttachInterfaceNotSupported( @@ -7585,6 +7598,18 @@ class ComputeManager(manager.Manager): @wrap_instance_fault def detach_interface(self, context, instance, port_id): """Detach a network adapter from an instance.""" + lockname = 'interface-%s-%s' % (instance.uuid, port_id) + + @utils.synchronized(lockname) + def do_detach_interface(context, instance, port_id): + self._detach_interface(context, instance, port_id) + + do_detach_interface(context, instance, port_id) + + def _detach_interface(self, context, instance, port_id): + # NOTE(aarents): we need to refresh info cache from DB here, + # as previous detach/attach lock holder just updated it. + compute_utils.refresh_info_cache_for_instance(context, instance) network_info = instance.info_cache.network_info condemned = None for vif in network_info: diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 1db1bff1054f..a72569418268 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -10344,11 +10344,15 @@ class ComputeAPITestCase(BaseTestCase): network_id = nwinfo[0]['network']['id'] port_id = nwinfo[0]['id'] req_ip = '1.2.3.4' + lock_name = 'interface-%s-%s' % (instance.uuid, port_id) mock_allocate = mock.Mock(return_value=nwinfo) self.compute.network_api.allocate_port_for_instance = mock_allocate - with mock.patch.dict(self.compute.driver.capabilities, - supports_attach_interface=True): + with test.nested( + mock.patch.dict(self.compute.driver.capabilities, + supports_attach_interface=True), + mock.patch('oslo_concurrency.lockutils.lock') + ) as (cap, mock_lock): vif = self.compute.attach_interface(self.context, instance, network_id, @@ -10363,6 +10367,9 @@ class ComputeAPITestCase(BaseTestCase): action='interface_attach', phase='start'), mock.call(self.context, instance, self.compute.host, action='interface_attach', phase='end')]) + mock_lock.assert_called_once_with(lock_name, mock.ANY, mock.ANY, + mock.ANY, delay=mock.ANY, do_log=mock.ANY, fair=mock.ANY, + semaphores=mock.ANY) return nwinfo, port_id @mock.patch.object(compute_utils, 'notify_about_instance_action') @@ -10460,6 +10467,7 @@ class ComputeAPITestCase(BaseTestCase): self.context, uuids.info_cache_instance) instance.info_cache.network_info = network_model.NetworkInfo.hydrate( nwinfo) + lock_name = 'interface-%s-%s' % (instance.uuid, port_id) port_allocation = {uuids.rp1: {'NET_BW_EGR_KILOBIT_PER_SEC': 10000}} with test.nested( @@ -10468,8 +10476,9 @@ class ComputeAPITestCase(BaseTestCase): 'remove_resources_from_instance_allocation'), mock.patch.object(self.compute.network_api, 'deallocate_port_for_instance', - return_value=([], port_allocation))) as ( - mock_remove_alloc, mock_deallocate): + return_value=([], port_allocation)), + mock.patch('oslo_concurrency.lockutils.lock')) as ( + mock_remove_alloc, mock_deallocate, mock_lock): self.compute.detach_interface(self.context, instance, port_id) mock_deallocate.assert_called_once_with( @@ -10483,6 +10492,9 @@ class ComputeAPITestCase(BaseTestCase): action='interface_detach', phase='start'), mock.call(self.context, instance, self.compute.host, action='interface_detach', phase='end')]) + mock_lock.assert_called_once_with(lock_name, mock.ANY, mock.ANY, + mock.ANY, delay=mock.ANY, do_log=mock.ANY, fair=mock.ANY, + semaphores=mock.ANY) @mock.patch('nova.compute.manager.LOG.log') def test_detach_interface_failed(self, mock_log): diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 5284c9bae5e8..09a8d05927c4 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2454,8 +2454,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @mock.patch.object(compute_utils, 'EventReporter') @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') + @mock.patch.object(compute_utils, 'refresh_info_cache_for_instance') @mock.patch.object(self.compute, '_set_instance_obj_error_state') - def do_test(meth, add_fault, event): + def do_test(meth, refresh, add_fault, event): self.assertRaises(exception.PortNotFound, self.compute.detach_interface, self.context, f_instance, 'port_id') @@ -2464,6 +2465,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, event.assert_called_once_with( self.context, 'compute_detach_interface', CONF.host, f_instance.uuid, graceful_exit=False) + refresh.assert_called_once_with(self.context, f_instance) do_test() @@ -2471,8 +2473,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, @mock.patch.object(compute_utils, 'EventReporter') @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') @mock.patch.object(compute_utils, 'notify_about_instance_action') - def test_detach_interface_instance_not_found(self, mock_notify, mock_fault, - mock_event, mock_log): + @mock.patch.object(compute_utils, 'refresh_info_cache_for_instance') + def test_detach_interface_instance_not_found(self, mock_refresh, + mock_notify, mock_fault, mock_event, mock_log): nw_info = network_model.NetworkInfo([ network_model.VIF(uuids.port_id)]) info_cache = objects.InstanceInfoCache(network_info=nw_info, @@ -2485,6 +2488,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.assertRaises(exception.InterfaceDetachFailed, self.compute.detach_interface, self.context, instance, uuids.port_id) + mock_refresh.assert_called_once_with(self.context, instance) self.assertEqual(1, mock_log.call_count) self.assertEqual(logging.DEBUG, mock_log.call_args[0][0]) diff --git a/releasenotes/notes/bug-1892870-eb894956bf04713d.yaml b/releasenotes/notes/bug-1892870-eb894956bf04713d.yaml new file mode 100644 index 000000000000..3a4fe968f30a --- /dev/null +++ b/releasenotes/notes/bug-1892870-eb894956bf04713d.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Resolve a race condition that may occur during concurrent + ``interface detach/attach``, resulting in an interface accidentally unbind + after attached. See `bug 1892870`_ for more details. + + .. _bug 1892870: https://bugs.launchpad.net/nova/+bug/1892870