NetApp fix to set non default server port in api

The non default netapp_server_port config option was not
getting set in api even if specified in cinder.conf. Its
made non mandatory and set if specified in the configuration.

Change-Id: I62943427e0caac2742cce4ce56c1a49917f9210d
Closes-bug: #1322379

cinder/tests/test_netapp.py

@@ -571,13 +571,58 @@ class NetAppDirectCmodeISCSIDriverTestCase(test.TestCase):
         configuration.netapp_password = 'pass'
         configuration.netapp_server_hostname = '127.0.0.1'
         configuration.netapp_transport_type = 'http'
-        configuration.netapp_server_port = '80'
+        configuration.netapp_server_port = None
         configuration.netapp_vserver = 'openstack'
         return configuration
 
     def test_connect(self):
         self.driver.check_for_setup_error()
 
+    def test_do_setup_all_default(self):
+        configuration = self._set_config(create_configuration())
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('80', driver.client.get_port())
+        self.assertEqual('http', driver.client.get_transport_type())
+
+    def test_do_setup_http_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'http'
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('80', driver.client.get_port())
+        self.assertEqual('http', driver.client.get_transport_type())
+
+    def test_do_setup_https_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'https'
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('443', driver.client.get_port())
+        self.assertEqual('https', driver.client.get_transport_type())
+
+    def test_do_setup_http_non_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_server_port = 81
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('81', driver.client.get_port())
+        self.assertEqual('http', driver.client.get_transport_type())
+
+    def test_do_setup_https_non_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'https'
+        configuration.netapp_server_port = 446
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('446', driver.client.get_port())
+        self.assertEqual('https', driver.client.get_transport_type())
+
     def test_create_destroy(self):
         self.driver.create_volume(self.volume)
         self.driver.delete_volume(self.volume)
@@ -1158,7 +1203,7 @@ class NetAppDirect7modeISCSIDriverTestCase_NV(
         configuration.netapp_password = 'pass'
         configuration.netapp_server_hostname = '127.0.0.1'
         configuration.netapp_transport_type = 'http'
-        configuration.netapp_server_port = '80'
+        configuration.netapp_server_port = None
         return configuration
 
     def test_create_on_select_vol(self):
@@ -1208,7 +1253,7 @@ class NetAppDirect7modeISCSIDriverTestCase_WV(
         configuration.netapp_password = 'pass'
         configuration.netapp_server_hostname = '127.0.0.1'
         configuration.netapp_transport_type = 'http'
-        configuration.netapp_server_port = '80'
+        configuration.netapp_server_port = None
         configuration.netapp_vfiler = 'openstack'
         return configuration
 

cinder/tests/test_netapp_eseries_iscsi.py

@@ -22,6 +22,7 @@ import re
 
 import mock
 import requests
+import six.moves.urllib.parse as urlparse
 
 from cinder import exception
 from cinder.openstack.common import log as logging
@@ -649,7 +650,7 @@ class NetAppEseriesIscsiDriverTestCase(test.TestCase):
         configuration.netapp_storage_protocol = 'iscsi'
         configuration.netapp_transport_type = 'http'
         configuration.netapp_server_hostname = '127.0.0.1'
-        configuration.netapp_server_port = '80'
+        configuration.netapp_server_port = None
         configuration.netapp_webservice_path = '/devmgr/vn'
         configuration.netapp_controller_ips = '127.0.0.2,127.0.0.3'
         configuration.netapp_sa_password = 'pass1234'
@@ -934,3 +935,63 @@ class NetAppEseriesIscsiDriverTestCase(test.TestCase):
         driver = common.NetAppDriver(configuration=configuration)
         self.assertRaises(exception.NetAppDriverException,
                           driver.check_for_setup_error)
+
+    def test_do_setup_all_default(self):
+        configuration = self._set_config(create_configuration())
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._check_mode_get_or_register_storage_system = mock.Mock()
+        driver.do_setup(context='context')
+        url = urlparse.urlparse(driver._client._endpoint)
+        port = url.port
+        scheme = url.scheme
+        self.assertEqual(8080, port)
+        self.assertEqual('http', scheme)
+
+    def test_do_setup_http_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'http'
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._check_mode_get_or_register_storage_system = mock.Mock()
+        driver.do_setup(context='context')
+        url = urlparse.urlparse(driver._client._endpoint)
+        port = url.port
+        scheme = url.scheme
+        self.assertEqual(8080, port)
+        self.assertEqual('http', scheme)
+
+    def test_do_setup_https_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'https'
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._check_mode_get_or_register_storage_system = mock.Mock()
+        driver.do_setup(context='context')
+        url = urlparse.urlparse(driver._client._endpoint)
+        port = url.port
+        scheme = url.scheme
+        self.assertEqual(8443, port)
+        self.assertEqual('https', scheme)
+
+    def test_do_setup_http_non_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_server_port = 81
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._check_mode_get_or_register_storage_system = mock.Mock()
+        driver.do_setup(context='context')
+        url = urlparse.urlparse(driver._client._endpoint)
+        port = url.port
+        scheme = url.scheme
+        self.assertEqual(81, port)
+        self.assertEqual('http', scheme)
+
+    def test_do_setup_https_non_default_port(self):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'https'
+        configuration.netapp_server_port = 446
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._check_mode_get_or_register_storage_system = mock.Mock()
+        driver.do_setup(context='context')
+        url = urlparse.urlparse(driver._client._endpoint)
+        port = url.port
+        scheme = url.scheme
+        self.assertEqual(446, port)
+        self.assertEqual('https', scheme)

cinder/tests/test_netapp_nfs.py

@@ -31,6 +31,7 @@ from cinder.openstack.common import log as logging
 from cinder import test
 from cinder.volume import configuration as conf
 from cinder.volume.drivers.netapp import api
+from cinder.volume.drivers.netapp import common
 from cinder.volume.drivers.netapp import nfs as netapp_nfs
 from cinder.volume.drivers.netapp import utils
 
@@ -188,11 +189,9 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
         mox = self.mox
         drv = self._driver
         required_flags = [
-            'netapp_transport_type',
             'netapp_login',
             'netapp_password',
-            'netapp_server_hostname',
+            'netapp_server_hostname']
-            'netapp_server_port']
 
         # set required flags
         for flag in required_flags:
@@ -803,6 +802,68 @@ class NetappDirectCmodeNfsDriverTestCase(test.TestCase):
         pool = self._driver.get_pool({'provider_location': 'fake-share'})
         self.assertEqual(pool, 'fake-share')
 
+    def _set_config(self, configuration):
+        configuration.netapp_storage_family = 'ontap_cluster'
+        configuration.netapp_storage_protocol = 'nfs'
+        configuration.netapp_login = 'admin'
+        configuration.netapp_password = 'pass'
+        configuration.netapp_server_hostname = '127.0.0.1'
+        configuration.netapp_transport_type = 'http'
+        configuration.netapp_server_port = None
+        configuration.netapp_vserver = 'openstack'
+        configuration.nfs_shares_config = '/nfs'
+        return configuration
+
+    @mock.patch.object(netapp_nfs.NetAppNFSDriver, 'do_setup')
+    def test_do_setup_all_default(self, mock_set_up):
+        configuration = self._set_config(create_configuration())
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('80', driver._client.get_port())
+        self.assertEqual('http', driver._client.get_transport_type())
+
+    @mock.patch.object(netapp_nfs.NetAppNFSDriver, 'do_setup')
+    def test_do_setup_http_default_port(self, mock_setup):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'http'
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('80', driver._client.get_port())
+        self.assertEqual('http', driver._client.get_transport_type())
+
+    @mock.patch.object(netapp_nfs.NetAppNFSDriver, 'do_setup')
+    def test_do_setup_https_default_port(self, mock_setup):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'https'
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('443', driver._client.get_port())
+        self.assertEqual('https', driver._client.get_transport_type())
+
+    @mock.patch.object(netapp_nfs.NetAppNFSDriver, 'do_setup')
+    def test_do_setup_http_non_default_port(self, mock_setup):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_server_port = 81
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('81', driver._client.get_port())
+        self.assertEqual('http', driver._client.get_transport_type())
+
+    @mock.patch.object(netapp_nfs.NetAppNFSDriver, 'do_setup')
+    def test_do_setup_https_non_default_port(self, mock_setup):
+        configuration = self._set_config(create_configuration())
+        configuration.netapp_transport_type = 'https'
+        configuration.netapp_server_port = 446
+        driver = common.NetAppDriver(configuration=configuration)
+        driver._do_custom_setup = mock.Mock()
+        driver.do_setup(context='')
+        self.assertEqual('446', driver._client.get_port())
+        self.assertEqual('https', driver._client.get_transport_type())
+
 
 class NetappDirectCmodeNfsDriverOnlyTestCase(test.TestCase):
     """Test direct NetApp C Mode driver only and not inherit."""
@@ -1255,3 +1316,9 @@ class NetappDirect7modeNfsDriverTestCase(NetappDirectCmodeNfsDriverTestCase):
     def test_get_pool(self):
         pool = self._driver.get_pool({'provider_location': 'fake-share'})
         self.assertEqual(pool, 'fake-share')
+
+    def _set_config(self, configuration):
+        super(NetappDirect7modeNfsDriverTestCase, self)._set_config(
+            configuration)
+        configuration.netapp_storage_family = 'ontap_7mode'
+        return configuration

cinder/volume/drivers/netapp/eseries/iscsi.py

@@ -95,10 +95,17 @@ class Driver(driver.ISCSIDriver):
     def do_setup(self, context):
         """Any initialization the volume driver does while starting."""
         self._check_flags()
+        port = self.configuration.netapp_server_port
+        scheme = self.configuration.netapp_transport_type.lower()
+        if port is None:
+            if scheme == 'http':
+                port = 8080
+            elif scheme == 'https':
+                port = 8443
         self._client = client.RestClient(
-            scheme=self.configuration.netapp_transport_type,
+            scheme=scheme,
             host=self.configuration.netapp_server_hostname,
-            port=self.configuration.netapp_server_port,
+            port=port,
             service_path=self.configuration.netapp_webservice_path,
             username=self.configuration.netapp_login,
             password=self.configuration.netapp_password)

cinder/volume/drivers/netapp/iscsi.py

@@ -87,9 +87,8 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
     VERSION = "1.0.0"
 
     IGROUP_PREFIX = 'openstack-'
-    required_flags = ['netapp_transport_type', 'netapp_login',
+    required_flags = ['netapp_login', 'netapp_password',
-                      'netapp_password', 'netapp_server_hostname',
+                      'netapp_server_hostname']
-                      'netapp_server_port']
 
     def __init__(self, *args, **kwargs):
         super(NetAppDirectISCSIDriver, self).__init__(*args, **kwargs)
@@ -114,6 +113,8 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
                                style=NaServer.STYLE_LOGIN_PASSWORD,
                                username=kwargs['login'],
                                password=kwargs['password'])
+        if kwargs['port'] is not None:
+            self.client.set_port(kwargs['port'])
 
     def _do_custom_setup(self):
         """Does custom setup depending on the type of filer."""

cinder/volume/drivers/netapp/nfs.py

@@ -670,9 +670,7 @@ class NetAppDirectNfsDriver (NetAppNFSDriver):
         """Raises error if any required configuration flag is missing."""
         required_flags = ['netapp_login',
                           'netapp_password',
-                          'netapp_server_hostname',
+                          'netapp_server_hostname']
-                          'netapp_server_port',
-                          'netapp_transport_type']
         for flag in required_flags:
             if not getattr(self.configuration, flag, None):
                 raise exception.CinderException(_('%s is not set') % flag)
@@ -686,6 +684,8 @@ class NetAppDirectNfsDriver (NetAppNFSDriver):
             style=NaServer.STYLE_LOGIN_PASSWORD,
             username=self.configuration.netapp_login,
             password=self.configuration.netapp_password)
+        if self.configuration.netapp_server_port is not None:
+            client.set_port(self.configuration.netapp_server_port)
         return client
 
     def _do_custom_setup(self, client):

cinder/volume/drivers/netapp/options.py

@@ -44,12 +44,11 @@ netapp_connection_opts = [
                help='The hostname (or IP address) for the storage system or '
                     'proxy server.'),
     cfg.IntOpt('netapp_server_port',
-               default=80,
+               default=None,
                help=('The TCP port to use for communication with the storage '
-                     'system or proxy server. Traditionally, port 80 is used '
+                     'system or proxy server. If not specified, Data ONTAP '
-                     'for HTTP and port 443 is used for HTTPS; however, this '
+                     'drivers will use 80 for HTTP and 443 for HTTPS; '
-                     'value should be changed if an alternate port has been '
+                     'E-Series will use 8080 for HTTP and 8443 for HTTPS.')), ]
-                     'configured on the storage system or proxy server.')), ]
 
 netapp_transport_opts = [
     cfg.StrOpt('netapp_transport_type',

etc/cinder/cinder.conf.sample

@@ -1612,11 +1612,10 @@
 #netapp_server_hostname=<None>
 
 # The TCP port to use for communication with the storage
-# system or proxy server. Traditionally, port 80 is used for
+# system or proxy server. If not specified, Data ONTAP drivers
-# HTTP and port 443 is used for HTTPS; however, this value
+# will use 80 for HTTP and 443 for HTTPS; E-Series will use
-# should be changed if an alternate port has been configured
+# 8080 for HTTP and 8443 for HTTPS. (integer value)
-# on the storage system or proxy server. (integer value)
+#netapp_server_port=<None>
-#netapp_server_port=80
 
 # This option is used to specify the path to the E-Series
 # proxy application on a proxy server. The value is combined