Ignore plug_vifs on the ironic driver

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
(cherry picked from commit 7f81cf2)
(cherry picked from commit eb6d70f)

Author: juliakreger

nova/tests/unit/virt/ironic/test_driver.py

@@ -2016,17 +2016,12 @@ def test_plug_vifs_with_port(self, mock_vatt):
     @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 @@ def test_unplug_vifs_no_network_info(self, mock_vdet):
         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):
@@ -2457,25 +2458,31 @@ def test_get_volume_connector_no_ip_without_mac(self):
     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')
         self.assertRaises(
             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):

nova/virt/ironic/driver.py

@@ -1558,13 +1558,21 @@ def _unplug_vifs(self, node, instance, network_info):
     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 @@ def attach_interface(self, context, instance, image_meta, vif):
         # 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.
@@ -1911,7 +1920,9 @@ def prepare_networks_before_block_device_mapping(self, instance,
         """
 
         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 "

releasenotes/notes/fix-ironic-compute-restart-port-attachments-3282e9ea051561d4.yaml

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