Update configure_tenant_networks

If there is no port.extra['vif_port_id'] in any ironic port deployment
should fail, since instance will not be plugged to tenant network.
This patch updates configure_tenant_networks() accordingly.

Closes-Bug: #1621783
Change-Id: I3631027853ccfb3857425f98ead7981ae1f603d5

ironic/drivers/modules/network/neutron.py

@@ -20,7 +20,7 @@ from oslo_log import log
 from oslo_utils import uuidutils
 
 from ironic.common import exception
-from ironic.common.i18n import _, _LI, _LW
+from ironic.common.i18n import _, _LI
 from ironic.common import neutron
 from ironic.drivers import base
 from ironic import objects
@@ -145,15 +145,12 @@ class NeutronNetwork(base.NetworkInterface):
         portmap = neutron.get_node_portmap(task)
 
         client = neutron.get_client(task.context.auth_token)
+        pobj_without_vif = 0
         for port_like_obj in ports + portgroups:
             vif_port_id = port_like_obj.extra.get('vif_port_id')
 
             if not vif_port_id:
-                LOG.warning(
-                    _LW('%(port_like_object)s %(pobj_uuid)s in node %(node)s '
-                        'has no vif_port_id value in extra field.'),
-                    {'port_like_object': port_like_obj.__class__.__name__,
-                     'pobj_uuid': port_like_obj.uuid, 'node': node.uuid})
+                pobj_without_vif += 1
                 continue
 
             LOG.debug('Mapping tenant port %(vif_port_id)s to node '
@@ -200,6 +197,12 @@ class NeutronNetwork(base.NetworkInterface):
                 LOG.error(msg)
                 raise exception.NetworkError(msg)
 
+        if pobj_without_vif == len(ports + portgroups):
+            msg = _("No neutron ports or portgroups are associated with "
+                    "node %s") % node.uuid
+            LOG.error(msg)
+            raise exception.NetworkError(msg)
+
     def unconfigure_tenant_networks(self, task):
         """Unconfigure tenant networks for a node.
 

ironic/tests/unit/drivers/modules/network/test_neutron.py

@@ -120,11 +120,44 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
     def test_configure_tenant_networks_no_vif_id(self, log_mock, client_mock):
         self.port.extra = {}
         self.port.save()
+        upd_mock = mock.Mock()
+        client_mock.return_value.update_port = upd_mock
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaisesRegex(exception.NetworkError,
+                                   'No neutron ports or portgroups are '
+                                   'associated with node',
+                                   self.interface.configure_tenant_networks,
+                                   task)
+        client_mock.assert_called_once_with(task.context.auth_token)
+        upd_mock.assert_not_called()
+        self.assertIn('No neutron ports or portgroups are associated with',
+                      log_mock.error.call_args[0][0])
+
+    @mock.patch.object(neutron_common, 'get_client')
+    @mock.patch.object(neutron, 'LOG')
+    def test_configure_tenant_networks_multiple_ports_one_vif_id(
+            self, log_mock, client_mock):
+        expected_body = {
+            'port': {
+                'device_owner': 'baremetal:none',
+                'device_id': self.node.instance_uuid or self.node.uuid,
+                'admin_state_up': True,
+                'binding:vnic_type': 'baremetal',
+                'binding:host_id': self.node.uuid,
+                'binding:profile': {'local_link_information':
+                                    [self.port.local_link_connection]}
+            }
+        }
+        utils.create_test_port(self.context, node_id=self.node.id,
+                               address='52:54:00:cf:2d:33', extra={},
+                               uuid=uuidutils.generate_uuid())
+        upd_mock = mock.Mock()
+        client_mock.return_value.update_port = upd_mock
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface.configure_tenant_networks(task)
-            client_mock.assert_called_once_with(task.context.auth_token)
-        self.assertIn('no vif_port_id value in extra',
-                      log_mock.warning.call_args[0][0])
+        client_mock.assert_called_once_with(task.context.auth_token)
+        upd_mock.assert_called_once_with(self.port.extra['vif_port_id'],
+                                         expected_body)
 
     @mock.patch.object(neutron_common, 'get_client')
     def test_configure_tenant_networks_update_fail(self, client_mock):

ironic/tests/unit/drivers/modules/test_agent_base_vendor.py

@@ -712,6 +712,39 @@ class TestHeartbeat(AgentDeployMixinBaseTest):
             self.assertEqual(states.NOSTATE, task.node.target_provision_state)
             self.assertFalse(mock_collect.called)
 
+    @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True)
+    @mock.patch.object(time, 'sleep', lambda seconds: None)
+    @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
+    @mock.patch.object(fake.FakePower, 'get_power_state',
+                       spec=types.FunctionType)
+    @mock.patch.object(agent_client.AgentClient, 'power_off',
+                       spec=types.FunctionType)
+    @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.'
+                'remove_provisioning_network', spec_set=True, autospec=True)
+    @mock.patch('ironic.drivers.modules.network.neutron.NeutronNetwork.'
+                'configure_tenant_networks', spec_set=True, autospec=True)
+    def test_reboot_and_finish_deploy_configure_tenant_network_exception(
+            self, configure_tenant_net_mock, remove_provisioning_net_mock,
+            power_off_mock, get_power_state_mock, node_power_action_mock,
+            mock_collect):
+        self.node.network_interface = 'neutron'
+        self.node.provision_state = states.DEPLOYING
+        self.node.target_provision_state = states.ACTIVE
+        self.node.save()
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            configure_tenant_net_mock.side_effect = exception.NetworkError(
+                "boom")
+            self.assertRaises(exception.InstanceDeployFailure,
+                              self.deploy.reboot_and_finish_deploy, task)
+            self.assertEqual(7, get_power_state_mock.call_count)
+            remove_provisioning_net_mock.assert_called_once_with(mock.ANY,
+                                                                 task)
+            configure_tenant_net_mock.assert_called_once_with(mock.ANY, task)
+            self.assertEqual(states.DEPLOYFAIL, task.node.provision_state)
+            self.assertEqual(states.ACTIVE, task.node.target_provision_state)
+            mock_collect.assert_called_once_with(task.node)
+
     @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True)
     @mock.patch.object(time, 'sleep', lambda seconds: None)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)

releasenotes/notes/fail-when-vif-port-id-is-missing-7640669f9d9e705d.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - Fail deployment when no ports or port groups are linked to
+    a node. This is to avoid active nodes not connected to any
+    tenant network.