NetApp ONTAP: Fix use of multiple subnets with DHSS=True

NetApp ONTAP Multi-SVM driver was raising an error while trying to create shares on multiple subnets that belong to the same neutron network, as it was trying to map multiple ipspaces to the same VLAN port. This fix allows the driver to use the same ipspace and VLAN port across all subnets belonging to the same neutron network. Change-Id: If9cbb34a890ee44806c404085e40cc924a1296a7 Closes-Bug: #1774159
2018-07-12 14:48:37 -03:00 · 2018-07-12 14:48:37 -03:00 · 3e564e9252
parent 636d851437
commit 3e564e9252
7 changed files with 220 additions and 28 deletions
--- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py
+++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py
@ -937,6 +937,37 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):

        return interfaces

+    @na_utils.trace
+    def get_ipspace_name_for_vlan_port(self, vlan_node, vlan_port, vlan_id):
+        """Gets IPSpace name for specified VLAN"""
+
+        if not self.features.IPSPACES:
+            return None
+
+        port = vlan_port if not vlan_id else '%(port)s-%(id)s' % {
+            'port': vlan_port,
+            'id': vlan_id,
+        }
+        api_args = {'node': vlan_node, 'port': port}
+
+        try:
+            result = self.send_request('net-port-get', api_args)
+        except netapp_api.NaApiError as e:
+            if e.code == netapp_api.EOBJECTNOTFOUND:
+                msg = _('No pre-existing port or ipspace was found for '
+                        '%(port)s, will attempt to create one.')
+                msg_args = {'port': port}
+                LOG.debug(msg, msg_args)
+                return None
+            else:
+                raise
+
+        attributes = result.get_child_by_name('attributes')
+        net_port_info = attributes.get_child_by_name('net-port-info')
+        ipspace_name = net_port_info.get_child_content('ipspace')
+
+        return ipspace_name
+
    @na_utils.trace
    def get_ipspaces(self, ipspace_name=None):
        """Gets one or more IPSpaces."""
--- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py
+++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py
@ -159,7 +159,14 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
            msg = _('Vserver %s already exists.')
            raise exception.NetAppException(msg % vserver_name)

-        ipspace_name = self._create_ipspace(network_info)
+        # NOTE(lseki): If there's already an ipspace created for the same VLAN
+        # port, reuse it. It will be named after the previously created share
+        # server's neutron subnet id.
+        node_name = self._client.list_cluster_nodes()[0]
+        port = self._get_node_data_port(node_name)
+        vlan = network_info['segmentation_id']
+        ipspace_name = self._client.get_ipspace_name_for_vlan_port(
+            node_name, port, vlan) or self._create_ipspace(network_info)

        LOG.debug('Vserver %s does not exist, creating.', vserver_name)
        self._client.create_vserver(
@ -222,8 +229,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
            return client_cmode.DEFAULT_IPSPACE

        ipspace_name = self._get_valid_ipspace_name(ipspace_id)
-        if not self._client.ipspace_exists(ipspace_name):
-            self._client.create_ipspace(ipspace_name)
+        self._client.create_ipspace(ipspace_name)

        return ipspace_name

--- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py
+++ b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py
@ -469,6 +469,74 @@ SECUTITY_KEY_MANAGER_NVE_SUPPORT_RESPONSE_FALSE = etree.XML("""
  </results>
 """)

+NET_PORT_GET_RESPONSE_NO_VLAN = etree.XML("""
+  <results status="passed">
+    <attributes>
+      <net-port-info>
+        <administrative-duplex>auto</administrative-duplex>
+        <administrative-flowcontrol>full</administrative-flowcontrol>
+        <administrative-speed>auto</administrative-speed>
+        <broadcast-domain>%(domain)s</broadcast-domain>
+        <health-status>healthy</health-status>
+        <ignore-health-status>false</ignore-health-status>
+        <ipspace>%(ipspace)s</ipspace>
+        <is-administrative-auto-negotiate>true</is-administrative-auto-negotiate>
+        <is-administrative-up>true</is-administrative-up>
+        <is-operational-auto-negotiate>true</is-operational-auto-negotiate>
+        <link-status>up</link-status>
+        <mac-address>00:0c:29:fc:04:f7</mac-address>
+        <mtu>1500</mtu>
+        <mtu-admin>1500</mtu-admin>
+        <node>%(node_name)s</node>
+        <operational-duplex>full</operational-duplex>
+        <operational-flowcontrol>receive</operational-flowcontrol>
+        <operational-speed>1000</operational-speed>
+        <port>%(port)s</port>
+        <port-type>physical</port-type>
+        <role>data</role>
+      </net-port-info>
+    </attributes>
+  </results>
+""" % {'domain': BROADCAST_DOMAIN,
+       'ipspace': IPSPACE_NAME,
+       'node_name': NODE_NAME,
+       'port': PORT})
+
+NET_PORT_GET_RESPONSE = etree.XML("""
+  <results status="passed">
+    <attributes>
+      <net-port-info>
+        <administrative-duplex>auto</administrative-duplex>
+        <administrative-flowcontrol>full</administrative-flowcontrol>
+        <administrative-speed>auto</administrative-speed>
+        <health-status>healthy</health-status>
+        <ignore-health-status>false</ignore-health-status>
+        <ipspace>%(ipspace)s</ipspace>
+        <is-administrative-auto-negotiate>true</is-administrative-auto-negotiate>
+        <is-administrative-up>true</is-administrative-up>
+        <is-operational-auto-negotiate>true</is-operational-auto-negotiate>
+        <link-status>up</link-status>
+        <mac-address>00:0c:29:fc:04:f7</mac-address>
+        <mtu>1500</mtu>
+        <mtu-admin>1500</mtu-admin>
+        <node>%(node_name)s</node>
+        <operational-duplex>full</operational-duplex>
+        <operational-flowcontrol>receive</operational-flowcontrol>
+        <operational-speed>1000</operational-speed>
+        <port>%(port)s-%(vlan)s</port>
+        <port-type>vlan</port-type>
+        <role>data</role>
+        <vlan-id>%(vlan)s</vlan-id>
+        <vlan-node>%(node_name)s</vlan-node>
+        <vlan-port>%(port)s</vlan-port>
+      </net-port-info>
+    </attributes>
+  </results>
+""" % {'ipspace': IPSPACE_NAME,
+       'node_name': NODE_NAME,
+       'port': PORT,
+       'vlan': VLAN})
+
 NET_PORT_GET_ITER_RESPONSE = etree.XML("""
  <results status="passed">
    <attributes-list>
--- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py
+++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py
@ -1849,6 +1849,68 @@ class NetAppClientCmodeTestCase(test.TestCase):
        self.client.send_request.assert_has_calls([
            mock.call('net-ipspaces-destroy', net_ipspaces_destroy_args)])

+    def test_get_ipspace_name_for_vlan_port(self):
+        self.client.features.add_feature('IPSPACES')
+        api_response = netapp_api.NaElement(fake.NET_PORT_GET_RESPONSE)
+        self.mock_object(self.client,
+                         'send_request',
+                         mock.Mock(return_value=api_response))
+
+        ipspace = self.client.get_ipspace_name_for_vlan_port(
+            fake.NODE_NAME, fake.PORT, fake.VLAN)
+
+        port = '%(port)s-%(id)s' % {'port': fake.PORT, 'id': fake.VLAN}
+        self.client.send_request.assert_called_once_with(
+            'net-port-get',
+            {'node': fake.NODE_NAME, 'port': port})
+        self.assertEqual(fake.IPSPACE_NAME, ipspace)
+
+    def test_get_ipspace_name_for_vlan_port_no_ipspace_feature(self):
+        self.mock_object(self.client, 'send_request')
+
+        ipspace = self.client.get_ipspace_name_for_vlan_port(
+            fake.NODE_NAME, fake.PORT, fake.VLAN)
+
+        self.client.send_request.assert_not_called()
+        self.assertIsNone(ipspace)
+
+    def test_get_ipspace_name_for_vlan_port_no_ipspace_found(self):
+        self.client.features.add_feature('IPSPACES')
+        self.mock_object(
+            self.client,
+            'send_request',
+            self._mock_api_error(code=netapp_api.EOBJECTNOTFOUND))
+
+        ipspace = self.client.get_ipspace_name_for_vlan_port(
+            fake.NODE_NAME, fake.PORT, fake.VLAN)
+
+        self.assertIsNone(ipspace)
+
+    def test_get_ipspace_name_for_vlan_port_no_vlan(self):
+        self.client.features.add_feature('IPSPACES')
+        api_response = netapp_api.NaElement(fake.NET_PORT_GET_RESPONSE_NO_VLAN)
+        self.mock_object(self.client,
+                         'send_request',
+                         mock.Mock(return_value=api_response))
+
+        ipspace = self.client.get_ipspace_name_for_vlan_port(
+            fake.NODE_NAME, fake.PORT, None)
+
+        self.client.send_request.assert_called_once_with(
+            'net-port-get',
+            {'node': fake.NODE_NAME, 'port': fake.PORT})
+        self.assertEqual(fake.IPSPACE_NAME, ipspace)
+
+    def test_get_ipspace_name_for_vlan_port_raises_api_error(self):
+        self.client.features.add_feature('IPSPACES')
+        self.mock_object(self.client,
+                         'send_request',
+                         mock.Mock(side_effect=self._mock_api_error()))
+
+        self.assertRaises(netapp_api.NaApiError,
+                          self.client.get_ipspace_name_for_vlan_port,
+                          fake.NODE_NAME, fake.VLAN_PORT, None)
+
    def test_add_vserver_to_ipspace(self):

        self.mock_object(self.client, 'send_request')
--- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py
+++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py
@ -288,7 +288,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):

        self.assertEqual(vserver_name, actual_result)

-    def test_create_vserver(self):
+    @ddt.data(None, fake.IPSPACE)
+    def test_create_vserver(self, existing_ipspace):

        versions = ['fake_v1', 'fake_v2']
        self.library.configuration.netapp_enabled_share_protocols = versions
@ -296,6 +297,12 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
        vserver_name = fake.VSERVER_NAME_TEMPLATE % vserver_id
        vserver_client = mock.Mock()

+        self.mock_object(self.library._client,
+                         'list_cluster_nodes',
+                         mock.Mock(return_value=fake.CLUSTER_NODES))
+        self.mock_object(self.library,
+                         '_get_node_data_port',
+                         mock.Mock(return_value='fake_port'))
        self.mock_object(context,
                         'get_admin_context',
                         mock.Mock(return_value='fake_admin_context'))
@ -311,13 +318,23 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
        self.mock_object(self.library,
                         '_create_ipspace',
                         mock.Mock(return_value=fake.IPSPACE))
+        get_ipspace_name_for_vlan_port = self.mock_object(
+            self.library._client,
+            'get_ipspace_name_for_vlan_port',
+            mock.Mock(return_value=existing_ipspace))
        self.mock_object(self.library, '_create_vserver_lifs')
        self.mock_object(self.library, '_create_vserver_admin_lif')
        self.mock_object(self.library, '_create_vserver_routes')

        self.library._create_vserver(vserver_name, fake.NETWORK_INFO)

-        self.library._create_ipspace.assert_called_once_with(fake.NETWORK_INFO)
+        get_ipspace_name_for_vlan_port.assert_called_once_with(
+            fake.CLUSTER_NODES[0],
+            'fake_port',
+            fake.NETWORK_INFO['segmentation_id'])
+        if not existing_ipspace:
+            self.library._create_ipspace.assert_called_once_with(
+                fake.NETWORK_INFO)
        self.library._client.create_vserver.assert_called_once_with(
            vserver_name, fake.ROOT_VOLUME_AGGREGATE, fake.ROOT_VOLUME,
            fake.AGGREGATES, fake.IPSPACE)
@ -351,13 +368,30 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
                          vserver_name,
                          fake.NETWORK_INFO)

-    @ddt.data(netapp_api.NaApiError, exception.NetAppException)
-    def test_create_vserver_lif_creation_failure(self, lif_exception):
+    @ddt.data(
+        {'lif_exception': netapp_api.NaApiError,
+         'existing_ipspace': fake.IPSPACE},
+        {'lif_exception': netapp_api.NaApiError,
+         'existing_ipspace': None},
+        {'lif_exception': exception.NetAppException,
+         'existing_ipspace': None},
+        {'lif_exception': exception.NetAppException,
+         'existing_ipspace': fake.IPSPACE})
+    @ddt.unpack
+    def test_create_vserver_lif_creation_failure(self,
+                                                 lif_exception,
+                                                 existing_ipspace):

        vserver_id = fake.NETWORK_INFO['server_id']
        vserver_name = fake.VSERVER_NAME_TEMPLATE % vserver_id
        vserver_client = mock.Mock()

+        self.mock_object(self.library._client,
+                         'list_cluster_nodes',
+                         mock.Mock(return_value=fake.CLUSTER_NODES))
+        self.mock_object(self.library,
+                         '_get_node_data_port',
+                         mock.Mock(return_value='fake_port'))
        self.mock_object(context,
                         'get_admin_context',
                         mock.Mock(return_value='fake_admin_context'))
@ -370,6 +404,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
        self.mock_object(self.library,
                         '_find_matching_aggregates',
                         mock.Mock(return_value=fake.AGGREGATES))
+        self.mock_object(self.library._client,
+                         'get_ipspace_name_for_vlan_port',
+                         mock.Mock(return_value=existing_ipspace))
        self.mock_object(self.library,
                         '_create_ipspace',
                         mock.Mock(return_value=fake.IPSPACE))
@ -423,27 +460,11 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):

        self.assertEqual('Default', result)

-    def test_create_ipspace_already_present(self):
-
-        self.library._client.features.IPSPACES = True
-        self.mock_object(self.library._client,
-                         'ipspace_exists',
-                         mock.Mock(return_value=True))
-
-        result = self.library._create_ipspace(fake.NETWORK_INFO)
-
-        expected = self.library._get_valid_ipspace_name(
-            fake.NETWORK_INFO['neutron_subnet_id'])
-        self.assertEqual(expected, result)
-        self.library._client.ipspace_exists.assert_has_calls([
-            mock.call(expected)])
-        self.assertFalse(self.library._client.create_ipspace.called)
-
    def test_create_ipspace(self):

        self.library._client.features.IPSPACES = True
        self.mock_object(self.library._client,
-                         'ipspace_exists',
+                         'create_ipspace',
                         mock.Mock(return_value=False))

        result = self.library._create_ipspace(fake.NETWORK_INFO)
@ -451,10 +472,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
        expected = self.library._get_valid_ipspace_name(
            fake.NETWORK_INFO['neutron_subnet_id'])
        self.assertEqual(expected, result)
-        self.library._client.ipspace_exists.assert_has_calls([
-            mock.call(expected)])
-        self.library._client.create_ipspace.assert_has_calls([
-            mock.call(expected)])
+        self.library._client.create_ipspace.assert_called_once_with(expected)

    def test_create_vserver_lifs(self):

--- a/manila/tests/share/drivers/netapp/dataontap/fakes.py
+++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py
@ -349,6 +349,7 @@ NETWORK_INFO = {
    'security_services': ['fake_ldap', 'fake_kerberos', 'fake_ad', ],
    'network_allocations': USER_NETWORK_ALLOCATIONS,
    'admin_network_allocations': ADMIN_NETWORK_ALLOCATIONS,
+    'neutron_net_id': '4eff22ca-5ad2-454d-a000-aadfd7b40b39',
    'neutron_subnet_id': '62bf1c2c-18eb-421b-8983-48a6d39aafe0',
    'segmentation_id': '1000',
 }
--- a/releasenotes/notes/bug-1774159-0afe3dbc39e3c6b0.yaml
+++ b/releasenotes/notes/bug-1774159-0afe3dbc39e3c6b0.yaml
@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The NetApp ONTAP DHSS=True driver has been fixed to allow multiple shares
+    to use the same ipspace and VLAN port across all subnets belonging to the
+    same neutron network.