From: Andrew Kerr Date: Thu, 29 May 2014 03:16:23 +0000 (+0530) Subject: NetApp fix to set non default server port in api X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=bfbc29f19037feb89408206625849bde14a7e84a;p=openstack-build%2Fcinder-build.git 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 --- diff --git a/cinder/tests/test_netapp.py b/cinder/tests/test_netapp.py index 0a77ad266..ac4442df1 100644 --- a/cinder/tests/test_netapp.py +++ b/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 diff --git a/cinder/tests/test_netapp_eseries_iscsi.py b/cinder/tests/test_netapp_eseries_iscsi.py index e9b6a2ced..6c6cf5d06 100644 --- a/cinder/tests/test_netapp_eseries_iscsi.py +++ b/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) diff --git a/cinder/tests/test_netapp_nfs.py b/cinder/tests/test_netapp_nfs.py index e9d27e016..a9b6d00c4 100644 --- a/cinder/tests/test_netapp_nfs.py +++ b/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_port'] + 'netapp_server_hostname'] # 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 diff --git a/cinder/volume/drivers/netapp/eseries/iscsi.py b/cinder/volume/drivers/netapp/eseries/iscsi.py index f626a4fa4..889a7f2e4 100644 --- a/cinder/volume/drivers/netapp/eseries/iscsi.py +++ b/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) diff --git a/cinder/volume/drivers/netapp/iscsi.py b/cinder/volume/drivers/netapp/iscsi.py index e08c0dc10..c518e4e4b 100644 --- a/cinder/volume/drivers/netapp/iscsi.py +++ b/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', - 'netapp_password', 'netapp_server_hostname', - 'netapp_server_port'] + required_flags = ['netapp_login', 'netapp_password', + 'netapp_server_hostname'] 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.""" diff --git a/cinder/volume/drivers/netapp/nfs.py b/cinder/volume/drivers/netapp/nfs.py index c74f8551e..a1e3960be 100644 --- a/cinder/volume/drivers/netapp/nfs.py +++ b/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_port', - 'netapp_transport_type'] + 'netapp_server_hostname'] 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): diff --git a/cinder/volume/drivers/netapp/options.py b/cinder/volume/drivers/netapp/options.py index 5b01d1f58..ad217c35d 100644 --- a/cinder/volume/drivers/netapp/options.py +++ b/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 ' - 'for HTTP and port 443 is used for HTTPS; however, this ' - 'value should be changed if an alternate port has been ' - 'configured on the storage system or proxy server.')), ] + 'system or proxy server. If not specified, Data ONTAP ' + 'drivers will use 80 for HTTP and 443 for HTTPS; ' + 'E-Series will use 8080 for HTTP and 8443 for HTTPS.')), ] netapp_transport_opts = [ cfg.StrOpt('netapp_transport_type', diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index 1d6dc8a9b..74d2ecb91 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -1612,11 +1612,10 @@ #netapp_server_hostname= # The TCP port to use for communication with the storage -# system or proxy server. Traditionally, port 80 is used for -# HTTP and port 443 is used for HTTPS; however, this value -# should be changed if an alternate port has been configured -# on the storage system or proxy server. (integer value) -#netapp_server_port=80 +# system or proxy server. If not specified, Data ONTAP drivers +# will use 80 for HTTP and 443 for HTTPS; E-Series will use +# 8080 for HTTP and 8443 for HTTPS. (integer value) +#netapp_server_port= # This option is used to specify the path to the E-Series # proxy application on a proxy server. The value is combined