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 53504f82a5)
This commit is contained in:
Gorka Eguileor 2020-07-02 14:24:37 +02:00 committed by raghavendrat
parent 4cf0d11243
commit 039b2bfd64
9 changed files with 67 additions and 49 deletions

View File

@ -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):

View File

@ -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.'

View File

@ -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)

View File

@ -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')):

View File

@ -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)

View File

@ -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(

View File

@ -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])

View File

@ -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``

View File

@ -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).