openstack/cinder
Code Issues Proposed changes

Add 'enforce_multipath' in connection_properties

Browse Source 
Previously, the check for multipath used to happen while fetching
connector information. The downside of this was we didn't know
what kind of connection (iSCSI, FC, NVMe) will be created and
it was not possible to handle the multipath checks for different
connectors.
With change[1], the check was moved to the connect_volume method
but that led to another problem that the connection information
returned from cinder didn't contain the ``enforce_multipath``
parameter.
The purpose of this patch is to include the ``enforce_multipath``
parameter in the connection information based on the value fetched
from the connector.

[1] https://review.opendev.org/c/openstack/os-brick/+/933250

Partial-Bug: #2085013

Change-Id: I4ea618a44b37cce5dd0b131ffac55bce51d08c0a
This commit is contained in:
Rajat Dhasmana 2024-11-04 03:19:34 +05:30
parent c4bedc1de1
commit 0c3c7b7bb5
6 changed files with 72 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cinder/api/contrib/volume_actions.py
View File

@ -172,6 +172,7 @@ class VolumeActionsController(wsgi.Controller):
if error.exc_type == 'InvalidInput':
raise exception.InvalidInput(reason=error.value)
raise
info['enforce_multipath'] = connector.get('enforce_multipath', False)
return {'connection_info': info}

8
cinder/tests/unit/api/contrib/test_volume_actions.py
View File

@ -97,12 +97,14 @@ class VolumeActionsTest(test.TestCase):
res = req.get_response(app)
self.assertEqual(HTTPStatus.ACCEPTED, res.status_int)
def test_initialize_connection(self):
@ddt.data(False, True)
def test_initialize_connection(self, enforce_mpath):
with mock.patch.object(volume_api.API,
'initialize_connection') as init_conn:
init_conn.return_value = {}
body = {'os-initialize_connection': {'connector': {
'fake': 'fake'}}}
'fake': 'fake', 'enforce_multipath': enforce_mpath}}}
expected_conn_info = {'enforce_multipath': enforce_mpath}
req = webob.Request.blank('/v3/%s/volumes/%s/action' %
(fake.PROJECT_ID, fake.VOLUME_ID))
req.method = "POST"
@ -111,7 +113,9 @@ class VolumeActionsTest(test.TestCase):
res = req.get_response(fakes.wsgi_app(
fake_auth_context=self.context))
actual_conn_info = jsonutils.loads(res.body).get('connection_info')
self.assertEqual(HTTPStatus.OK, res.status_int)
self.assertEqual(expected_conn_info, actual_conn_info)
def test_initialize_connection_without_connector(self):
with mock.patch.object(volume_api.API,

15
cinder/tests/unit/attachments/test_attachments_manager.py
View File

@ -12,6 +12,7 @@
from unittest import mock
import ddt
from oslo_config import cfg
from oslo_utils import importutils
@ -28,6 +29,7 @@ from cinder.volume import configuration as conf
CONF = cfg.CONF
@ddt.ddt
class AttachmentManagerTestCase(test.TestCase):
"""Attachment related test for volume.manager.py."""
@ -46,11 +48,13 @@ class AttachmentManagerTestCase(test.TestCase):
self.manager.stats = {'allocated_capacity_gb': 100,
'pools': {}}
@ddt.data(False, True)
@mock.patch.object(db.sqlalchemy.api, '_volume_type_get',
v2_fakes.fake_volume_type_get)
@mock.patch('cinder.db.sqlalchemy.api.volume_type_qos_specs_get')
@mock.patch('cinder.volume.volume_types.get_volume_type_extra_specs')
def test_attachment_update(self, get_extra_specs, mock_type_get):
def test_attachment_update(self, enforce_mpath, get_extra_specs,
mock_type_get):
"""Test attachment_update."""
volume_params = {'status': 'available'}
connector = {
@ -59,7 +63,8 @@ class AttachmentManagerTestCase(test.TestCase):
"platform": "x86_64",
"host": "tempest-1",
"os_type": "linux2",
"multipath": False}
"multipath": False,
"enforce_multipath": enforce_mpath}
vref = tests_utils.create_volume(self.context, **volume_params)
self.manager.create_volume(self.context, vref)
@ -77,7 +82,8 @@ class AttachmentManagerTestCase(test.TestCase):
'cacheable': False,
'access_mode': 'rw',
'driver_volume_type': 'iscsi',
'attachment_id': attachment_ref.id}
'attachment_id': attachment_ref.id,
'enforce_multipath': enforce_mpath}
get_extra_specs.return_value = {}
self.assertEqual(expected,
@ -92,7 +98,8 @@ class AttachmentManagerTestCase(test.TestCase):
'cacheable': True,
'access_mode': 'rw',
'driver_volume_type': 'iscsi',
'attachment_id': attachment_ref.id}
'attachment_id': attachment_ref.id,
'enforce_multipath': enforce_mpath}
get_extra_specs.return_value = {'cacheable': '<is> True'}
self.assertEqual(expected,

44
cinder/tests/unit/volume/test_driver.py
View File

@ -383,6 +383,47 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase):
db.volume_destroy(self.context, src_vol['id'])
db.volume_destroy(self.context, dest_vol['id'])
@ddt.data(False, True)
@mock.patch(driver_name + '.initialize_connection')
@mock.patch(driver_name + '.create_export', return_value=None)
@mock.patch(driver_name + '._connect_device')
def test_attach_volume_enforce_mpath(self, enforce_mpath, connect_mock,
export_mock, initialize_mock):
properties = {'host': 'myhost', 'ip': '192.168.1.43',
'initiator': u'iqn.1994-05.com.redhat:d9be887375',
'multipath': False, 'os_type': 'linux2',
'platform': 'x86_64',
'enforce_multipath': enforce_mpath}
data = {'target_discovered': True,
'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
'target_portal': '127.0.0.0.1:3260',
'volume_id': 1,
'discard': False,
'encrypted': False,
'enforce_multipath': enforce_mpath}
passed_conn = {'driver_volume_type': 'iscsi', 'data': data.copy()}
initialize_mock.return_value = passed_conn
expected_conn = {'driver_volume_type': 'iscsi', 'data': data.copy()}
volume = tests_utils.create_volume(
self.context, status='available',
size=2)
attach_info, vol = self.volume.driver._attach_volume(self.context,
volume,
properties)
export_mock.assert_called_once_with(self.context, volume, properties)
initialize_mock.assert_called_once_with(volume, properties)
connect_mock.assert_called_once_with(expected_conn)
self.assertEqual(connect_mock.return_value, attach_info)
self.assertEqual(volume, vol)
@mock.patch(driver_name + '.initialize_connection')
@mock.patch(driver_name + '.create_export', return_value=None)
@mock.patch(driver_name + '._connect_device')
@ -397,7 +438,8 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase):
'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
'target_portal': '127.0.0.0.1:3260',
'volume_id': 1,
'discard': False}
'discard': False,
'enforce_multipath': False}
passed_conn = {'driver_volume_type': 'iscsi', 'data': data.copy()}
initialize_mock.return_value = passed_conn

4
cinder/volume/driver.py
View File

@ -1087,6 +1087,10 @@ class BaseVD(object, metaclass=abc.ABCMeta):
encrypted = bool(volume.encryption_key_id)
conn['data']['encrypted'] = encrypted
# Append the enforce_multipath value if the connector has it
conn['data']['enforce_multipath'] = properties.get(
'enforce_multipath', False)
try:
attach_info = self._connect_device(conn)
except Exception as exc:

7
cinder/volume/manager.py
View File

@ -1980,6 +1980,8 @@ class VolumeManager(manager.CleanableManager,
raise exception.VolumeBackendAPIException(data=err_msg)
conn_info = self._parse_connection_options(context, volume, conn_info)
conn_info['data']['enforce_multipath'] = connector.get(
'enforce_multipath', False)
LOG.info("Initialize volume connection completed successfully.",
resource=volume)
return conn_info
@ -2040,6 +2042,8 @@ class VolumeManager(manager.CleanableManager,
LOG.error(ex_msg)
raise exception.VolumeBackendAPIException(data=ex_msg)
raise exception.VolumeBackendAPIException(data=err_msg)
conn['data']['enforce_multipath'] = connector.get(
'enforce_multipath', False)
LOG.info("Initialize snapshot connection completed successfully.",
resource=snapshot)
@ -4851,6 +4855,9 @@ class VolumeManager(manager.CleanableManager,
self.db.volume_attachment_update(ctxt, attachment.id, values)
connection_info['attachment_id'] = attachment.id
# Append the enforce_multipath value if the connector has it
connection_info['enforce_multipath'] = connector.get(
'enforce_multipath', False)
LOG.debug("Connection info returned from driver %(connection_info)s",
{'connection_info':
strutils.mask_dict_password(connection_info)})