openstack
/
ironic
Code Issues Proposed changes

Resolve race in validating neutron networks due to caching 

This patch removes the caching from the code that validates neutron
network's name and UUID to solve a race caused by storing a network UUID
(which can now be different per node) on a singleton network interface
object.

Change-Id: Ib5fb1ff93ffecec4b5ffb033013d3ae09dcaf4bc
This commit is contained in:
Sam Betts 2018-01-25 15:41:31 +00:00
parent f5654bfd00
commit 78e689e539
3 changed files with 51 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
ironic/common/neutron.py
View File

@ -564,22 +564,14 @@ def get_physnets_by_port_uuid(client, port_uuid):
class NeutronNetworkInterfaceMixin(object):
_cleaning_network_uuid = None
_provisioning_network_uuid = None
_rescuing_network_uuid = None
def get_cleaning_network_uuid(self, task):
cleaning_network = (
task.node.driver_info.get('cleaning_network') or
CONF.neutron.cleaning_network
)
# NOTE(dtantsur): if the last used cleaning network UUID is
# the same as the new one, we can skip validating it.
if (self._cleaning_network_uuid is None or
self._cleaning_network_uuid != cleaning_network):
self._cleaning_network_uuid = validate_network(
cleaning_network, _('cleaning network'),
context=task.context)
return validate_network(
cleaning_network, _('cleaning network'),
context=task.context)
return self._cleaning_network_uuid
def get_provisioning_network_uuid(self, task):
@ -587,14 +579,9 @@ class NeutronNetworkInterfaceMixin(object):
task.node.driver_info.get('provisioning_network') or
CONF.neutron.provisioning_network
)
# NOTE(dtantsur): if the last used provisioning network UUID is
# the same as the new one, we can skip validating it.
if (self._provisioning_network_uuid is None or
self._provisioning_network_uuid != provisioning_network):
self._provisioning_network_uuid = validate_network(
provisioning_network, _('provisioning network'),
context=task.context)
return self._provisioning_network_uuid
return validate_network(
provisioning_network, _('provisioning network'),
context=task.context)
# TODO(stendulker): FlatNetwork should not use this method.
# FlatNetwork uses tenant network for rescue operation.
@ -603,11 +590,6 @@ class NeutronNetworkInterfaceMixin(object):
task.node.driver_info.get('rescuing_network') or
CONF.neutron.rescuing_network
)
# NOTE(dtantsur): if the last used provisioning network UUID is
# the same as the new one, we can skip validating it.
if (self._rescuing_network_uuid is None or
self._rescuing_network_uuid != rescuing_network):
self._rescuing_network_uuid = validate_network(
rescuing_network, _('rescuing network'),
context=task.context)
return self._rescuing_network_uuid
return validate_network(
rescuing_network, _('rescuing network'),
context=task.context)

10
ironic/tests/unit/drivers/modules/network/test_flat.py
View File

@ -107,9 +107,13 @@ class TestFlatInterface(db_base.DbTestCase):
task, CONF.neutron.cleaning_network)
add_mock.assert_called_once_with(
task, CONF.neutron.cleaning_network)
validate_mock.assert_called_once_with(
CONF.neutron.cleaning_network,
'cleaning network', context=task.context)
validate_calls = [
mock.call(CONF.neutron.cleaning_network, 'cleaning network',
context=task.context),
mock.call(CONF.neutron.cleaning_network, 'cleaning network',
context=task.context)
]
validate_mock.assert_has_calls(validate_calls)
self.port.refresh()
self.assertEqual('vif-port-id',
self.port.internal_info['cleaning_vif_port_id'])

50
ironic/tests/unit/drivers/modules/network/test_neutron.py
View File

@ -120,9 +120,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
add_ports_mock.assert_called_once_with(
task, CONF.neutron.provisioning_network,
security_groups=[])
validate_mock.assert_called_once_with(
CONF.neutron.provisioning_network,
'provisioning network', context=task.context)
validate_calls = [
mock.call(CONF.neutron.provisioning_network,
'provisioning network', context=task.context),
mock.call(CONF.neutron.provisioning_network,
'provisioning network', context=task.context)
]
validate_mock.assert_has_calls(validate_calls)
self.port.refresh()
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['provisioning_vif_port_id'])
@ -151,9 +155,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
add_ports_mock.assert_called_with(
task, provisioning_network_uuid,
security_groups=[])
validate_mock.assert_called_with(
provisioning_network_uuid,
'provisioning network', context=task.context)
validate_calls = [
mock.call(provisioning_network_uuid,
'provisioning network', context=task.context),
mock.call(provisioning_network_uuid,
'provisioning network', context=task.context)
]
validate_mock.assert_has_calls(validate_calls)
self.port.refresh()
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['provisioning_vif_port_id'])
@ -234,9 +242,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
rollback_mock.assert_called_once_with(
task, CONF.neutron.cleaning_network)
self.assertEqual(res, add_ports_mock.return_value)
validate_mock.assert_called_once_with(
CONF.neutron.cleaning_network,
'cleaning network', context=task.context)
validate_calls = [
mock.call(CONF.neutron.cleaning_network,
'cleaning network', context=task.context),
mock.call(CONF.neutron.cleaning_network,
'cleaning network', context=task.context)
]
validate_mock.assert_has_calls(validate_calls)
self.port.refresh()
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['cleaning_vif_port_id'])
@ -350,9 +362,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
rollback_mock.assert_called_once_with(
task, CONF.neutron.rescuing_network)
self.assertEqual(add_ports_mock.return_value, res)
validate_mock.assert_called_once_with(
CONF.neutron.rescuing_network,
'rescuing network', context=task.context)
validate_calls = [
mock.call(CONF.neutron.rescuing_network,
'rescuing network', context=task.context),
mock.call(CONF.neutron.rescuing_network,
'rescuing network', context=task.context)
]
validate_mock.assert_has_calls(validate_calls)
other_port.refresh()
self.assertEqual(neutron_other_port['id'],
other_port.internal_info['rescuing_vif_port_id'])
@ -386,9 +402,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
rollback_mock.assert_called_once_with(
task, rescuing_network_uuid)
self.assertEqual(add_ports_mock.return_value, res)
validate_mock.assert_called_once_with(
rescuing_network_uuid,
'rescuing network', context=task.context)
validate_calls = [
mock.call(rescuing_network_uuid,
'rescuing network', context=task.context),
mock.call(rescuing_network_uuid,
'rescuing network', context=task.context)
]
validate_mock.assert_has_calls(validate_calls)
other_port.refresh()
self.assertEqual(neutron_other_port['id'],
other_port.internal_info['rescuing_vif_port_id'])