Merge "Ignore plug_vifs on the ironic driver" into stable/xena

Author: Zuul

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):
@@ -2440,25 +2441,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.
@@ -1907,7 +1916,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.