openstack/nova
Code Issues Proposed changes

Ignore plug_vifs on the ironic driver

Browse Source 
When the nova-compute service starts, by default it attempts to
startup instance configuration states for aspects such as networking.
This is fine in most cases, and makes a lot of sense if the
nova-compute service is just managing virtual machines on a hypervisor.

This is done, one instance at a time.

However, when the compute driver is ironic, the networking is managed
as part of the physical machine lifecycle potentially all the way into
committed switch configurations. As such, there is no need to attempt
to call ``plug_vifs`` on every single instance managed by the
nova-compute process which is backed by Ironic.

Additionally, using ironic tends to manage far more physical machines
per nova-compute service instance then when when operating co-installed
with a hypervisor. Often this means a cluster of a thousand machines,
with three controllers, will see thousands of un-needed API calls upon
service start, which elongates the entire process and negatively
impacts operations.

In essence, nova.virt.ironic's plug_vifs call now does nothing,
and merely issues a debug LOG entry when called.

Closes-Bug: #1777608
Change-Id: Iba87cef50238c5b02ab313f2311b826081d5b4ab
This commit is contained in:
Julia Kreger 2021-10-08 14:35:00 -07:00
parent fdfdba2658
commit 7f81cf28bf
3 changed files with 47 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
nova/tests/unit/virt/ironic/test_driver.py
View File

@ -2016,17 +2016,12 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
def test_plug_vifs(self, mock__plug_vifs):
node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = _get_cached_node(id=node_id)
self.mock_conn.get_node.return_value = node
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_id)
network_info = utils.get_test_network_info()
self.driver.plug_vifs(instance, network_info)
self.mock_conn.get_node.assert_called_once_with(
node_id, fields=ironic_driver._NODE_FIELDS)
mock__plug_vifs.assert_called_once_with(node, instance, network_info)
mock__plug_vifs.assert_not_called()
@mock.patch.object(FAKE_CLIENT.node, 'vif_attach')
def test_plug_vifs_multiple_ports(self, mock_vatt):
@ -2160,11 +2155,17 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.driver.unplug_vifs(instance, network_info)
self.assertFalse(mock_vdet.called)
@mock.patch.object(ironic_driver.IronicDriver, 'plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
def test_attach_interface(self, mock_pv):
self.driver.attach_interface('fake_context', 'fake_instance',
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = _get_cached_node(uuid=node_uuid)
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
self.mock_conn.get_node.return_value = node
self.driver.attach_interface('fake_context', instance,
'fake_image_meta', 'fake_vif')
mock_pv.assert_called_once_with('fake_instance', ['fake_vif'])
mock_pv.assert_called_once_with(node, instance, ['fake_vif'])
@mock.patch.object(ironic_driver.IronicDriver, 'unplug_vifs')
def test_detach_interface(self, mock_uv):
@ -2440,17 +2441,23 @@ class IronicDriverTestCase(test.NoDBTestCase):
def test_get_volume_connector_no_ip_no_fixed_ip(self):
self._test_get_volume_connector_no_ip(False, no_fixed_ip=True)
@mock.patch.object(ironic_driver.IronicDriver, 'plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
def test_prepare_networks_before_block_device_mapping(self, mock_pvifs):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = _get_cached_node(uuid=node_uuid)
self.mock_conn.get_node.return_value = node
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = utils.get_test_network_info()
self.driver.prepare_networks_before_block_device_mapping(instance,
network_info)
mock_pvifs.assert_called_once_with(instance, network_info)
mock_pvifs.assert_called_once_with(node, instance, network_info)
@mock.patch.object(ironic_driver.IronicDriver, 'plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
def test_prepare_networks_before_block_device_mapping_error(self,
mock_pvifs):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = _get_cached_node(uuid=node_uuid)
self.mock_conn.get_node.return_value = node
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = utils.get_test_network_info()
mock_pvifs.side_effect = ironic_exception.BadRequest('fake error')
@ -2458,7 +2465,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
ironic_exception.BadRequest,
self.driver.prepare_networks_before_block_device_mapping,
instance, network_info)
mock_pvifs.assert_called_once_with(instance, network_info)
mock_pvifs.assert_called_once_with(node, instance, network_info)
@mock.patch.object(ironic_driver.IronicDriver, 'unplug_vifs')
def test_clean_networks_preparation(self, mock_upvifs):

21
nova/virt/ironic/driver.py
View File

@ -1558,13 +1558,21 @@ class IronicDriver(virt_driver.ComputeDriver):
def plug_vifs(self, instance, network_info):
"""Plug VIFs into networks.
This method is present for compatability. Any call will result
in a DEBUG log entry being generated, and will otherwise be
ignored, as Ironic manages VIF attachments through a node
lifecycle. Please see ``attach_interface``, which is the
proper and current method to utilize.
:param instance: The instance object.
:param network_info: Instance network information.
"""
# instance.node is the ironic node's UUID.
node = self._get_node(instance.node)
self._plug_vifs(node, instance, network_info)
LOG.debug('VIF plug called for instance %(instance)s on node '
'%(node)s, however Ironic manages VIF attachments '
'for nodes.',
{'instance': instance.uuid,
'node': instance.node})
def unplug_vifs(self, instance, network_info):
"""Unplug VIFs from networks.
@ -1595,7 +1603,8 @@ class IronicDriver(virt_driver.ComputeDriver):
# event from neutron or by _heal_instance_info_cache periodic task. In
# both cases, this is done asynchronously, so the cache may not be up
# to date immediately after attachment.
self.plug_vifs(instance, [vif])
node = self._get_node(instance.node)
self._plug_vifs(node, instance, [vif])
def detach_interface(self, context, instance, vif):
"""Use hotunplug to remove a network interface from a running instance.
@ -1907,7 +1916,9 @@ class IronicDriver(virt_driver.ComputeDriver):
"""
try:
self.plug_vifs(instance, network_info)
node = self._get_node(instance.node)
self._plug_vifs(node, instance, network_info)
except Exception:
with excutils.save_and_reraise_exception():
LOG.error("Error preparing deploy for instance "

11
releasenotes/notes/fix-ironic-compute-restart-port-attachments-3282e9ea051561d4.yaml Normal file
View File

@ -0,0 +1,11 @@
---
fixes:
- |
Fixes slow compute restart when using the ``nova.virt.ironic`` compute
driver where the driver was previously attempting to attach VIFS on
start-up via the ``plug_vifs`` driver method. This method has grown
otherwise unused since the introduction of the ``attach_interface``
method of attaching VIFs. As Ironic manages the attachment of VIFs to
baremetal nodes in order to align with the security requirements of a
physical baremetal node's lifecycle. The ironic driver now ignores calls
to the ``plug_vifs`` method.