From f7dd7790f5c6e3149af4680ba521089328d1eb0c Mon Sep 17 00:00:00 2001 From: elajkat Date: Fri, 4 Nov 2022 16:51:03 +0100 Subject: [PATCH] Fix bulk create without mac Bulk port create without mac address fails as when Neutron calls oslo_utils.netutils.get_ipv6_addr_by_EUI64, as the mac field of the port is an ATTR_NOT_SPECIFIED Sentinel() object. With some reshuffling of the code to fill the mac field this can be fixed. Closes-Bug: #1995732 Related-Bug: #1954763 Change-Id: Id594003681f4755d8fd1af3b98e281c3109420f6 --- neutron/plugins/ml2/plugin.py | 72 +++++++------- neutron/tests/unit/plugins/ml2/test_plugin.py | 93 +++++++++++++++++++ 2 files changed, 133 insertions(+), 32 deletions(-) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 927c69f340a..a5aa278e300 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -20,6 +20,7 @@ import netaddr from netaddr.strategy import eui48 from neutron_lib.agent import constants as agent_consts from neutron_lib.agent import topics +from neutron_lib.api import converters from neutron_lib.api.definitions import address_group as addrgrp_def from neutron_lib.api.definitions import address_scope from neutron_lib.api.definitions import agent as agent_apidef @@ -1595,11 +1596,36 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, return bound_context.current - def allocate_ips_for_ports(self, context, ports): + def allocate_macs_and_ips_for_ports(self, context, ports): + macs = self._generate_macs(len(ports)) + network_cache = dict() for port in ports: port['port']['id'] = ( port['port'].get('id') or uuidutils.generate_uuid()) + network_id = port['port'].get('network_id') + if network_id not in network_cache: + network = self.get_network(context, network_id) + network_cache[network_id] = network + + raw_mac_address = port['port'].get('mac_address', + const.ATTR_NOT_SPECIFIED) + if raw_mac_address is const.ATTR_NOT_SPECIFIED: + raw_mac_address = macs.pop() + elif self._is_mac_in_use(context, network_id, raw_mac_address): + raise exc.MacAddressInUse(net_id=network_id, + mac=raw_mac_address) + eui_mac_address = converters.convert_to_sanitized_mac_address( + raw_mac_address) + # Create the Port object + # Note: netaddr has an issue with using the correct dialect when + # the input for EUI is another EUI object, see: + # https://github.com/netaddr/netaddr/issues/250 + # TODO(lajoskatona): remove this once + # https://review.opendev.org/c/openstack/neutron-lib/+/865517 is + # released. + port['port']['mac_address'] = str(eui_mac_address) + # Call IPAM to allocate IP addresses try: port['ipams'] = self.ipam.allocate_ips_for_port(context, port) @@ -1609,7 +1635,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, except ipam_exc.DeferIpam: port['ip_allocation'] = (ipalloc_apidef. IP_ALLOCATION_DEFERRED) - return ports + return ports, network_cache @utils.transaction_guard def create_port_bulk(self, context, ports): @@ -1617,10 +1643,11 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, for port in port_list: self._before_create_port(context, port) - port_list = self.allocate_ips_for_ports(context, port_list) + port_list, net_cache = self.allocate_macs_and_ips_for_ports( + context, port_list) try: - return self._create_port_bulk(context, port_list) + return self._create_port_bulk(context, port_list, net_cache) except Exception: with excutils.save_and_reraise_exception(): # If any issue happened allocated IP addresses needs to be @@ -1630,17 +1657,16 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, context, port, port['ipams']) @db_api.retry_if_session_inactive() - def _create_port_bulk(self, context, port_list): + def _create_port_bulk(self, context, port_list, network_cache): # TODO(njohnston): Break this up into smaller functions. port_data = [] - network_cache = dict() - macs = self._generate_macs(len(port_list)) with db_api.CONTEXT_WRITER.using(context): for port in port_list: # Set up the port request dict pdata = port.get('port') project_id = pdata.get('project_id') or pdata.get('tenant_id') security_group_ids = pdata.get('security_groups') + network_id = pdata.get('network_id') if security_group_ids is const.ATTR_NOT_SPECIFIED: security_group_ids = None else: @@ -1652,38 +1678,20 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, bulk_port_data = dict( project_id=project_id, name=pdata.get('name'), - network_id=pdata.get('network_id'), + network_id=network_id, admin_state_up=pdata.get('admin_state_up'), status=pdata.get('status', const.PORT_STATUS_ACTIVE), device_id=pdata.get('device_id'), device_owner=pdata.get('device_owner'), description=pdata.get('description')) - # Ensure that the networks exist. - network_id = pdata.get('network_id') - if network_id not in network_cache: - network = self.get_network(context, network_id) - network_cache[network_id] = network - else: - network = network_cache[network_id] + network = network_cache[network_id] - # Determine the MAC address - raw_mac_address = pdata.get('mac_address', - const.ATTR_NOT_SPECIFIED) - if raw_mac_address is const.ATTR_NOT_SPECIFIED: - raw_mac_address = macs.pop() - elif self._is_mac_in_use(context, network_id, raw_mac_address): - raise exc.MacAddressInUse(net_id=network_id, - mac=raw_mac_address) - eui_mac_address = netaddr.EUI(raw_mac_address, - dialect=eui48.mac_unix_expanded) - port['port']['mac_address'] = str(eui_mac_address) - - # Create the Port object - db_port_obj = ports_obj.Port(context, - mac_address=eui_mac_address, - id=port['port']['id'], - **bulk_port_data) + db_port_obj = ports_obj.Port( + context, + mac_address=netaddr.EUI(port['port']['mac_address'], + dialect=eui48.mac_unix_expanded), + id=port['port']['id'], **bulk_port_data) db_port_obj.create() # Call IPAM to store allocated IP addresses diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 1b4a51dcfb6..b49e20e162f 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -42,6 +42,7 @@ from neutron_lib.plugins.ml2 import api as driver_api from neutron_lib.plugins import utils as p_utils from oslo_config import cfg from oslo_db import exception as db_exc +from oslo_utils import netutils from oslo_utils import uuidutils import testtools import webob @@ -50,6 +51,7 @@ from neutron._i18n import _ from neutron.agent import rpc as agent_rpc from neutron.common import utils from neutron.db import agents_db +from neutron.db import ipam_pluggable_backend from neutron.db import provisioning_blocks from neutron.db import securitygroups_db as sg_db from neutron.db import segments_db @@ -1596,6 +1598,97 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): self.assertEqual(2, deallocate_mock.call_count) + def test_create_ports_bulk_ip_allocation_without_mac(self): + ctx = context.get_admin_context() + plugin = directory.get_plugin() + fake_prefix = '2001:db8::/64' + fake_gateway = 'fe80::1' + with self.network() as net: + with self.subnet(net, + gateway_ip=fake_gateway, + cidr=fake_prefix, + ip_version=constants.IP_VERSION_6, + ipv6_ra_mode='slaac', + ipv6_address_mode='slaac') as snet_v6,\ + mock.patch.object(plugin, '_before_create_port'),\ + mock.patch.object(plugin, 'get_network') as mock_getnet,\ + mock.patch.object( + ipam_pluggable_backend.IpamPluggableBackend, + '_ipam_get_subnets') as fcs: + fcs.return_value = [snet_v6['subnet']] + mock_getnet.return_value = net['network'] + net_id = net['network']['id'] + ports = { + 'ports': [{'port': {'network_id': net_id, + 'admin_state_up': True, + 'name': 'test_0', + 'mac_address': + constants.ATTR_NOT_SPECIFIED, + 'fixed_ips': + constants.ATTR_NOT_SPECIFIED, + 'device_owner': '', + 'device_id': '', + 'security_groups': + constants.ATTR_NOT_SPECIFIED, + 'project_id': + snet_v6['subnet']['project_id'], + 'tenant_id': + snet_v6['subnet']['tenant_id']}}, + {'port': {'network_id': net_id, + 'admin_state_up': True, + 'name': 'test_1', + 'mac_address': + constants.ATTR_NOT_SPECIFIED, + 'fixed_ips': + constants.ATTR_NOT_SPECIFIED, + 'device_owner': '', + 'device_id': '', + 'security_groups': + constants.ATTR_NOT_SPECIFIED, + 'project_id': + snet_v6['subnet']['project_id'], + 'tenant_id': + snet_v6['subnet']['tenant_id']}} + ]} + b_ports = self.plugin.create_port_bulk( + ctx, ports) + self.assertEqual(len(ports['ports']), len(b_ports)) + for port in b_ports: + self.assertIsNotNone(port['fixed_ips'][0]['ip_address']) + self.assertTrue( + netutils.is_valid_ipv6( + port['fixed_ips'][0]['ip_address'])) + self.assertEqual(1, mock_getnet.call_count) + + def test_create_ports_bulk_ip_allocation_without_mac_no_net(self): + ctx = context.get_admin_context() + plugin = directory.get_plugin() + project_id = uuidutils.generate_uuid() + net_id = uuidutils.generate_uuid() + with mock.patch.object(plugin, '_before_create_port'),\ + mock.patch.object(ipam_pluggable_backend.IpamPluggableBackend, + '_ipam_get_subnets'): + ports = { + 'ports': [{'port': {'network_id': net_id, + 'admin_state_up': True, + 'name': 'test_0', + 'mac_address': + constants.ATTR_NOT_SPECIFIED, + 'fixed_ips': + constants.ATTR_NOT_SPECIFIED, + 'device_owner': '', + 'device_id': '', + 'security_groups': + constants.ATTR_NOT_SPECIFIED, + 'project_id': project_id, + 'tenant_id': project_id}} + ]} + self.assertRaises( + exc.NetworkNotFound, + self.plugin.create_port_bulk, + ctx, ports + ) + def test_delete_port_no_notify_in_disassociate_floatingips(self): ctx = context.get_admin_context() plugin = directory.get_plugin()