From 039b2bfd64bebc011ee9debe8962c8340db37b69 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. Conflicts: cinder/volume/drivers/hpe/hpe_3par_common.py 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 cbdeff5b10e..976362bb799 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -31,6 +31,7 @@ import fixtures from keystonemiddleware import auth_token import mock 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 @@ -470,6 +473,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 171c6c7c45f..f3ecde06337 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 @@ -732,6 +731,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( @@ -5071,39 +5071,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, @@ -5111,15 +5110,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 6366b61e39c..077bc8d6d42 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 0d668018a19..f7afb289b96 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_common.py +++ b/cinder/volume/drivers/hpe/hpe_3par_common.py @@ -1449,11 +1449,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 LOG.debug("unique_fqdn_network: %(fqdn)s", {'fqdn': unique_fqdn_network}) if(not unique_fqdn_network and connector.get('initiator')): 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 e929941e2b6..6bc393868ff 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 adc16c2a1de..ea5e27e47f9 100644 --- a/cinder/volume/drivers/kaminario/kaminario_common.py +++ b/cinder/volume/drivers/kaminario/kaminario_common.py @@ -837,10 +837,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).