diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 82bbece2df..2eaa4fec58 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/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. diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 062e74aa75..1717cb55b9 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/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): diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index bbbb9dd5f0..994d77d78f 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/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) diff --git a/releasenotes/notes/fail-when-vif-port-id-is-missing-7640669f9d9e705d.yaml b/releasenotes/notes/fail-when-vif-port-id-is-missing-7640669f9d9e705d.yaml new file mode 100644 index 0000000000..16bb4ad61d --- /dev/null +++ b/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.