Browse Source

Merge "Add a lock to prevent race during detach/attach of interface" into stable/ussuri

changes/49/751349/1
Zuul 2 weeks ago
committed by Gerrit Code Review
parent
commit
7d556106bf
4 changed files with 56 additions and 7 deletions
  1. +25
    -0
      nova/compute/manager.py
  2. +16
    -4
      nova/tests/unit/compute/test_compute.py
  3. +7
    -3
      nova/tests/unit/compute/test_compute_mgr.py
  4. +8
    -0
      releasenotes/notes/bug-1892870-eb894956bf04713d.yaml

+ 25
- 0
nova/compute/manager.py View File

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


+ 16
- 4
nova/tests/unit/compute/test_compute.py View File

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


+ 7
- 3
nova/tests/unit/compute/test_compute_mgr.py View File

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



+ 8
- 0
releasenotes/notes/bug-1892870-eb894956bf04713d.yaml View File

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

Loading…
Cancel
Save