diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 0b16acdd85..5a7509efae 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -127,24 +127,20 @@ def add_ports_to_network(task, network_uuid, is_flat=False): try: port = client.create_port(body) except neutron_exceptions.NeutronClientException as e: - rollback_ports(task, network_uuid) - msg = (_('Could not create neutron port for ironic port ' - '%(ir-port)s on given network %(net)s from node ' - '%(node)s. %(exc)s') % - {'net': network_uuid, 'node': node.uuid, - 'ir-port': ironic_port.uuid, 'exc': e}) - LOG.exception(msg) - raise exception.NetworkError(msg) - - try: - ports[ironic_port.uuid] = port['port']['id'] - except KeyError: failures.append(ironic_port.uuid) + LOG.warning(_LW("Could not create neutron port for node's " + "%(node)s port %(ir-port) on the neutron " + "network %(net)s. %(exc)s"), + {'net': network_uuid, 'node': node.uuid, + 'ir-port': ironic_port.uuid, 'exc': e}) + else: + ports[ironic_port.uuid] = port['port']['id'] if failures: if len(failures) == len(pxe_enabled_ports): + rollback_ports(task, network_uuid) raise exception.NetworkError(_( - "Failed to update vif_port_id for any PXE enabled port " + "Failed to create neutron ports for any PXE enabled port " "on node %s.") % node.uuid) else: LOG.warning(_LW("Some errors were encountered when updating " @@ -203,14 +199,6 @@ def remove_neutron_ports(task, params): return for port in ports: - if not port['id']: - # TODO(morgabra) client.list_ports() sometimes returns - # port objects with null ids. It's unclear why this happens. - LOG.warning(_LW("Deleting neutron port failed, missing 'id'. " - "Node: %(node)s, neutron port: %(port)s."), - {'node': node_uuid, 'port': port}) - continue - LOG.debug('Deleting neutron port %(vif_port_id)s of node ' '%(node_id)s.', {'vif_port_id': port['id'], 'node_id': node_uuid}) diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index ef18f5733d..bc9c032691 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -223,31 +223,6 @@ class TestNeutronNetworkActions(db_base.DbTestCase): def test_add_ports_with_client_id_to_flat_network(self): self._test_add_ports_to_flat_network(is_client_id=True) - def test_add_ports_to_flat_network_no_neutron_port_id(self): - port = self.ports[0] - expected_body = { - 'port': { - 'network_id': self.network_uuid, - 'admin_state_up': True, - 'binding:vnic_type': 'baremetal', - 'device_owner': 'baremetal:none', - 'device_id': self.node.uuid, - 'mac_address': port.address, - 'binding:profile': { - 'local_link_information': [port.local_link_connection] - } - } - } - del self.neutron_port['id'] - self.client_mock.create_port.return_value = { - 'port': self.neutron_port} - with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaises(exception.NetworkError, - neutron.add_ports_to_network, - task, self.network_uuid, is_flat=True) - self.client_mock.create_port.assert_called_once_with( - expected_body) - def test_add_ports_to_vlan_network_instance_uuid(self): self.node.instance_uuid = uuidutils.generate_uuid() self.node.save() @@ -275,44 +250,33 @@ class TestNeutronNetworkActions(db_base.DbTestCase): self.client_mock.create_port.assert_called_once_with(expected_body) @mock.patch.object(neutron, 'rollback_ports') - def test_add_network_fail(self, rollback_mock): + def test_add_network_all_ports_fail(self, rollback_mock): # Check that if creating a port fails, the ports are cleaned up self.client_mock.create_port.side_effect = \ neutron_client_exc.ConnectionFailed with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaisesRegex( - exception.NetworkError, 'Could not create neutron port', - neutron.add_ports_to_network, task, self.network_uuid) + self.assertRaises( + exception.NetworkError, neutron.add_ports_to_network, task, + self.network_uuid) rollback_mock.assert_called_once_with(task, self.network_uuid) - @mock.patch.object(neutron, 'rollback_ports') - def test_add_network_fail_create_any_port_empty(self, rollback_mock): - self.client_mock.create_port.return_value = {} - with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaisesRegex( - exception.NetworkError, 'any PXE enabled port', - neutron.add_ports_to_network, task, self.network_uuid) - self.assertFalse(rollback_mock.called) - @mock.patch.object(neutron, 'LOG') - @mock.patch.object(neutron, 'rollback_ports') - def test_add_network_fail_create_some_ports_empty(self, rollback_mock, - log_mock): - port2 = object_utils.create_test_port( + def test_add_network_create_some_ports_fail(self, log_mock): + object_utils.create_test_port( self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), address='52:54:55:cf:2d:32', extra={'vif_port_id': uuidutils.generate_uuid()} ) self.client_mock.create_port.side_effect = [ - {'port': self.neutron_port}, {}] + {'port': self.neutron_port}, neutron_client_exc.ConnectionFailed] with task_manager.acquire(self.context, self.node.uuid) as task: neutron.add_ports_to_network(task, self.network_uuid) - self.assertIn(str(port2.uuid), - # Call #0, argument #1 - log_mock.warning.call_args[0][1]['ports']) - self.assertFalse(rollback_mock.called) + self.assertIn("Could not create neutron port for node's", + log_mock.warning.call_args_list[0][0][0]) + self.assertIn("Some errors were encountered when updating", + log_mock.warning.call_args_list[1][0][0]) @mock.patch.object(neutron, 'remove_neutron_ports') def test_remove_ports_from_network(self, remove_mock): diff --git a/releasenotes/notes/remove-neutron-client-workarounds-996c59623684929b.yaml b/releasenotes/notes/remove-neutron-client-workarounds-996c59623684929b.yaml new file mode 100644 index 0000000000..9644ca3fd0 --- /dev/null +++ b/releasenotes/notes/remove-neutron-client-workarounds-996c59623684929b.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - Update create provisioning ports logic to fail only + when no neutron ports were created. If we created at + least one neutron port, proceed with the deployment. + It was the default behaviour for flat scenario.