VMAX driver and docs - change san_rest_port to san_api_port

This is part of the effort to consolidate and eliminate duplicate
configuration tags. san_rest_port will be deprecated in this
release and replaced by san_api_port in the next.

Change-Id: I5051e39c3bf45a3a4903e94c1597a142321a77e4
Closes-Bug: #1758010

cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py

@@ -1150,6 +1150,8 @@ class FakeConfiguration(object):
                 self.san_password = value
             elif key == 'san_ip':
                 self.san_ip = value
+            elif key == 'san_api_port':
+                self.san_api_port = value
             elif key == 'san_rest_port':
                 self.san_rest_port = value
             elif key == 'vmax_srp':
@@ -5351,7 +5353,30 @@ class VMAXCommonTest(test.TestCase):
         self.assertEqual(ref_dev_id, src_dev_id1)
         self.assertEqual(ref_dev_id, src_dev_id2)
 
-    def test_get_attributes_from_cinder_config(self):
+    def test_get_attributes_from_cinder_config_new(self):
+        kwargs_expected = (
+            {'RestServerIp': '1.1.1.1',
+             'RestServerPort': 8443,
+             'RestUserName': 'smc',
+             'RestPassword': 'smc',
+             'SSLCert': None,
+             'SSLVerify': False,
+             'SerialNumber': self.data.array,
+             'srpName': 'SRP_1',
+             'PortGroup': self.data.port_group_name_i})
+        backup_conf = self.common.configuration
+        configuration = FakeConfiguration(
+            None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
+            vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc',
+            san_api_port=8443, vmax_port_groups=[self.data.port_group_name_i])
+        self.common.configuration = configuration
+        kwargs_returned = self.common.get_attributes_from_cinder_config()
+        self.assertEqual(kwargs_expected, kwargs_returned)
+        self.common.configuration = backup_conf
+        kwargs = self.common.get_attributes_from_cinder_config()
+        self.assertIsNone(kwargs)
+
+    def test_get_attributes_from_cinder_config_old(self):
         kwargs_expected = (
             {'RestServerIp': '1.1.1.1',
              'RestServerPort': 8443,
@@ -5374,6 +5399,63 @@ class VMAXCommonTest(test.TestCase):
         kwargs = self.common.get_attributes_from_cinder_config()
         self.assertIsNone(kwargs)
 
+    def test_get_attributes_from_cinder_config_with_port_override_old(self):
+        kwargs_expected = (
+            {'RestServerIp': '1.1.1.1',
+             'RestServerPort': 3448,
+             'RestUserName': 'smc',
+             'RestPassword': 'smc',
+             'SSLCert': None,
+             'SSLVerify': False,
+             'SerialNumber': self.data.array,
+             'srpName': 'SRP_1',
+             'PortGroup': self.data.port_group_name_i})
+        configuration = FakeConfiguration(
+            None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
+            vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc',
+            san_rest_port=3448, vmax_port_groups=[self.data.port_group_name_i])
+        self.common.configuration = configuration
+        kwargs_returned = self.common.get_attributes_from_cinder_config()
+        self.assertEqual(kwargs_expected, kwargs_returned)
+
+    def test_get_attributes_from_cinder_config_with_port_override_new(self):
+        kwargs_expected = (
+            {'RestServerIp': '1.1.1.1',
+             'RestServerPort': 3448,
+             'RestUserName': 'smc',
+             'RestPassword': 'smc',
+             'SSLCert': None,
+             'SSLVerify': False,
+             'SerialNumber': self.data.array,
+             'srpName': 'SRP_1',
+             'PortGroup': self.data.port_group_name_i})
+        configuration = FakeConfiguration(
+            None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
+            vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc',
+            san_api_port=3448, vmax_port_groups=[self.data.port_group_name_i])
+        self.common.configuration = configuration
+        kwargs_returned = self.common.get_attributes_from_cinder_config()
+        self.assertEqual(kwargs_expected, kwargs_returned)
+
+    def test_get_attributes_from_cinder_config_no_port(self):
+        kwargs_expected = (
+            {'RestServerIp': '1.1.1.1',
+             'RestServerPort': 8443,
+             'RestUserName': 'smc',
+             'RestPassword': 'smc',
+             'SSLCert': None,
+             'SSLVerify': False,
+             'SerialNumber': self.data.array,
+             'srpName': 'SRP_1',
+             'PortGroup': self.data.port_group_name_i})
+        configuration = FakeConfiguration(
+            None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
+            vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc',
+            vmax_port_groups=[self.data.port_group_name_i])
+        self.common.configuration = configuration
+        kwargs_returned = self.common.get_attributes_from_cinder_config()
+        self.assertEqual(kwargs_expected, kwargs_returned)
+
     @mock.patch.object(rest.VMAXRest,
                        'get_size_of_device_on_array',
                        return_value=2.0)

cinder/volume/drivers/dell_emc/vmax/common.py

@@ -73,7 +73,12 @@ vmax_opts = [
                 default=False,
                 help='Use this value to enable '
                      'the initiator_check.'),
-    cfg.PortOpt(utils.VMAX_SERVER_PORT,
+    cfg.PortOpt(utils.VMAX_SERVER_PORT_OLD,
+                deprecated_for_removal=True,
+                deprecated_since="13.0.0",
+                deprecated_reason='Unisphere port should now be '
+                                  'set using the common san_api_port '
+                                  'config option instead.',
                 default=8443,
                 help='REST server port number.'),
     cfg.StrOpt(utils.VMAX_ARRAY,
@@ -4275,6 +4280,10 @@ class VMAXCommon(object):
         return model_update, vol_model_updates
 
     def get_attributes_from_cinder_config(self):
+        """Get all attributes from the configuration file
+
+        :returns: kwargs
+        """
         LOG.debug("Using cinder.conf file")
         kwargs = None
         username = self.configuration.safe_get(utils.VMAX_USER_NAME)
@@ -4293,17 +4302,18 @@ class VMAXCommon(object):
             if port_groups:
                 random_portgroup = random.choice(self.configuration.safe_get(
                     utils.VMAX_PORT_GROUPS))
+
             kwargs = (
                 {'RestServerIp': self.configuration.safe_get(
                     utils.VMAX_SERVER_IP),
-                 'RestServerPort': self.configuration.safe_get(
+                 'RestServerPort': self._get_unisphere_port(),
-                    utils.VMAX_SERVER_PORT),
                  'RestUserName': username,
                  'RestPassword': password,
                  'SSLCert': self.configuration.safe_get('driver_client_cert'),
                  'SerialNumber': serial_number,
                  'srpName': srp_name,
                  'PortGroup': random_portgroup})
+
             if self.configuration.safe_get('driver_ssl_cert_verify'):
                 kwargs.update({'SSLVerify': self.configuration.safe_get(
                     'driver_ssl_cert_path')})
@@ -4313,6 +4323,20 @@ class VMAXCommon(object):
                 kwargs.update({'ServiceLevel': slo, 'Workload': workload})
         return kwargs
 
+    def _get_unisphere_port(self):
+        """Get unisphere port from the configuration file
+
+        :returns: unisphere port
+        """
+        if self.configuration.safe_get(utils.VMAX_SERVER_PORT_OLD):
+            return self.configuration.safe_get(utils.VMAX_SERVER_PORT_OLD)
+        elif self.configuration.safe_get(utils.VMAX_SERVER_PORT_NEW):
+            return self.configuration.safe_get(utils.VMAX_SERVER_PORT_NEW)
+        else:
+            LOG.debug("VMAX port is not set, using default port: %s",
+                      utils.DEFAULT_PORT)
+            return utils.DEFAULT_PORT
+
     def revert_to_snapshot(self, volume, snapshot):
         """Revert volume to snapshot.
 

cinder/volume/drivers/dell_emc/vmax/utils.py

@@ -73,6 +73,8 @@ RDF_ACTIVE = 'active'
 RDF_ACTIVEACTIVE = 'activeactive'
 RDF_ACTIVEBIAS = 'activebias'
 METROBIAS = 'metro_bias'
+DEFAULT_PORT = 8443
+
 # Multiattach constants
 IS_MULTIATTACH = 'multiattach'
 OTHER_PARENT_SG = 'other_parent_sg_name'
@@ -83,7 +85,8 @@ NO_SLO_SG = 'no_slo_sg'
 VMAX_SERVER_IP = 'san_ip'
 VMAX_USER_NAME = 'san_login'
 VMAX_PASSWORD = 'san_password'
-VMAX_SERVER_PORT = 'san_rest_port'
+VMAX_SERVER_PORT_NEW = 'san_api_port'
+VMAX_SERVER_PORT_OLD = 'san_rest_port'
 VMAX_ARRAY = 'vmax_array'
 VMAX_WORKLOAD = 'vmax_workload'
 VMAX_SRP = 'vmax_srp'

doc/source/configuration/block-storage/drivers/dell-emc-vmax-driver.rst

@@ -164,7 +164,7 @@ VMAX Driver Integration
    |  RestServerIp   | san_ip                 | "       | Yes      | IP address of the         |
    |                 |                        |         |          | Unisphere server          |
    +-----------------+------------------------+---------+----------+---------------------------+
-   |  RestServerPort | san_rest_port          | 8443    | No       | Port of the               |
+   |  RestServerPort | san_api_port           | 8443    | No       | Port of the               |
    |                 |                        |         |          | Unisphere server          |
    +-----------------+------------------------+---------+----------+---------------------------+
    |  RestUserName   | san_login              | 'admin' | Yes      | Username of the           |
@@ -188,7 +188,7 @@ VMAX Driver Integration
 
    .. note::
 
-      ``san_rest_port`` is ``8443`` by default but can be changed if
+      ``san_api_port`` is ``8443`` by default but can be changed if
       necessary. For the purposes of this documentation the default is
       assumed so the tag will not appear in any of the ``cinder.conf``
       extracts below.

releasenotes/notes/deprecate_san_rest_port-0d8610a872e92e09.yaml

@@ -0,0 +1,5 @@
+---
+deprecations:
+  - |
+    VMAX driver - configuration tag san_rest_port will be replaced by
+    san_api_port in the next release.