From 33cf05eba2f9dfd09b6eceb75724af231be77d34 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 2 Jul 2020 14:24:37 +0200 Subject: [PATCH] Kaminario: Fix unique_fqdn_network option The Kaminario driver used to accept the unique_fqdn_network configuration option in the specific driver section, but when we moved this option to be an option also used by 3PAR we introduced a regression where the parameter was now only used if it was defined in the shared configuration group. This patch fixes this and makes it possible for the option to be defined in the shared configuration group as well as the driver specific section. Closes-Bug: #1886042 Change-Id: I88c5afacff5f1f780c2dc850e3af417b1b081592 (cherry picked from commit 53504f82a53146151935e6aac301a41e2b387623) --- cinder/test.py | 22 +++++++++++ .../unit/volume/drivers/hpe/test_hpe3par.py | 39 ++++++++----------- .../unit/volume/drivers/test_kaminario.py | 25 ++++++------ cinder/volume/drivers/hpe/hpe_3par_common.py | 7 ++-- cinder/volume/drivers/hpe/hpe_3par_fc.py | 4 +- cinder/volume/drivers/hpe/hpe_3par_iscsi.py | 4 +- .../drivers/kaminario/kaminario_common.py | 5 +-- .../block-storage/drivers/hpe-3par-driver.rst | 3 +- ...-unique_fqdn_network-ecde36f614c30733.yaml | 7 ++++ 9 files changed, 67 insertions(+), 49 deletions(-) create mode 100644 releasenotes/notes/fix-kaminario-unique_fqdn_network-ecde36f614c30733.yaml diff --git a/cinder/test.py b/cinder/test.py index 99fdeb57bdf..f78af4d362b 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -31,6 +31,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 @@ -55,6 +56,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 @@ -469,6 +472,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): diff --git a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py index ca3866e6804..4972c79679b 100644 --- a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py +++ b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py @@ -30,7 +30,6 @@ from cinder import test from cinder.tests.unit import fake_volume 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( @@ -5165,39 +5165,38 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): self.driver.unmanage_snapshot, snapshot=snapshot) - def test__safe_hostname(self): - long_hostname = "abc123abc123abc123abc123abc123abc123" + @ddt.data(True, False) + 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): - long_hostname = "abc123abc123abc123abc123abc123abc123" + @ddt.data(True, False) + 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, - 'unique_fqdn_network', - group=cvol_cfg.SHARED_CONF_GROUP) - CONF.set_override('unique_fqdn_network', - False, - group=cvol_cfg.SHARED_CONF_GROUP) + config = self._set_unique_fqdn_override(False, in_shared) 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): - long_hostname = "abc123abc123abc123abc123abc123abc123" + @ddt.data(True, False) + def test__safe_hostname_unique_without_initiator(self, in_shared): fixed_hostname = "abc123abc123abc123abc123abc123a" mock_client = self.setup_driver() with mock.patch.object(hpecommon.HPE3PARCommon, @@ -5205,15 +5204,11 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): mock_create_client.return_value = mock_client common = self.driver._login() - self.addCleanup(CONF.clear_override, - 'unique_fqdn_network', - group=cvol_cfg.SHARED_CONF_GROUP) - CONF.set_override('unique_fqdn_network', - False, - group=cvol_cfg.SHARED_CONF_GROUP) + conf = self._set_unique_fqdn_override(False, in_shared) 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.' diff --git a/cinder/tests/unit/volume/drivers/test_kaminario.py b/cinder/tests/unit/volume/drivers/test_kaminario.py index a5fd75aeb3b..c8d0ffb12ac 100644 --- a/cinder/tests/unit/volume/drivers/test_kaminario.py +++ b/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): - 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) + @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) 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) diff --git a/cinder/volume/drivers/hpe/hpe_3par_common.py b/cinder/volume/drivers/hpe/hpe_3par_common.py index b65902705de..03bb96ff3dd 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_common.py +++ b/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' - shared_backend_conf = CONF._get(SHARED_CONF_GROUP) - unique_fqdn_network = shared_backend_conf.unique_fqdn_network + hostname = connector['host'] + unique_fqdn_network = configuration.unique_fqdn_network if(not unique_fqdn_network and connector.get('initiator')): iqn = connector.get('initiator') iqn = iqn.replace(":", "-") diff --git a/cinder/volume/drivers/hpe/hpe_3par_fc.py b/cinder/volume/drivers/hpe/hpe_3par_fc.py index 2054efab812..d4461ca8511 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_fc.py +++ b/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) diff --git a/cinder/volume/drivers/hpe/hpe_3par_iscsi.py b/cinder/volume/drivers/hpe/hpe_3par_iscsi.py index c19bd7bc17e..83febae894a 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_iscsi.py +++ b/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( diff --git a/cinder/volume/drivers/kaminario/kaminario_common.py b/cinder/volume/drivers/kaminario/kaminario_common.py index 7fe418350d0..1c7e20f5880 100644 --- a/cinder/volume/drivers/kaminario/kaminario_common.py +++ b/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' - shared_backend_conf = CONF._get(SHARED_CONF_GROUP) - unique_fqdn_network = shared_backend_conf.unique_fqdn_network - if unique_fqdn_network: + if self.configuration.unique_fqdn_network: name = connector.get('host', name) return re.sub('[^0-9a-zA-Z-_]', '_', name[:32]) diff --git a/doc/source/configuration/block-storage/drivers/hpe-3par-driver.rst b/doc/source/configuration/block-storage/drivers/hpe-3par-driver.rst index cc7dc30f7ec..d013cdc0392 100644 --- a/doc/source/configuration/block-storage/drivers/hpe-3par-driver.rst +++ b/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`` diff --git a/releasenotes/notes/fix-kaminario-unique_fqdn_network-ecde36f614c30733.yaml b/releasenotes/notes/fix-kaminario-unique_fqdn_network-ecde36f614c30733.yaml new file mode 100644 index 00000000000..f013dbb7aa4 --- /dev/null +++ b/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).