Merge "Kaminario: Fix unique_fqdn_network option"

cinder/tests/unit/test.py

@@ -30,6 +30,7 @@ from eventlet import tpool
 import fixtures
 from keystonemiddleware import auth_token
 from oslo_concurrency import lockutils
+from oslo_config import cfg
 from oslo_config import fixture as config_fixture
 from oslo_log.fixture import logging_error as log_fixture
 import oslo_messaging
@@ -53,6 +54,8 @@ from cinder import service
 from cinder.tests import fixtures as cinder_fixtures
 from cinder.tests.unit import conf_fixture
 from cinder.tests.unit import fake_notifier
+from cinder.volume import configuration
+from cinder.volume import driver as vol_driver
 from cinder.volume import volume_types
 from cinder.volume import volume_utils
 
@@ -433,6 +436,25 @@ class TestCase(testtools.TestCase):
         """
         self.useFixture(fixtures.MonkeyPatch(old, new))
 
+    def _set_unique_fqdn_override(self, value, in_shared):
+        """Override the unique_fqdn_network configuration option.
+
+        Meant for driver tests that use a Mock for their driver configuration
+        instead of a real Oslo Conf.
+        """
+        # Since we don't use a real oslo config for the driver we don't get
+        # the default initialization, so create a group and register the option
+        cfg.CONF.register_group(cfg.OptGroup('driver_cfg'))
+        new_config = configuration.Configuration([], config_group='driver_cfg')
+        new_config.append_config_values(vol_driver.fqdn_opts)
+
+        # Now we override the value for this test
+        group = configuration.SHARED_CONF_GROUP if in_shared else 'driver_cfg'
+        self.addCleanup(CONF.clear_override, 'unique_fqdn_network',
+                        group=group)
+        cfg.CONF.set_override('unique_fqdn_network', value, group=group)
+        return new_config
+
 
 class ModelsObjectComparatorMixin(object):
     def _dict_from_object(self, obj, ignored_keys):

cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py

@@ -30,7 +30,6 @@ from cinder.tests.unit import fake_volume
 from cinder.tests.unit import test
 from cinder.tests.unit.volume.drivers.hpe \
     import fake_hpe_3par_client as hpe3parclient
-from cinder.volume import configuration as cvol_cfg
 from cinder.volume.drivers.hpe import hpe_3par_base as hpedriverbase
 from cinder.volume.drivers.hpe import hpe_3par_common as hpecommon
 from cinder.volume.drivers.hpe import hpe_3par_fc as hpefcdriver
@@ -756,6 +755,7 @@ class HPE3PARBaseDriver(test.TestCase):
         configuration.image_volume_cache_enabled = False
         configuration.replication_device = None
         configuration.hpe3par_target_nsp = None
+        configuration.unique_fqdn_network = True
         return configuration
 
     @mock.patch(
@@ -5158,39 +5158,38 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
                               self.driver.unmanage_snapshot,
                               snapshot=snapshot)
 
-    def test__safe_hostname(self):
+    @ddt.data(True, False)
-        long_hostname = "abc123abc123abc123abc123abc123abc123"
+    def test__safe_hostname(self, in_shared):
+        config = self._set_unique_fqdn_override(True, in_shared)
+        my_connector = self.connector.copy()
+        my_connector['host'] = "abc123abc123abc123abc123abc123abc123"
         fixed_hostname = "abc123abc123abc123abc123abc123a"
         mock_client = self.setup_driver()
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
             mock_create_client.return_value = mock_client
             common = self.driver._login()
-            safe_host = common._safe_hostname(long_hostname)
+            safe_host = common._safe_hostname(my_connector, config)
             self.assertEqual(fixed_hostname, safe_host)
 
-    def test__safe_hostname_unique(self):
+    @ddt.data(True, False)
-        long_hostname = "abc123abc123abc123abc123abc123abc123"
+    def test__safe_hostname_unique(self, in_shared):
         mock_client = self.setup_driver()
         with mock.patch.object(hpecommon.HPE3PARCommon,
                                '_create_client') as mock_create_client:
             mock_create_client.return_value = mock_client
             common = self.driver._login()
 
-            self.addCleanup(CONF.clear_override,
+            config = self._set_unique_fqdn_override(False, in_shared)
-                            'unique_fqdn_network',
-                            group=cvol_cfg.SHARED_CONF_GROUP)
-            CONF.set_override('unique_fqdn_network',
-                              False,
-                              group=cvol_cfg.SHARED_CONF_GROUP)
             my_connector = self.connector.copy()
+            my_connector['host'] = "abc123abc123abc123abc123abc123abc123"
             my_connector['initiator'] = 'iqn.1993-08.org.debian:01:222:12345'
             ret_name = '54321-222-10-naibed.gro.80-3991'
-            safe_host = common._safe_hostname(long_hostname, my_connector)
+            safe_host = common._safe_hostname(my_connector, config)
             self.assertEqual(ret_name, safe_host)
 
-    def test__safe_hostname_unique_without_initiator(self):
+    @ddt.data(True, False)
-        long_hostname = "abc123abc123abc123abc123abc123abc123"
+    def test__safe_hostname_unique_without_initiator(self, in_shared):
         fixed_hostname = "abc123abc123abc123abc123abc123a"
         mock_client = self.setup_driver()
         with mock.patch.object(hpecommon.HPE3PARCommon,
@@ -5198,15 +5197,11 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
             mock_create_client.return_value = mock_client
             common = self.driver._login()
 
-            self.addCleanup(CONF.clear_override,
+            conf = self._set_unique_fqdn_override(False, in_shared)
-                            'unique_fqdn_network',
-                            group=cvol_cfg.SHARED_CONF_GROUP)
-            CONF.set_override('unique_fqdn_network',
-                              False,
-                              group=cvol_cfg.SHARED_CONF_GROUP)
             my_connector = self.connector.copy()
             del(my_connector['initiator'])
-            safe_host = common._safe_hostname(long_hostname, my_connector)
+            my_connector['host'] = "abc123abc123abc123abc123abc123abc123"
+            safe_host = common._safe_hostname(my_connector, conf)
             self.assertEqual(fixed_hostname, safe_host)
 
     @mock.patch('cinder.volume.drivers.hpe.hpe_3par_common.HPE3PARCommon.'

cinder/tests/unit/volume/drivers/test_kaminario.py

@@ -124,6 +124,7 @@ class Replication(object):
     rpo = 500
 
 
+@ddt.ddt
 class TestKaminarioCommon(test.TestCase):
     driver = None
     conf = None
@@ -145,6 +146,7 @@ class TestKaminarioCommon(test.TestCase):
         self.conf.kaminario_dedup_type_name = "dedup"
         self.conf.volume_dd_blocksize = 2
         self.conf.disable_discovery = False
+        self.conf.unique_fqdn_network = True
 
     def _setup_driver(self):
         self.driver = (kaminario_iscsi.
@@ -525,13 +527,10 @@ class TestKaminarioCommon(test.TestCase):
         result = self.driver.get_initiator_host_name(CONNECTOR)
         self.assertEqual(CONNECTOR['host'], result)
 
-    def test_get_initiator_host_name_unique(self):
+    @ddt.data(True, False)
-        self.addCleanup(CONF.clear_override,
+    def test_get_initiator_host_name_unique(self, in_shared):
-                        'unique_fqdn_network',
+        cfg = self._set_unique_fqdn_override(False, in_shared)
-                        group=configuration.SHARED_CONF_GROUP)
+        self.mock_object(self.driver, 'configuration', cfg)
-        CONF.set_override('unique_fqdn_network',
-                          False,
-                          group=configuration.SHARED_CONF_GROUP)
         result = self.driver.get_initiator_host_name(CONNECTOR)
         expected = re.sub('[:.]', '_', CONNECTOR['initiator'][::-1][:32])
         self.assertEqual(expected, result)
@@ -599,6 +598,7 @@ class TestKaminarioISCSI(TestKaminarioCommon):
         self.assertIsNone(result)
 
 
+@ddt.ddt
 class TestKaminarioFC(TestKaminarioCommon):
 
     def _setup_driver(self):
@@ -631,15 +631,12 @@ class TestKaminarioFC(TestKaminarioCommon):
         result = self.driver.terminate_connection(self.vol, None)
         self.assertIn('data', result)
 
-    def test_get_initiator_host_name_unique(self):
+    @ddt.data(True, False)
+    def test_get_initiator_host_name_unique(self, in_shared):
+        cfg = self._set_unique_fqdn_override(False, in_shared)
+        self.mock_object(self.driver, 'configuration', cfg)
         connector = CONNECTOR.copy()
         del connector['initiator']
-        self.addCleanup(CONF.clear_override,
-                        'unique_fqdn_network',
-                        group=configuration.SHARED_CONF_GROUP)
-        CONF.set_override('unique_fqdn_network',
-                          False,
-                          group=configuration.SHARED_CONF_GROUP)
         result = self.driver.get_initiator_host_name(connector)
         expected = re.sub('[:.]', '_', connector['wwnns'][0][::-1][:32])
         self.assertEqual(expected, result)

cinder/volume/drivers/hpe/hpe_3par_common.py

@@ -1473,11 +1473,10 @@ class HPE3PARCommon(object):
                 raise exception.VolumeBackendAPIException(
                     data=e.get_description())
 
-    def _safe_hostname(self, hostname, connector=None):
+    def _safe_hostname(self, connector, configuration):
         """We have to use a safe hostname length for 3PAR host names."""
-        SHARED_CONF_GROUP = 'backend_defaults'
+        hostname = connector['host']
-        shared_backend_conf = CONF._get(SHARED_CONF_GROUP)
+        unique_fqdn_network = configuration.unique_fqdn_network
-        unique_fqdn_network = shared_backend_conf.unique_fqdn_network
         if(not unique_fqdn_network and connector.get('initiator')):
             iqn = connector.get('initiator')
             iqn = iqn.replace(":", "-")

cinder/volume/drivers/hpe/hpe_3par_fc.py

@@ -339,7 +339,7 @@ class HPE3PARFCDriver(hpebasedriver.HPE3PARDriverBase):
                 # for now, do not remove zones
                 zone_remove = False
             else:
-                hostname = common._safe_hostname(connector['host'], connector)
+                hostname = common._safe_hostname(connector, self.configuration)
                 common.terminate_connection(volume, hostname,
                                             wwn=connector['wwpns'],
                                             remote_client=remote_client)
@@ -528,7 +528,7 @@ class HPE3PARFCDriver(hpebasedriver.HPE3PARDriverBase):
         """Creates or modifies existing 3PAR host."""
         host = None
         domain = None
-        hostname = common._safe_hostname(connector['host'], connector)
+        hostname = common._safe_hostname(connector, self.configuration)
         if remote_target:
             cpg = common._get_cpg_from_cpg_map(
                 remote_target['cpg_map'], src_cpg)

cinder/volume/drivers/hpe/hpe_3par_iscsi.py

@@ -495,7 +495,7 @@ class HPE3PARISCSIDriver(hpebasedriver.HPE3PARDriverBase):
             if is_force_detach:
                 common.terminate_connection(volume, None, None)
             else:
-                hostname = common._safe_hostname(connector['host'], connector)
+                hostname = common._safe_hostname(connector, self.configuration)
                 common.terminate_connection(
                     volume,
                     hostname,
@@ -601,7 +601,7 @@ class HPE3PARISCSIDriver(hpebasedriver.HPE3PARDriverBase):
         domain = None
         username = None
         password = None
-        hostname = common._safe_hostname(connector['host'], connector)
+        hostname = common._safe_hostname(connector, self.configuration)
 
         if remote_target:
             cpg = common._get_cpg_from_cpg_map(

cinder/volume/drivers/kaminario/kaminario_common.py

@@ -839,10 +839,7 @@ class KaminarioCinderDriver(cinder.volume.driver.ISCSIDriver):
         """
         name = connector.get('initiator',
                              connector.get('wwnns', [''])[0])[::-1]
-        SHARED_CONF_GROUP = 'backend_defaults'
+        if self.configuration.unique_fqdn_network:
-        shared_backend_conf = CONF._get(SHARED_CONF_GROUP)
-        unique_fqdn_network = shared_backend_conf.unique_fqdn_network
-        if unique_fqdn_network:
             name = connector.get('host', name)
         return re.sub('[^0-9a-zA-Z-_]', '_', name[:32])
 

doc/source/configuration/block-storage/drivers/hpe-3par-driver.rst

@@ -555,7 +555,8 @@ creating VMs and they all have names like controller-0.localdomain and
 compute-0.localdomain.
 
 To support these kind of environments, the user can specify below flag
-in backend_defaults section of cinder.conf as follows:
+in backend_defaults section or the specific cinder driver section of
+cinder.conf as follows:
 
 ``unique_fqdn_network = False``
 

releasenotes/notes/fix-kaminario-unique_fqdn_network-ecde36f614c30733.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fix ``unique_fqdn_network`` configuration option for the Kaminario driver,
+    as it was being ignored when defined in the driver section, which used to
+    work.
+    (Bug #1886042).