NetApp: set real ifgroup MAC on Neutron ports for consistency

manila/exception.py

@@ -763,6 +763,10 @@ class IPAddressInUse(InUse):
     message = _("IP address %(ip)s is already used.")
 
 
+class MacAddressInUse(InUse):
+    message = _("MAC address %(mac)s is already in use.")
+
+
 class ShareGroupTypeInUse(ManilaException):
     message = _("Share group Type %(type_id)s deletion is not allowed "
                 "with groups present with the type.")

manila/network/neutron/api.py

@@ -72,13 +72,18 @@ class PortBindingAlreadyExistsClient(neutron_client_exc.Conflict):
     pass
 
 
+class MacAddressInUseClient(neutron_client_exc.Conflict):
+    pass
+
+
 # We need to monkey-patch neutronclient.common.exceptions module, to make
 # neutron client to raise error specific exceptions. E.g. exception
 # PortBindingAlreadyExistsClient is raised for Neutron API error
 # PortBindingAlreadyExists. If not defined, a general exception of type
 # Conflict will be raised.
 neutron_client_exc.PortBindingAlreadyExistsClient = \
     PortBindingAlreadyExistsClient
+neutron_client_exc.MacAddressInUseClient = MacAddressInUseClient
 
 
 def list_opts():
@@ -197,6 +202,8 @@ def _create_port(self, tenant_id, network_id, host_id=None, subnet_id=None,
         except neutron_client_exc.NeutronClientException as e:
             LOG.warning('Neutron error creating port on network %s',
                         network_id)
+            if e.status_code == 409 and 'mac' in str(e.message).lower():
+                raise exception.MacAddressInUse(mac=e.message)
             if e.status_code == 409:
                 raise exception.PortLimitExceeded()
             raise exception.NetworkException(code=e.status_code,

manila/network/neutron/neutron_network_plugin.py

@@ -165,6 +165,7 @@ def allocate_network(self, context, share_server, share_network=None,
 
         allocation_count = kwargs.get('count', 1)
         device_owner = kwargs.get('device_owner', 'share')
+        mac_address = kwargs.get('mac_address')
 
         ports = []
         for current_count in range(0, allocation_count):
@@ -175,7 +176,8 @@ def allocate_network(self, context, share_server, share_network=None,
                                   share_network_subnet,
                                   device_owner,
                                   current_count,
-                                  is_external_network=is_external_network),
+                                  is_external_network=is_external_network,
+                                  mac_address=mac_address),
             )
 
         return ports
@@ -348,8 +350,8 @@ def deallocate_network(self, context, share_server_id,
 
     def _get_port_create_args(self, share_server, share_network_subnet,
                               device_owner, count=0,
-                              is_external_network=False):
-        return {
+                              is_external_network=False, mac_address=None):
+        args = {
             "network_id": share_network_subnet['neutron_net_id'],
             "subnet_id": share_network_subnet['neutron_subnet_id'],
             "device_owner": 'manila:' + device_owner,
@@ -360,16 +362,32 @@ def _get_port_create_args(self, share_server, share_network_subnet,
             # neutron creates merely assist in IPAM.
             "admin_state_up": not is_external_network,
         }
+        if mac_address:
+            args["mac_address"] = mac_address
+        return args
 
     def _create_port(self, context, share_server, share_network,
                      share_network_subnet, device_owner, count=0,
-                     is_external_network=False):
+                     is_external_network=False, mac_address=None):
         create_args = self._get_port_create_args(
             share_server, share_network_subnet, device_owner, count,
-            is_external_network=is_external_network)
+            is_external_network=is_external_network,
+            mac_address=mac_address)
 
-        port = self.neutron_api.create_port(
-            share_network['project_id'], **create_args)
+        try:
+            port = self.neutron_api.create_port(
+                share_network['project_id'], **create_args)
+        except exception.MacAddressInUse:
+            LOG.warning('MAC %s already in use on network %s; retrying '
+                        'without MAC (overflow port — real wire MAC recorded '
+                        'in port description).',
+                        mac_address,
+                        share_network_subnet['neutron_net_id'])
+            create_args.pop('mac_address', None)
+            create_args['description'] = (
+                'NetApp LIF; effective wire MAC=%s' % mac_address)
+            port = self.neutron_api.create_port(
+                share_network['project_id'], **create_args)
 
         if is_external_network:
             msg = (
@@ -639,11 +657,12 @@ def _wait_for_ports_bind(self, ports, share_server):
 
     def _get_port_create_args(self, share_server, share_network_subnet,
                               device_owner, count=0,
-                              is_external_network=False):
+                              is_external_network=False, mac_address=None):
         arguments = super(
             NeutronBindNetworkPlugin, self)._get_port_create_args(
             share_server, share_network_subnet, device_owner, count,
-            is_external_network=is_external_network
+            is_external_network=is_external_network,
+            mac_address=mac_address
         )
         arguments['host_id'] = self.config.neutron_host_id
         arguments['binding:vnic_type'] = self.config.neutron_vnic_type

manila/share/drivers/netapp/dataontap/client/client_cmode.py

@@ -757,6 +757,7 @@ def get_node_data_ports(self, node):
                     'node': None,
                     'operational-speed': None,
                     'ifgrp-port': None,
+                    'mac-address': None,
                 },
             },
         }
@@ -775,6 +776,7 @@ def get_node_data_ports(self, node):
                 'node': port_info.get_child_content('node'),
                 'port': port_info.get_child_content('port'),
                 'speed': port_info.get_child_content('operational-speed'),
+                'mac-address': port_info.get_child_content('mac-address'),
             }
             ports.append(port)
 

manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py

@@ -5141,7 +5141,7 @@ def get_node_data_ports(self, node):
             'state': 'up',
             'type': 'physical',
             'broadcast_domain.name': 'Default',
-            'fields': 'node.name,speed,name'
+            'fields': 'node.name,speed,name,mac_address'
         }
 
         result = self.send_request('/network/ethernet/ports', 'get',
@@ -5174,6 +5174,7 @@ def get_node_data_ports(self, node):
                         'node': port_info['node']['name'],
                         'port': port_info['name'],
                         'speed': port_info['speed'],
+                        'mac-address': port_info.get('mac_address'),
                     }
                     ports.append(port)
 

manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py

@@ -139,6 +139,17 @@ def get_share_server_pools(self, share_server):
     def get_network_allocations_number(self):
         return self.library.get_network_allocations_number()
 
+    def allocate_network(self, context, share_server, share_network,
+                         share_network_subnet, **kwargs):
+        """Allocate network, injecting the ifgroup MAC for the first port."""
+
+        mac = self.library.get_node_mac_for_network_allocation()
+        if mac:
+            kwargs.setdefault('mac_address', mac)
+        return super().allocate_network(
+            context, share_server, share_network, share_network_subnet,
+            **kwargs)
+
     def get_admin_network_allocations_number(self):
         return self.library.get_admin_network_allocations_number(
             self.admin_network_api)

manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py

@@ -123,6 +123,12 @@ def get_share_server_pools(self, share_server):
     def get_network_allocations_number(self):
         return self.library.get_network_allocations_number()
 
+    def allocate_network(self, context, share_server, share_network,
+                         share_network_subnet, **kwargs):
+        return super().allocate_network(
+            context, share_server, share_network, share_network_subnet,
+            **kwargs)
+
     def get_admin_network_allocations_number(self):
         return self.library.get_admin_network_allocations_number()
 

manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py

@@ -29,6 +29,8 @@
 from oslo_utils import uuidutils
 
 from manila.common import constants
+from manila import context as manila_context
+from manila import db
 from manila import exception
 from manila.i18n import _
 from manila.message import message_field
@@ -534,6 +536,25 @@ def _create_vserver_lifs(self, vserver_name, vserver_client, network_info,
                              node_name, lif_name, network_allocation,
                              lif_home_port=lif_home_port)
 
+            # Sync the real LIF MAC (inherited from the node's physical port)
+            self._sync_lif_mac_to_db(node_name, network_allocation)
+
+    @na_utils.trace
+    def _sync_lif_mac_to_db(self, node_name, network_allocation):
+        """Update network_allocations.mac_address with the LIF's real MAC."""
+
+        node_mac = self._get_node_data_port_mac(node_name)
+        if not node_mac:
+            LOG.warning('Could not retrieve MAC for node %s; leaving '
+                        'network_allocation %s untouched.',
+                        node_name, network_allocation['id'])
+            return
+        admin_ctx = manila_context.get_admin_context()
+        db.network_allocation_update(
+            admin_ctx, network_allocation['id'], {'mac_address': node_mac})
+        LOG.info('Synced LIF MAC for node %s: allocation %s updated to %s.',
+                 node_name, network_allocation['id'], node_mac)
+
     @na_utils.trace
     def _create_vserver_admin_lif(self, vserver_name, vserver_client,
                                   network_info, ipspace_name,
@@ -558,6 +579,8 @@ def _create_vserver_admin_lif(self, vserver_name, vserver_client,
                          node_name, lif_name, network_allocation,
                          lif_home_port=home_port)
 
+        self._sync_lif_mac_to_db(node_name, network_allocation)
+
     @na_utils.trace
     def _create_vserver_routes(self, vserver_client, network_info):
         """Create Vserver route and set gateways."""
@@ -584,6 +607,17 @@ def _get_node_data_port(self, node):
                   'to create Vserver LIFs.') % node)
         return matched_port_names[0]
 
+    @na_utils.trace
+    def _get_node_data_port_mac(self, node):
+        """Return MAC address of the data port selected for LIF creation."""
+
+        pattern = self.configuration.netapp_port_name_search_pattern
+        ports = self._client.get_node_data_ports(node)
+        matched = [p for p in ports if re.match(pattern, p.get('port', ''))]
+        if matched:
+            return matched[0].get('mac-address')
+        return None
+
     def _get_lif_name(self, node_name, network_allocation):
         """Get LIF name based on template from manila.conf file."""
         lif_name_args = {
@@ -647,6 +681,15 @@ def get_network_allocations_number(self):
         """Get number of network interfaces to be created."""
         return len(self._client.list_cluster_nodes())
 
+    @na_utils.trace
+    def get_node_mac_for_network_allocation(self):
+        """Return the ifgroup MAC of the first cluster node."""
+
+        nodes = self._client.list_cluster_nodes()
+        if nodes:
+            return self._get_node_data_port_mac(nodes[0])
+        return None
+
     @na_utils.trace
     def get_admin_network_allocations_number(self, admin_network_api):
         """Get number of network allocations for creating admin LIFs."""

manila/tests/network/neutron/test_neutron_api.py

@@ -276,6 +276,36 @@ def test_create_port_exception_status_409(self):
         self.assertTrue(clientv20.Client.called)
         self.assertTrue(self.neutron_api.client.create_port.called)
 
+    @mock.patch.object(neutron_api.LOG, 'warning', mock.Mock())
+    def test_create_port_exception_status_409_mac_in_use(self):
+        self.mock_object(
+            self.neutron_api.client, 'create_port',
+            mock.Mock(side_effect=neutron_client_exc.NeutronClientException(
+                status_code=409,
+                message='The mac address fa:16:3e:xx is in use.')))
+        port_args = {'tenant_id': 'test tenant', 'network_id': 'test net'}
+
+        self.assertRaises(exception.MacAddressInUse,
+                          self.neutron_api.create_port,
+                          **port_args)
+
+        self.assertTrue(neutron_api.LOG.warning.called)
+
+    @mock.patch.object(neutron_api.LOG, 'warning', mock.Mock())
+    def test_create_port_exception_status_409_quota_not_mac(self):
+        self.mock_object(
+            self.neutron_api.client, 'create_port',
+            mock.Mock(side_effect=neutron_client_exc.NeutronClientException(
+                status_code=409,
+                message='Quota exceeded for resources: [\'port\']')))
+        port_args = {'tenant_id': 'test tenant', 'network_id': 'test net'}
+
+        self.assertRaises(exception.PortLimitExceeded,
+                          self.neutron_api.create_port,
+                          **port_args)
+
+        self.assertTrue(neutron_api.LOG.warning.called)
+
     def test_delete_port(self):
         # Set up test data
         self.mock_object(self.neutron_api.client, 'delete_port')

manila/tests/network/neutron/test_neutron_plugin.py

@@ -416,6 +416,77 @@ def test_allocate_network_create_port_exception(self):
         save_subnet_data.stop()
         create_port.stop()
 
+    @mock.patch.object(db_api, 'network_allocation_create',
+                       mock.Mock(return_values=fake_network_allocation))
+    @mock.patch.object(db_api, 'share_network_get',
+                       mock.Mock(return_value=fake_share_network))
+    @mock.patch.object(db_api, 'share_server_get',
+                       mock.Mock(return_value=fake_share_server))
+    def test_allocate_network_with_mac_address(self):
+        fake_mac = 'd2:39:ea:ac:06:9c'
+        has_provider_nw_ext = mock.patch.object(
+            self.plugin, '_has_provider_network_extension').start()
+        has_provider_nw_ext.return_value = True
+        mock.patch.object(self.plugin, '_save_neutron_network_data').start()
+        mock.patch.object(self.plugin, '_save_neutron_subnet_data').start()
+
+        with mock.patch.object(self.plugin.neutron_api, 'create_port',
+                               mock.Mock(return_value=fake_neutron_port)):
+            self.plugin.allocate_network(
+                self.fake_context,
+                fake_share_server,
+                fake_share_network,
+                fake_share_network_subnet,
+                mac_address=fake_mac)
+
+            self.plugin.neutron_api.create_port.assert_called_once_with(
+                fake_share_network['project_id'],
+                network_id=fake_share_network_subnet['neutron_net_id'],
+                subnet_id=fake_share_network_subnet['neutron_subnet_id'],
+                device_owner='manila:share',
+                device_id=fake_share_network['id'],
+                name=fake_share_network['id'] + '_0',
+                admin_state_up=False,
+                mac_address=fake_mac,
+            )
+
+    @mock.patch.object(db_api, 'network_allocation_create',
+                       mock.Mock(return_values=fake_network_allocation))
+    @mock.patch.object(db_api, 'share_network_get',
+                       mock.Mock(return_value=fake_share_network))
+    @mock.patch.object(db_api, 'share_server_get',
+                       mock.Mock(return_value=fake_share_server))
+    @mock.patch.object(plugin.LOG, 'warning', mock.Mock())
+    def test_allocate_network_mac_in_use_retries_without_mac(self):
+        fake_mac = 'd2:39:ea:ac:06:9c'
+        has_provider_nw_ext = mock.patch.object(
+            self.plugin, '_has_provider_network_extension').start()
+        has_provider_nw_ext.return_value = True
+        mock.patch.object(self.plugin, '_save_neutron_network_data').start()
+        mock.patch.object(self.plugin, '_save_neutron_subnet_data').start()
+
+        create_port = mock.patch.object(
+            self.plugin.neutron_api, 'create_port',
+            mock.Mock(side_effect=[
+                exception.MacAddressInUse(mac=fake_mac),
+                fake_neutron_port,
+            ])).start()
+
+        self.plugin.allocate_network(
+            self.fake_context,
+            fake_share_server,
+            fake_share_network,
+            fake_share_network_subnet,
+            mac_address=fake_mac)
+
+        self.assertEqual(2, create_port.call_count)
+        _, second_kwargs = create_port.call_args
+
+        self.assertNotIn('mac_address', second_kwargs)
+        self.assertIn('description', second_kwargs)
+        self.assertIn(fake_mac, second_kwargs['description'])
+        plugin.LOG.warning.assert_called()
+
     def _setup_manage_network_allocations(self):
 
         allocations = ['192.168.0.11', '192.168.0.12', 'fd12::2000']