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.

Conflicts:
  neutron/plugins/ml2/plugin.py

Closes-Bug: #1995732
Related-Bug: #1954763

Change-Id: Id594003681f4755d8fd1af3b98e281c3109420f6
(cherry picked from commit f7dd779)

Author: elajkat

neutron/plugins/ml2/plugin.py

@@ -20,6 +20,7 @@
 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
@@ -1563,11 +1564,36 @@ def _after_create_port(self, context, result, mech_context):
 
         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)
@@ -1577,18 +1603,19 @@ def allocate_ips_for_ports(self, context, ports):
             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):
         port_list = ports.get('ports')
         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
@@ -1598,17 +1625,16 @@ def create_port_bulk(self, context, ports):
                         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:
@@ -1620,39 +1646,21 @@ def _create_port_bulk(self, context, port_list):
                 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]
-
-                # 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)
+                network = network_cache[network_id]
+
+                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

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -42,6 +42,7 @@
 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.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
@@ -1595,6 +1597,97 @@ def test_create_ports_bulk_ip_allocation_reverted_in_case_of_error(self):
 
                 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()