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 7f81cf28bf21ad2afa98accfde3087c83b8e269b)
(cherry picked from commit eb6d70f02daa14920a2522e5c734a3775ea2ea7c)
(cherry picked from commit f210115bcba3436b957a609cd388a13e6d77a638)

3 files changed, 47 insertions, 18 deletions

diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py
index 801846c4dd..7f907336f9 100644
--- a/nova/tests/unit/virt/ironic/test_driver.py
+++ b/nova/tests/unit/virt/ironic/test_driver.py
@@ -2018,17 +2018,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):
@@ -2162,11 +2157,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):
@@ -2459,17 +2460,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')
@@ -2477,7 +2484,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):
diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py
index 3bd4d18ecc..d8140e733b 100644
--- a/nova/virt/ironic/driver.py
+++ b/nova/virt/ironic/driver.py
@@ -1623,13 +1623,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.
@@ -1660,7 +1668,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.
@@ -1977,7 +1986,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 "
diff --git a/releasenotes/notes/fix-ironic-compute-restart-port-attachments-3282e9ea051561d4.yaml b/releasenotes/notes/fix-ironic-compute-restart-port-attachments-3282e9ea051561d4.yaml
new file mode 100644
index 0000000000..2c2c9e7838
--- /dev/null
+++ b/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.