openstack/cinder
Code Issues Proposed changes

Merge "Move require_driver_initialized / log_unsupp to volume_utils"

Browse Source
This commit is contained in:
Zuul 2021-07-09 13:17:46 +00:00 committed by Gerrit Code Review
commit 7a9a914df1
8 changed files with 100 additions and 131 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
cinder/tests/unit/test_utils.py
View File

@ -108,16 +108,6 @@ class GenericUtilsTestCase(test.TestCase):
unit=False,
pretty_keys=False)
def test_require_driver_intialized(self):
driver = mock.Mock()
driver.initialized = True
utils.require_driver_initialized(driver)
driver.initialized = False
self.assertRaises(exception.DriverNotInitialized,
utils.require_driver_initialized,
driver)
def test_hostname_unicode_sanitization(self):
hostname = u"\u7684.test.example.com"
self.assertEqual("test.example.com",

10
cinder/tests/unit/test_volume_utils.py
View File

@ -1232,6 +1232,16 @@ class VolumeUtilsTestCase(test.TestCase):
mock_configuration.assert_called_with(driver.volume_opts,
config_group='backendA')
def test_require_driver_initialized(self):
driver = mock.Mock()
driver.initialized = True
volume_utils.require_driver_initialized(driver)
driver.initialized = False
self.assertRaises(exception.DriverNotInitialized,
volume_utils.require_driver_initialized,
driver)
@ddt.ddt
class LogTracingTestCase(test.TestCase):

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

@ -663,8 +663,10 @@ class ReplicationTestCase(base.BaseVolumeTestCase):
[], [], [],
self.manager.FAILBACK_SENTINEL)
@mock.patch('cinder.utils.log_unsupported_driver_warning', mock.Mock())
@mock.patch('cinder.utils.require_driver_initialized', mock.Mock())
@mock.patch('cinder.volume.volume_utils.log_unsupported_driver_warning',
mock.Mock())
@mock.patch('cinder.volume.volume_utils.require_driver_initialized',
mock.Mock())
def test_init_host_with_rpc_clustered_replication(self):
# These are not OVOs but ORM instances
cluster = utils.create_cluster(self.context)

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

@ -27,7 +27,7 @@ from cinder.volume import manager as vol_manager
class VolumeManagerTestCase(base.BaseVolumeTestCase):
@mock.patch('cinder.message.api.API.create')
@mock.patch('cinder.utils.require_driver_initialized')
@mock.patch('cinder.volume.volume_utils.require_driver_initialized')
@mock.patch('cinder.volume.manager.VolumeManager.'
'_notify_about_snapshot_usage')
def test_create_snapshot_driver_not_initialized_generates_user_message(
@ -54,7 +54,7 @@ class VolumeManagerTestCase(base.BaseVolumeTestCase):
detail=message_field.Detail.SNAPSHOT_CREATE_ERROR)
@mock.patch('cinder.message.api.API.create')
@mock.patch('cinder.utils.require_driver_initialized')
@mock.patch('cinder.volume.volume_utils.require_driver_initialized')
@mock.patch('cinder.volume.manager.VolumeManager.'
'_notify_about_snapshot_usage')
def test_create_snapshot_metadata_update_failure_generates_user_message(
@ -92,7 +92,7 @@ class VolumeManagerTestCase(base.BaseVolumeTestCase):
detail=message_field.Detail.SNAPSHOT_UPDATE_METADATA_FAILED)
@mock.patch('cinder.message.api.API.create')
@mock.patch('cinder.utils.require_driver_initialized')
@mock.patch('cinder.volume.volume_utils.require_driver_initialized')
@mock.patch('cinder.volume.manager.VolumeManager.'
'_notify_about_snapshot_usage')
def test_delete_snapshot_when_busy_generates_user_message(
@ -116,7 +116,7 @@ class VolumeManagerTestCase(base.BaseVolumeTestCase):
exception=fake_exp)
@mock.patch('cinder.message.api.API.create')
@mock.patch('cinder.utils.require_driver_initialized')
@mock.patch('cinder.volume.volume_utils.require_driver_initialized')
@mock.patch('cinder.volume.manager.VolumeManager.'
'_notify_about_snapshot_usage')
def test_delete_snapshot_general_exception_generates_user_message(

14
cinder/tests/unit/zonemanager/test_volume_driver.py
View File

@ -23,8 +23,8 @@ from unittest import mock
from cinder.tests import fake_driver
from cinder.tests.unit import test
from cinder import utils
from cinder.volume import configuration as conf
from cinder.volume import volume_utils
from cinder.zonemanager.drivers.brocade import brcd_fc_zone_driver
from cinder.zonemanager import fc_zone_manager
@ -44,7 +44,7 @@ class TestVolumeDriver(test.TestCase):
super(TestVolumeDriver, self).__init__(*args, **kwargs)
@mock.patch('oslo_config.cfg._is_opt_registered', return_value=False)
@mock.patch.object(utils, 'require_driver_initialized')
@mock.patch.object(volume_utils, 'require_driver_initialized')
def test_initialize_connection_with_decorator(self, utils_mock, opt_mock):
utils_mock.return_value = True
with mock.patch.object(fc_zone_manager.ZoneManager, 'add_connection')\
@ -57,7 +57,7 @@ class TestVolumeDriver(test.TestCase):
@mock.patch('cinder.zonemanager.utils.create_zone_manager')
@mock.patch('oslo_config.cfg._is_opt_registered', return_value=False)
@mock.patch.object(utils, 'require_driver_initialized')
@mock.patch.object(volume_utils, 'require_driver_initialized')
def test_initialize_connection_with_decorator_and_empty_map(
self, utils_mock, opt_mock, zm_create_mock):
utils_mock.return_value = True
@ -67,7 +67,7 @@ class TestVolumeDriver(test.TestCase):
zm_create_mock.assert_not_called()
add_zone_mock.assert_not_called()
@mock.patch.object(utils, 'require_driver_initialized')
@mock.patch.object(volume_utils, 'require_driver_initialized')
def test_initialize_connection_no_decorator(self, utils_mock):
utils_mock.return_value = True
with mock.patch.object(fc_zone_manager.ZoneManager, 'add_connection')\
@ -79,7 +79,7 @@ class TestVolumeDriver(test.TestCase):
add_zone_mock.assert_not_called()
@mock.patch('oslo_config.cfg._is_opt_registered', return_value=False)
@mock.patch.object(utils, 'require_driver_initialized')
@mock.patch.object(volume_utils, 'require_driver_initialized')
def test_terminate_connection_with_decorator(self, utils_mock, opt_mock):
utils_mock.return_value = True
with mock.patch.object(fc_zone_manager.ZoneManager,
@ -92,7 +92,7 @@ class TestVolumeDriver(test.TestCase):
@mock.patch('cinder.zonemanager.utils.create_zone_manager')
@mock.patch('oslo_config.cfg._is_opt_registered', return_value=False)
@mock.patch.object(utils, 'require_driver_initialized')
@mock.patch.object(volume_utils, 'require_driver_initialized')
def test_terminate_connection_with_decorator_and_empty_map(
self, utils_mock, opt_mock, zm_create_mock):
utils_mock.return_value = True
@ -102,7 +102,7 @@ class TestVolumeDriver(test.TestCase):
zm_create_mock.assert_not_called()
remove_zone_mock.assert_not_called()
@mock.patch.object(utils, 'require_driver_initialized')
@mock.patch.object(volume_utils, 'require_driver_initialized')
def test_terminate_connection_no_decorator(self, utils_mock):
utils_mock.return_value = True
with mock.patch.object(fc_zone_manager.ZoneManager,

40
cinder/utils.py
View File

@ -15,7 +15,15 @@
# License for the specific language governing permissions and limitations
# under the License.
"""Utilities and helper functions."""
"""Utilities and helper functions for all Cinder code.
This file is for utilities useful in all of Cinder,
including cinder-manage, the api service, the scheduler,
etc.
Code related to volume drivers and connecting to volumes
should be placed in volume_utils instead.
"""
from collections import OrderedDict
import contextlib
@ -424,36 +432,6 @@ def get_root_helper() -> str:
return 'sudo cinder-rootwrap %s' % CONF.rootwrap_config
def require_driver_initialized(driver) -> None:
"""Verifies if `driver` is initialized
If the driver is not initialized, an exception will be raised.
:params driver: The driver instance.
:raises: `exception.DriverNotInitialized`
"""
# we can't do anything if the driver didn't init
if not driver.initialized:
driver_name = driver.__class__.__name__
LOG.error("Volume driver %s not initialized", driver_name)
raise exception.DriverNotInitialized()
else:
log_unsupported_driver_warning(driver)
def log_unsupported_driver_warning(driver) -> None:
"""Annoy the log about unsupported drivers."""
if not driver.supported:
# Check to see if the driver is flagged as supported.
LOG.warning("Volume driver (%(driver_name)s %(version)s) is "
"currently unsupported and may be removed in the "
"next release of OpenStack. Use at your own risk.",
{'driver_name': driver.__class__.__name__,
'version': driver.get_version()},
resource={'type': 'driver',
'id': driver.__class__.__name__})
def get_file_mode(path: str) -> int:
"""This primarily exists to make unit testing easier."""
return stat.S_IMODE(os.stat(path).st_mode)

113
cinder/volume/manager.py
View File

@ -74,7 +74,6 @@ from cinder.objects import cgsnapshot
from cinder.objects import consistencygroup
from cinder.objects import fields
from cinder import quota
from cinder import utils
from cinder import volume as cinder_volume
from cinder.volume import configuration as config
from cinder.volume.flows.manager import create_volume
@ -443,7 +442,7 @@ class VolumeManager(manager.CleanableManager,
**kwargs) -> None:
"""Perform any required initialization."""
if not self.driver.supported:
utils.log_unsupported_driver_warning(self.driver)
volume_utils.log_unsupported_driver_warning(self.driver)
if not self.configuration.enable_unsupported_driver:
LOG.error("Unsupported drivers are disabled."
@ -607,8 +606,8 @@ class VolumeManager(manager.CleanableManager,
try:
# Make sure the driver is initialized first
utils.log_unsupported_driver_warning(self.driver)
utils.require_driver_initialized(self.driver)
volume_utils.log_unsupported_driver_warning(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
LOG.error("Cannot complete RPC initialization because "
"driver isn't initialized properly.",
@ -701,7 +700,7 @@ class VolumeManager(manager.CleanableManager,
allow_reschedule=True) -> ovo_fields.UUIDField:
"""Creates the volume."""
# Log about unsupported drivers
utils.log_unsupported_driver_warning(self.driver)
volume_utils.log_unsupported_driver_warning(self.driver)
# Make sure the host in the DB matches our own when clustered
self._set_resource_host(volume)
@ -894,10 +893,7 @@ class VolumeManager(manager.CleanableManager,
if not is_temp_vol:
self._notify_about_volume_usage(context, volume, notification)
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
self.driver.remove_export(context, volume)
if unmanage_only:
@ -1159,10 +1155,7 @@ class VolumeManager(manager.CleanableManager,
context, snapshot, "create.start")
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the snapshot status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
# Pass context so that drivers that want to use it, can,
# but it is not a requirement for all drivers.
@ -1241,10 +1234,7 @@ class VolumeManager(manager.CleanableManager,
context, snapshot, "delete.start")
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the snapshot status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
# Pass context so that drivers that want to use it, can,
# but it is not a requirement for all drivers.
@ -1373,10 +1363,7 @@ class VolumeManager(manager.CleanableManager,
if volume_metadata.get('readonly') == 'True' and mode != 'ro':
raise exception.InvalidVolumeAttachMode(mode=mode,
volume_id=volume.id)
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
LOG.info('Attaching volume %(volume_id)s to instance '
'%(instance)s at mountpoint %(mount)s on host '
@ -1460,10 +1447,7 @@ class VolumeManager(manager.CleanableManager,
self._notify_about_volume_usage(context, volume, "detach.start")
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
LOG.info('Detaching volume %(volume_id)s from instance '
'%(instance)s.',
@ -1486,7 +1470,7 @@ class VolumeManager(manager.CleanableManager,
# We're going to remove the export here
# (delete the iscsi target)
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
self.driver.remove_export(context.elevated(), volume)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
@ -1692,10 +1676,7 @@ class VolumeManager(manager.CleanableManager,
try:
volume = objects.Volume.get_by_id(context, volume_id)
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
image_service, image_id = \
glance.get_remote_image_service(context, image_meta['id'])
@ -1868,12 +1849,8 @@ class VolumeManager(manager.CleanableManager,
various places, so it should not contain any non-json data types.
"""
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
# TODO(jdg): Add deprecation warning
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
try:
self.driver.validate_connector(connector)
except exception.InvalidConnectorException as err:
@ -1924,7 +1901,7 @@ class VolumeManager(manager.CleanableManager,
ctxt,
snapshot_id: ovo_fields.UUIDField,
connector: dict) -> dict:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
snapshot = objects.Snapshot.get_by_id(ctxt, snapshot_id)
try:
self.driver.validate_connector(connector)
@ -1990,10 +1967,7 @@ class VolumeManager(manager.CleanableManager,
The format of connector is the same as for initialize_connection.
"""
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
volume_ref = self.db.volume_get(context, volume_id)
try:
@ -2012,7 +1986,7 @@ class VolumeManager(manager.CleanableManager,
snapshot_id: ovo_fields.UUIDField,
connector: dict,
force=False) -> None:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
snapshot = objects.Snapshot.get_by_id(ctxt, snapshot_id)
try:
@ -2028,7 +2002,7 @@ class VolumeManager(manager.CleanableManager,
def remove_export(self, context, volume_id: ovo_fields.UUIDField) -> None:
"""Removes an export for a volume."""
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
volume_ref = self.db.volume_get(context, volume_id)
try:
self.driver.remove_export(context, volume_ref)
@ -2044,7 +2018,7 @@ class VolumeManager(manager.CleanableManager,
ctxt,
snapshot_id: ovo_fields.UUIDField) -> None:
"""Removes an export for a snapshot."""
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
snapshot = objects.Snapshot.get_by_id(ctxt, snapshot_id)
try:
self.driver.remove_export_snapshot(ctxt, snapshot)
@ -2058,10 +2032,7 @@ class VolumeManager(manager.CleanableManager,
def accept_transfer(self, context, volume_id, new_user, new_project,
no_snapshots=False) -> dict:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
# NOTE(jdg): need elevated context as we haven't "given" the vol
# yet
@ -2410,10 +2381,7 @@ class VolumeManager(manager.CleanableManager,
new_volume,
error=False) -> ovo_fields.UUIDField:
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the migration status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
volume.migration_status = 'error'
@ -2584,10 +2552,7 @@ class VolumeManager(manager.CleanableManager,
diff=None) -> None:
"""Migrate the volume to the specified host (called on source host)."""
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the migration status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
volume.migration_status = 'error'
@ -2868,10 +2833,7 @@ class VolumeManager(manager.CleanableManager,
new_size: int,
reservations) -> None:
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
volume.status = 'error_extending'
@ -2975,10 +2937,7 @@ class VolumeManager(manager.CleanableManager,
project_id = context.project_id
try:
# NOTE(flaper87): Verify the driver is enabled
# before going forward. The exception will be caught
# and the volume status updated.
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
# NOTE(flaper87): Other exceptions in this method don't
@ -3210,7 +3169,7 @@ class VolumeManager(manager.CleanableManager,
def get_manageable_volumes(self, ctxt, marker, limit, offset, sort_keys,
sort_dirs, want_objects=False):
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
LOG.exception("Listing manageable volumes failed, due "
@ -3247,7 +3206,7 @@ class VolumeManager(manager.CleanableManager,
self._notify_about_group_usage(context, group, "create.start")
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
LOG.info("Group %s: creating", group.name)
@ -3376,7 +3335,7 @@ class VolumeManager(manager.CleanableManager,
self._notify_about_group_usage(
context, group, "create.start")
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
try:
model_update, volumes_model_update = (
@ -3658,7 +3617,7 @@ class VolumeManager(manager.CleanableManager,
volumes_model_update = None
model_update = None
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
try:
model_update, volumes_model_update = (
@ -3905,7 +3864,7 @@ class VolumeManager(manager.CleanableManager,
context, group, "update.start")
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
try:
model_update, add_volumes_update, remove_volumes_update = (
@ -4004,7 +3963,7 @@ class VolumeManager(manager.CleanableManager,
snapshots_model_update = None
model_update = None
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
LOG.debug("Group snapshot %(grp_snap_id)s: creating.",
{'grp_snap_id': group_snapshot.id})
@ -4173,7 +4132,7 @@ class VolumeManager(manager.CleanableManager,
snapshots_model_update = None
model_update = None
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
LOG.debug("group_snapshot %(grp_snap_id)s: deleting",
{'grp_snap_id': group_snapshot.id})
@ -4613,7 +4572,7 @@ class VolumeManager(manager.CleanableManager,
def get_manageable_snapshots(self, ctxt, marker, limit, offset,
sort_keys, sort_dirs, want_objects=False):
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
except exception.DriverNotInitialized:
with excutils.save_and_reraise_exception():
LOG.exception("Listing manageable snapshots failed, due "
@ -4754,7 +4713,7 @@ class VolumeManager(manager.CleanableManager,
connector)
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
self.driver.attach_volume(context,
vref,
attachment_ref.instance_uuid,
@ -4794,7 +4753,7 @@ class VolumeManager(manager.CleanableManager,
Exits early if the attachment does not have a connector and returns
None to indicate shared connections are irrelevant.
"""
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
connector = attachment.connector
if not connector and not force:
# It's possible to attach a volume to a shelved offloaded server
@ -4840,7 +4799,7 @@ class VolumeManager(manager.CleanableManager,
param: attachment_id: Attachment id to remove
param: vref: Volume object associated with the attachment
"""
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
attachment = objects.VolumeAttachment.get_by_id(context, attachment_id)
self._notify_about_volume_usage(context, vref, "detach.start")
@ -4898,7 +4857,7 @@ class VolumeManager(manager.CleanableManager,
volumes_model_update = None
model_update = None
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
model_update, volumes_model_update = (
self.driver.enable_replication(ctxt, group, volumes))
@ -4982,7 +4941,7 @@ class VolumeManager(manager.CleanableManager,
volumes_model_update = None
model_update = None
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
model_update, volumes_model_update = (
self.driver.disable_replication(ctxt, group, volumes))
@ -5073,7 +5032,7 @@ class VolumeManager(manager.CleanableManager,
volumes_model_update = None
model_update = None
try:
utils.require_driver_initialized(self.driver)
volume_utils.require_driver_initialized(self.driver)
model_update, volumes_model_update = (
self.driver.failover_replication(

30
cinder/volume/volume_utils.py
View File

@ -1634,3 +1634,33 @@ def setup_tracing(trace_flags):
LOG.warning('Invalid trace flag: %s', invalid_flag)
TRACE_METHOD = 'method' in trace_flags
TRACE_API = 'api' in trace_flags
def require_driver_initialized(driver):
"""Verifies if `driver` is initialized
If the driver is not initialized, an exception will be raised.
:params driver: The driver instance.
:raises: `exception.DriverNotInitialized`
"""
# we can't do anything if the driver didn't init
if not driver.initialized:
driver_name = driver.__class__.__name__
LOG.error("Volume driver %s not initialized", driver_name)
raise exception.DriverNotInitialized()
else:
log_unsupported_driver_warning(driver)
def log_unsupported_driver_warning(driver):
"""Annoy the log about unsupported drivers."""
if not driver.supported:
# Check to see if the driver is flagged as supported.
LOG.warning("Volume driver (%(driver_name)s %(version)s) is "
"currently unsupported and may be removed in the "
"next release of OpenStack. Use at your own risk.",
{'driver_name': driver.__class__.__name__,
'version': driver.get_version()},
resource={'type': 'driver',
'id': driver.__class__.__name__})