openstack/cinder
Code Issues Proposed changes

Add support volume local cache

Browse Source 
Use volume type extra spec 'cacheable' to identify if a volume can be
cached or not. If it is set to '<is> True', then it means the volume can
be cached in compute node locally via caching software (e.g. open-cas)

open-cas: https://open-cas.github.io/
Nova Spec: https://review.opendev.org/#/c/689070/
Cinder Spec: https://review.opendev.org/#/c/684556/

Change-Id: I61a795f4ab2c4208094245bd577c39de1b35d6ef
Signed-off-by: Liang Fang <liang.a.fang@intel.com>
This commit is contained in:
Liang Fang 2020-04-11 09:42:49 +00:00
parent 61e24eb8ab
commit afcaf0b9dd
8 changed files with 188 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
cinder/api/contrib/types_extra_specs.py
View File

@ -63,6 +63,20 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
raise webob.exc.HTTPBadRequest(explanation=expl)
return
def _check_cacheable(self, specs, type_id):
multiattach = volume_types.get_volume_type_extra_specs(
type_id, key='multiattach')
cacheable = volume_types.get_volume_type_extra_specs(
type_id, key='cacheable')
isTrue = '<is> True'
if (specs.get('multiattach') == isTrue and cacheable == isTrue) or (
specs.get('cacheable') == isTrue and multiattach ==
isTrue) or (specs.get('cacheable') == isTrue and
specs.get('multiattach') == isTrue):
expl = _('cacheable cannot be set with multiattach.')
raise webob.exc.HTTPBadRequest(explanation=expl)
return
@validation.schema(types_extra_specs.create)
def create(self, req, type_id, body):
context = req.environ['cinder.context']
@ -76,6 +90,9 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
image_service_store_id = specs['image_service:store_id']
image_utils.validate_stores_id(context, image_service_store_id)
# Check if multiattach be set with cacheable
self._check_cacheable(specs, type_id)
db.volume_type_extra_specs_update_or_create(context,
type_id,
specs)
@ -104,6 +121,11 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
image_service_store_id = body['image_service:store_id']
image_utils.validate_stores_id(context, image_service_store_id)
if 'extra_specs' in body:
specs = body['extra_specs']
# Check if multiattach be set with cacheable
self._check_cacheable(specs, type_id)
db.volume_type_extra_specs_update_or_create(context,
type_id,
body)

53
cinder/tests/unit/api/contrib/test_types_extra_specs.py
View File

@ -454,3 +454,56 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
self.assertRaises(exception.ValidationError,
self.controller.create, req, fake.VOLUME_ID,
body=body)
@mock.patch('cinder.volume.volume_types.get_volume_type_extra_specs')
def test_check_cacheable(self, get_extra_specs):
ret_multiattach = ''
ret_cacheable = ''
def side_get_specs(type_id, key):
if key == 'multiattach':
return ret_multiattach
if key == 'cacheable':
return ret_cacheable
get_extra_specs.return_value = ''
get_extra_specs.side_effect = side_get_specs
specs = {'multiattach': '<is> True',
'cacheable': '<is> True'}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._check_cacheable,
specs, 'typeid')
ret_multiattach = '<is> True'
ret_cacheable = ''
specs = {'cacheable': '<is> True'}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._check_cacheable,
specs, 'typeid')
ret_multiattach = ''
ret_cacheable = '<is> True'
specs = {'multiattach': '<is> True'}
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._check_cacheable,
specs, 'typeid')
ret_multiattach = '<is> False'
ret_cacheable = ''
specs = {'multiattach': '<is> True'}
# Should NOT has exception when calling below line
self.controller._check_cacheable(specs, 'typeid')
ret_multiattach = '<is> True'
ret_cacheable = ''
specs = {'multiattach': '<is> False', 'cacheable': '<is> True'}
# Should NOT setting both at the same time
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._check_cacheable,
specs, 'typeid')
ret_multiattach = '<is> False'
ret_cacheable = ''
specs = {'multiattach': '<is> False', 'cacheable': '<is> True'}
# Should NOT has exception when calling below line
self.controller._check_cacheable(specs, 'typeid')

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

@ -48,7 +48,8 @@ class AttachmentManagerTestCase(test.TestCase):
@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')
def test_attachment_update(self, mock_type_get):
@mock.patch('cinder.volume.volume_types.get_volume_type_extra_specs')
def test_attachment_update(self, get_extra_specs, mock_type_get):
"""Test attachment_update."""
volume_params = {'status': 'available'}
connector = {
@ -74,10 +75,12 @@ class AttachmentManagerTestCase(test.TestCase):
expected = {
'encrypted': False,
'qos_specs': None,
'cacheable': False,
'access_mode': 'rw',
'driver_volume_type': 'iscsi',
'attachment_id': attachment_ref.id}
get_extra_specs.return_value = ''
self.assertEqual(expected,
self.manager.attachment_update(
self.context,
@ -89,6 +92,21 @@ class AttachmentManagerTestCase(test.TestCase):
attachment_ref.instance_uuid,
connector['host'],
"na")
expected = {
'encrypted': False,
'qos_specs': None,
'cacheable': True,
'access_mode': 'rw',
'driver_volume_type': 'iscsi',
'attachment_id': attachment_ref.id}
get_extra_specs.return_value = '<is> True'
self.assertEqual(expected,
self.manager.attachment_update(
self.context,
vref,
connector,
attachment_ref.id))
new_attachment_ref = db.volume_attachment_get(self.context,
attachment_ref.id)

16
cinder/tests/unit/volume/test_connection.py
View File

@ -152,9 +152,13 @@ class VolumeConnectionTestCase(base.BaseVolumeTestCase):
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type_qos_specs') as type_qos, \
mock.patch.object(cinder.volume.volume_types,
'get_volume_type_extra_specs'
) as type_extra_specs, \
mock.patch.object(cinder.tests.fake_driver.FakeLoggingVolumeDriver,
'initialize_connection') as driver_init:
type_qos.return_value = dict(qos_specs=qos_values)
type_extra_specs.return_value = 'True'
driver_init.return_value = {'data': {}}
mock_get_target.return_value = None
qos_specs_expected = {'key1': 'value1',
@ -218,9 +222,13 @@ class VolumeConnectionTestCase(base.BaseVolumeTestCase):
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type_qos_specs') as type_qos, \
mock.patch.object(cinder.volume.volume_types,
'get_volume_type_extra_specs'
) as type_extra_specs, \
mock.patch.object(cinder.tests.fake_driver.FakeLoggingVolumeDriver,
'initialize_connection') as driver_init:
type_qos.return_value = dict(qos_specs=qos_values)
type_extra_specs.return_value = 'True'
driver_init.return_value = {'data': {}}
mock_get_target.return_value = None
qos_specs_expected = {'write_iops_sec': 90,
@ -286,9 +294,13 @@ class VolumeConnectionTestCase(base.BaseVolumeTestCase):
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type_qos_specs') as type_qos, \
mock.patch.object(cinder.volume.volume_types,
'get_volume_type_extra_specs'
) as type_extra_specs, \
mock.patch.object(cinder.tests.fake_driver.FakeLoggingVolumeDriver,
'initialize_connection') as driver_init:
type_qos.return_value = dict(qos_specs=qos_values)
type_extra_specs.return_value = 'True'
driver_init.return_value = {'data': {}}
mock_get_target.return_value = None
qos_specs_expected = {'write_iops_sec': 800,
@ -354,9 +366,13 @@ class VolumeConnectionTestCase(base.BaseVolumeTestCase):
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type_qos_specs') as type_qos, \
mock.patch.object(cinder.volume.volume_types,
'get_volume_type_extra_specs'
) as type_extra_specs, \
mock.patch.object(cinder.tests.fake_driver.FakeLoggingVolumeDriver,
'initialize_connection') as driver_init:
type_qos.return_value = dict(qos_specs=qos_values)
type_extra_specs.return_value = 'True'
driver_init.return_value = {'data': {}}
mock_get_target.return_value = None
qos_specs_expected = {'write_iops_sec': 3000,

6
cinder/tests/unit/volume/test_volume.py
View File

@ -172,6 +172,8 @@ class VolumeTestCase(base.BaseVolumeTestCase):
myfilterfunction = "myFilterFunction"
mygoodnessfunction = "myGoodnessFunction"
expected = {'name': 'cinder-volumes',
'storage_protocol': 'iSCSI',
'cacheable': True,
'filter_function': myfilterfunction,
'goodness_function': mygoodnessfunction,
}
@ -181,7 +183,9 @@ class VolumeTestCase(base.BaseVolumeTestCase):
'get_goodness_function') as m_get_goodness:
with mock.patch.object(manager.driver,
'get_filter_function') as m_get_filter:
m_get_stats.return_value = {'name': 'cinder-volumes'}
m_get_stats.return_value = {'name': 'cinder-volumes',
'storage_protocol': 'iSCSI',
}
m_get_filter.return_value = myfilterfunction
m_get_goodness.return_value = mygoodnessfunction
manager._report_driver_status(context.get_admin_context())

42
cinder/tests/unit/volume/test_volume_manager.py
View File

@ -18,6 +18,7 @@ from unittest import mock
from cinder import exception
from cinder.message import message_field
from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import fake_volume
from cinder.tests.unit import volume as base
from cinder.volume import manager as vol_manager
@ -245,3 +246,44 @@ class VolumeManagerTestCase(base.BaseVolumeTestCase):
mock_detach.assert_called_once_with(
ctxt, mock_connect.return_value, vol, mock.sentinel.properties,
force=True, remote=mock.sentinel.remote)
@mock.patch('cinder.volume.volume_types.get_volume_type_extra_specs')
@mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs',
return_value={'qos_specs': None})
def test_parse_connection_options_cacheable(self,
mock_get_qos,
mock_get_extra_specs):
ctxt = mock.Mock()
manager = vol_manager.VolumeManager()
vol = fake_volume.fake_volume_obj(ctxt)
vol.volume_type_id = fake.VOLUME_TYPE_ID
# no 'cacheable' set by driver, should be extra spec
conn_info = {"data": {}}
mock_get_extra_specs.return_value = '<is> True'
manager._parse_connection_options(ctxt, vol, conn_info)
self.assertIn('cacheable', conn_info['data'])
self.assertIs(conn_info['data']['cacheable'], True)
# driver sets 'cacheable' False, should override extra spec
conn_info = {"data": {"cacheable": False}}
mock_get_extra_specs.return_value = '<is> True'
manager._parse_connection_options(ctxt, vol, conn_info)
self.assertIn('cacheable', conn_info['data'])
self.assertIs(conn_info['data']['cacheable'], False)
# driver sets 'cacheable' True, nothing in extra spec,
# extra spec should override driver
conn_info = {"data": {"cacheable": True}}
mock_get_extra_specs.return_value = None
manager._parse_connection_options(ctxt, vol, conn_info)
self.assertIn('cacheable', conn_info['data'])
self.assertIs(conn_info['data']['cacheable'], False)
# driver sets 'cacheable' True, extra spec has False,
# extra spec should override driver
conn_info = {"data": {"cacheable": True}}
mock_get_extra_specs.return_value = '<is> False'
manager._parse_connection_options(ctxt, vol, conn_info)
self.assertIn('cacheable', conn_info['data'])
self.assertIs(conn_info['data']['cacheable'], False)

7
cinder/volume/driver.py
View File

@ -1464,6 +1464,13 @@ class BaseVD(object):
def initialize_connection(self, volume, connector):
"""Allow connection to connector and return connection info.
..note::
Whether or not a volume is 'cacheable' for volume local cache on
the hypervisor is normally configured in the volume-type
extra-specs. Support may be disabled at the driver level, however,
by returning "cacheable": False in the conn_info. This will
override any setting in the volume-type extra-specs.
:param volume: The volume to be attached
:param connector: Dictionary containing information about what is being
connected to.

24
cinder/volume/manager.py
View File

@ -1760,6 +1760,18 @@ class VolumeManager(manager.CleanableManager,
encrypted = bool(volume.encryption_key_id)
conn_info['data']['encrypted'] = encrypted
# Add cacheable flag to connection_info if not set in the driver.
if typeid:
cacheable = volume_types.get_volume_type_extra_specs(
typeid, key='cacheable')
if conn_info['data'].get('cacheable') is not None:
driver_setting = bool(conn_info['data']['cacheable'])
# override a True driver_setting but respect False
conn_info['data']['cacheable'] = (driver_setting and
(cacheable == '<is> True'))
else:
conn_info['data']['cacheable'] = (cacheable == '<is> True')
# Add discard flag to connection_info if not set in the driver and
# configured to be reported.
if conn_info['data'].get('discard') is None:
@ -2613,6 +2625,18 @@ class VolumeManager(manager.CleanableManager,
# Append volume stats with 'allocated_capacity_gb'
self._append_volume_stats(volume_stats)
# Append cacheable flag for iSCSI/FC/NVMe-oF and only when
# cacheable is not set in driver level
if volume_stats['storage_protocol'] in [
'iSCSI', 'FC', 'NVMe-oF']:
if volume_stats.get('pools'):
for pool in volume_stats.get('pools'):
if pool.get('cacheable') is None:
pool['cacheable'] = True
else:
if volume_stats.get('cacheable') is None:
volume_stats['cacheable'] = True
# Append filter and goodness function if needed
volume_stats = (
self._append_filter_goodness_functions(volume_stats))