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

2018-11-19 20:31:55 +00:00 · 2018-11-19 20:31:55 +00:00 · f22e0d2361
parent 20a2baac13 3e564e9252
commit f22e0d2361
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.