diff options
Diffstat (limited to 'cinder')
24 files changed, 485 insertions, 318 deletions
diff --git a/cinder/backup/chunkeddriver.py b/cinder/backup/chunkeddriver.py index 6d28fc399..b5dcf5d45 100644 --- a/cinder/backup/chunkeddriver.py +++ b/cinder/backup/chunkeddriver.py @@ -555,6 +555,7 @@ class ChunkedBackupDriver(driver.BackupDriver, metaclass=abc.ABCMeta): 'backup. Do a full backup.') raise exception.InvalidBackup(reason=err) + win32_disk_size = None if sys.platform == 'win32': # When dealing with Windows physical disks, we need the exact # size of the disk. Attempting to read passed this boundary will @@ -602,7 +603,7 @@ class ChunkedBackupDriver(driver.BackupDriver, metaclass=abc.ABCMeta): break data_offset = volume_file.tell() - if sys.platform == 'win32': + if win32_disk_size is not None: read_bytes = min(self.chunk_size_bytes, win32_disk_size - data_offset) else: diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index 5df343609..761d61ec9 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -314,7 +314,7 @@ class Volume(BASE, CinderBase): __tablename__ = 'volumes' __table_args__ = ( - sa.Index('volumes_service_uuid_idx', 'deleted', 'service_uuid'), + sa.Index('volumes_service_uuid_idx', 'service_uuid', 'deleted'), # Speed up normal listings sa.Index('volumes_deleted_project_id_idx', 'deleted', 'project_id'), # Speed up service start, create volume from image when using direct diff --git a/cinder/exception.py b/cinder/exception.py index 906ed3923..dddcbc68a 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -1092,3 +1092,7 @@ class DriverInitiatorDataExists(Duplicate): "Driver initiator data for initiator '%(initiator)s' and backend " "'%(namespace)s' with key '%(key)s' already exists." ) + + +class RequirementMissing(CinderException): + message = _('Requirement %(req)s is not installed.') diff --git a/cinder/ssh_utils.py b/cinder/ssh_utils.py index 6bd3fb731..bef393e28 100644 --- a/cinder/ssh_utils.py +++ b/cinder/ssh_utils.py @@ -24,7 +24,11 @@ from eventlet import pools from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils -import paramiko + +try: + import paramiko +except ImportError: + paramiko = None from cinder import exception from cinder.i18n import _ @@ -65,6 +69,9 @@ class SSHPool(pools.Pool): self.hosts_key_file = None self.current_size = 0 + if paramiko is None: + raise exception.RequirementMissing(req='paramiko') + # Validate good config setting here. # Paramiko handles the case where the file is inaccessible. if not CONF.ssh_hosts_key_file: diff --git a/cinder/tests/unit/attachments/test_attachments_manager.py b/cinder/tests/unit/attachments/test_attachments_manager.py index 6e5def3d3..ab722e1bf 100644 --- a/cinder/tests/unit/attachments/test_attachments_manager.py +++ b/cinder/tests/unit/attachments/test_attachments_manager.py @@ -17,7 +17,6 @@ from oslo_utils import importutils from cinder import context from cinder import db -from cinder.db.sqlalchemy import api as sqla_db from cinder.objects import fields from cinder.objects import volume_attachment from cinder.tests.unit.api.v2 import fakes as v2_fakes @@ -186,13 +185,12 @@ class AttachmentManagerTestCase(test.TestCase): @mock.patch('cinder.objects.VolumeAttachment.get_by_id', side_effect=[attachment1, attachment2]) - @mock.patch.object(sqla_db, 'volume_admin_metadata_delete') - @mock.patch.object(sqla_db, 'volume_detached') @mock.patch.object(self.context, 'elevated') + @mock.patch.object(self.manager, '_notify_about_volume_usage') @mock.patch.object(self.manager, '_connection_terminate') @mock.patch.object(self.manager.driver, 'remove_export') - def _test(mock_rm_export, mock_con_term, mock_elevated, - mock_db_detached, mock_db_meta_delete, mock_get_attachment): + def _test(mock_rm_export, mock_con_term, mock_notify, mock_elevated, + mock_get_attachment): mock_elevated.return_value = self.context mock_con_term.return_value = False @@ -202,8 +200,11 @@ class AttachmentManagerTestCase(test.TestCase): self.manager.attachment_delete(self.context, attachment1.id, vref) - mock_db_detached.assert_not_called() - mock_db_meta_delete.assert_not_called() + mock_elevated.assert_called_once_with() + mock_notify.assert_called_once_with(self.context, vref, + "detach.start") + mock_con_term.assert_called_once_with(self.context, vref, + attachment1) mock_rm_export.assert_called_once_with(self.context, vref) # test more than 1 attachment. This should skip @@ -211,15 +212,20 @@ class AttachmentManagerTestCase(test.TestCase): mock_con_term.return_value = True vref.volume_attachment.objects.append(attachment2) + mock_elevated.reset_mock() + mock_notify.reset_mock() + mock_con_term.reset_mock() mock_rm_export.reset_mock() - mock_db_detached.reset_mock() - mock_db_meta_delete.reset_mock() self.manager.attachment_delete(self.context, attachment2.id, vref) + mock_elevated.assert_not_called() + mock_notify.assert_called_once_with(self.context, vref, + "detach.start") + mock_con_term.assert_called_once_with(self.context, vref, + attachment2) mock_rm_export.assert_not_called() - mock_db_detached.assert_not_called() - mock_db_meta_delete.assert_not_called() + _test() def test_connection_terminate_no_connector_force_false(self): diff --git a/cinder/tests/unit/backup/drivers/test_backup_glusterfs.py b/cinder/tests/unit/backup/drivers/test_backup_glusterfs.py index dca01aa98..d002a4b73 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_glusterfs.py +++ b/cinder/tests/unit/backup/drivers/test_backup_glusterfs.py @@ -83,7 +83,7 @@ class BackupGlusterfsShareTestCase(test.TestCase): path = driver._init_backup_repo_path() self.assertEqual(FAKE_BACKUP_PATH, path) - utils.get_root_helper.called_once() + utils.get_root_helper.assert_called_once_with() mock_remotefsclient.mount.assert_called_once_with(FAKE_BACKUP_SHARE) mock_remotefsclient.get_mount_point.assert_called_once_with( FAKE_BACKUP_SHARE) diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 4e0c7b91d..a85cd905d 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -916,8 +916,8 @@ class BackupTestCase(BaseBackupTest): mock_open.assert_not_called() backup_service.backup.assert_called_once_with( backup, device_path) - mock_finish.called_once_with(self.ctxt, backup, volume, - mock.sentinel.backup_update) + mock_finish.assert_called_once_with(self.ctxt, backup, volume, + mock.sentinel.backup_update) @mock.patch('cinder.backup.manager.BackupManager._start_backup') @ddt.data((fields.SnapshotStatus.BACKING_UP, 'available'), diff --git a/cinder/tests/unit/test_coordination.py b/cinder/tests/unit/test_coordination.py index e291c84fd..db725c498 100644 --- a/cinder/tests/unit/test_coordination.py +++ b/cinder/tests/unit/test_coordination.py @@ -139,7 +139,7 @@ class CoordinatorTestCase(test.TestCase): mock_glob.assert_called_once_with( '/data/cinder-attachment_update-UUID-*') self.assertEqual(2, mock_remove.call_count) - mock_remove.has_calls( + mock_remove.assert_has_calls( [mock.call('/data/cinder-attachment_update-UUID-1'), mock.call('/data/cinder-attachment_update-UUID-2')]) @@ -157,7 +157,7 @@ class CoordinatorTestCase(test.TestCase): mock_glob.assert_called_once_with( '/data/cinder-attachment_update-UUID-*') self.assertEqual(2, mock_remove.call_count) - mock_remove.has_calls( + mock_remove.assert_has_calls( [mock.call('/data/cinder-attachment_update-UUID-1'), mock.call('/data/cinder-attachment_update-UUID-2')]) mock_log.assert_not_called() @@ -177,7 +177,7 @@ class CoordinatorTestCase(test.TestCase): mock_glob.assert_called_once_with( '/data/cinder-attachment_update-UUID-*') self.assertEqual(2, mock_remove.call_count) - mock_remove.has_calls( + mock_remove.assert_has_calls( [mock.call('/data/cinder-attachment_update-UUID-1'), mock.call('/data/cinder-attachment_update-UUID-2')]) self.assertEqual(1, mock_log.call_count) diff --git a/cinder/tests/unit/test_ssh_utils.py b/cinder/tests/unit/test_ssh_utils.py index 3d4dd6225..67091179f 100644 --- a/cinder/tests/unit/test_ssh_utils.py +++ b/cinder/tests/unit/test_ssh_utils.py @@ -405,3 +405,11 @@ class SSHPoolTestCase(test.TestCase): sshpool = None self.assertEqual(fake_close.mock_calls, close_expect_calls + close_expect_calls) + + @mock.patch('cinder.ssh_utils.paramiko', new=None) + def test_missing_paramiko(self): + self.assertRaises(exception.RequirementMissing, + ssh_utils.SSHPool, + '192.0.2.1', 22, 10, + 'test', + password='hello') diff --git a/cinder/tests/unit/volume/drivers/ceph/fake_rbd_iscsi_client.py b/cinder/tests/unit/volume/drivers/ceph/fake_rbd_iscsi_client.py deleted file mode 100644 index 3f6555004..000000000 --- a/cinder/tests/unit/volume/drivers/ceph/fake_rbd_iscsi_client.py +++ /dev/null @@ -1,25 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# -"""Fake rbd-iscsi-client for testing without installing the client.""" - -import sys -from unittest import mock - -from cinder.tests.unit.volume.drivers.ceph \ - import fake_rbd_iscsi_client_exceptions as clientexceptions - -rbdclient = mock.MagicMock() -rbdclient.version = "0.1.5" -rbdclient.exceptions = clientexceptions - -sys.modules['rbd_iscsi_client'] = rbdclient diff --git a/cinder/tests/unit/volume/drivers/ceph/fake_rbd_iscsi_client_exceptions.py b/cinder/tests/unit/volume/drivers/ceph/fake_rbd_iscsi_client_exceptions.py deleted file mode 100644 index 7a70ce755..000000000 --- a/cinder/tests/unit/volume/drivers/ceph/fake_rbd_iscsi_client_exceptions.py +++ /dev/null @@ -1,116 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# -"""Fake client exceptions to use.""" - - -class UnsupportedVersion(Exception): - """Unsupported version of the client.""" - pass - - -class ClientException(Exception): - """The base exception class for these fake exceptions.""" - _error_code = None - _error_desc = None - _error_ref = None - - _debug1 = None - _debug2 = None - - def __init__(self, error=None): - if error: - if 'code' in error: - self._error_code = error['code'] - if 'desc' in error: - self._error_desc = error['desc'] - if 'ref' in error: - self._error_ref = error['ref'] - - if 'debug1' in error: - self._debug1 = error['debug1'] - if 'debug2' in error: - self._debug2 = error['debug2'] - - def get_code(self): - return self._error_code - - def get_description(self): - return self._error_desc - - def get_ref(self): - return self._error_ref - - def __str__(self): - formatted_string = self.message - if self.http_status: - formatted_string += " (HTTP %s)" % self.http_status - if self._error_code: - formatted_string += " %s" % self._error_code - if self._error_desc: - formatted_string += " - %s" % self._error_desc - if self._error_ref: - formatted_string += " - %s" % self._error_ref - - if self._debug1: - formatted_string += " (1: '%s')" % self._debug1 - - if self._debug2: - formatted_string += " (2: '%s')" % self._debug2 - - return formatted_string - - -class HTTPConflict(ClientException): - http_status = 409 - message = "Conflict" - - def __init__(self, error=None): - if error: - super(HTTPConflict, self).__init__(error) - if 'message' in error: - self._error_desc = error['message'] - - def get_description(self): - return self._error_desc - - -class HTTPNotFound(ClientException): - http_status = 404 - message = "Not found" - - -class HTTPForbidden(ClientException): - http_status = 403 - message = "Forbidden" - - -class HTTPBadRequest(ClientException): - http_status = 400 - message = "Bad request" - - -class HTTPUnauthorized(ClientException): - http_status = 401 - message = "Unauthorized" - - -class HTTPServerError(ClientException): - http_status = 500 - message = "Error" - - def __init__(self, error=None): - if error and 'message' in error: - self._error_desc = error['message'] - - def get_description(self): - return self._error_desc diff --git a/cinder/tests/unit/volume/drivers/ceph/test_rbd_iscsi.py b/cinder/tests/unit/volume/drivers/ceph/test_rbd_iscsi.py index d593345b6..dab0b2996 100644 --- a/cinder/tests/unit/volume/drivers/ceph/test_rbd_iscsi.py +++ b/cinder/tests/unit/volume/drivers/ceph/test_rbd_iscsi.py @@ -23,8 +23,6 @@ from cinder import exception from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import fake_volume from cinder.tests.unit import test -from cinder.tests.unit.volume.drivers.ceph \ - import fake_rbd_iscsi_client as fake_client import cinder.volume.drivers.ceph.rbd_iscsi as driver # This is used to collect raised exceptions so that tests may check what was @@ -43,12 +41,6 @@ class RBDISCSITestCase(test.TestCase): self.context = context.get_admin_context() - # bogus access to prevent pep8 violation - # from the import of fake_client. - # fake_client must be imported to create the fake - # rbd_iscsi_client system module - fake_client.rbdclient - self.fake_target_iqn = 'iqn.2019-01.com.suse.iscsi-gw:iscsi-igw' self.fake_valid_response = {'status': '200'} @@ -96,30 +88,19 @@ class RBDISCSITestCase(test.TestCase): config.rbd_iscsi_api_url = 'http://fake.com:5000' return config - @mock.patch( - 'rbd_iscsi_client.client.RBDISCSIClient', - spec=True, - ) - def setup_mock_client(self, _m_client, config=None, mock_conf=None): - _m_client = _m_client.return_value - - # Configure the base constants, defaults etc... - if mock_conf: - _m_client.configure_mock(**mock_conf) - - if config is None: - config = self.setup_configuration() - - self.driver = driver.RBDISCSIDriver(configuration=config) - self.driver.set_initialized() - return _m_client - - @mock.patch('rbd_iscsi_client.version', '0.1.0') - def test_unsupported_client_version(self): - self.setup_mock_client() - with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'): - self.assertRaises(exception.InvalidInput, - self.driver.do_setup, None) + @mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup', + new=mock.MagicMock()) + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.rbd_iscsi_client') + def test_unsupported_client_version(self, m_rbd_iscsi_client, m_client): + m_rbd_iscsi_client.version = '0.1.0' + m_client.version = '0.1.0' + drv = driver.RBDISCSIDriver(configuration=self.setup_configuration()) + drv.set_initialized() + self.assertRaisesRegex(exception.InvalidInput, + 'version', + drv.do_setup, + None) @ddt.data({'user': None, 'password': 'foo', 'url': 'http://fake.com:5000', 'iqn': None}, @@ -133,40 +114,55 @@ class RBDISCSITestCase(test.TestCase): 'url': 'fake', 'iqn': None}, ) @ddt.unpack - def test_min_config(self, user, password, url, iqn): + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.rbd_iscsi_client') + def test_min_config(self, m_rbd_iscsi_client, m_client, + user, password, url, iqn): config = self.setup_configuration() config.rbd_iscsi_api_user = user config.rbd_iscsi_api_password = password config.rbd_iscsi_api_url = url config.rbd_iscsi_target_iqn = iqn - self.setup_mock_client(config=config) + + drv = driver.RBDISCSIDriver(configuration=config) + drv.set_initialized() with mock.patch('cinder.volume.drivers.rbd.RBDDriver' '.check_for_setup_error'): self.assertRaises(exception.InvalidConfigurationValue, - self.driver.check_for_setup_error) + drv.check_for_setup_error) @ddt.data({'response': None}, {'response': {'nothing': 'nothing'}}, {'response': {'status': '300'}}) @ddt.unpack - def test_do_setup(self, response): - mock_conf = { - 'get_api.return_value': (response, None)} - mock_client = self.setup_mock_client(mock_conf=mock_conf) - - with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'), \ - mock.patch.object(driver.RBDISCSIDriver, - '_create_client') as mock_create_client: - mock_create_client.return_value = mock_client + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.RBDISCSIDriver.' + '_create_client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.rbd_iscsi_client') + def test_do_setup(self, m_rbd_iscsi_client, m_client, m_create_client, + response): + m_create_client.return_value.get_api.return_value = (response, None) + m_client.version = '3.0.0' + m_rbd_iscsi_client.version = '3.0.0' + + drv = driver.RBDISCSIDriver(configuration=self.setup_configuration()) + drv.set_initialized() + + with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'): self.assertRaises(exception.InvalidConfigurationValue, - self.driver.do_setup, None) + drv.do_setup, None) - @mock.patch('rbd_iscsi_client.version', "0.1.4") - def test_unsupported_version(self): - self.setup_mock_client() - self.assertRaises(exception.InvalidInput, - self.driver._create_client) + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.rbd_iscsi_client') + def test_unsupported_version(self, m_rbd_iscsi_client, m_client): + m_rbd_iscsi_client.version = '0.1.4' + drv = driver.RBDISCSIDriver(configuration=self.setup_configuration()) + drv.set_initialized() + + self.assertRaisesRegex(exception.InvalidInput, + 'Invalid rbd_iscsi_client version found', + drv._create_client) @ddt.data({'status': '200', 'target_iqn': 'iqn.2019-01.com.suse.iscsi-gw:iscsi-igw', @@ -176,35 +172,40 @@ class RBDISCSITestCase(test.TestCase): 'clients': None} ) @ddt.unpack - def test__get_clients(self, status, target_iqn, clients): + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.RBDISCSIDriver.' + '_create_client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.rbd_iscsi_client') + def test__get_clients(self, m_rbd_iscsi_client, m_client, m_create_client, + status, target_iqn, clients): + m_create_client.return_value.get_api.return_value = ( + self.fake_valid_response, None) config = self.setup_configuration() config.rbd_iscsi_target_iqn = target_iqn + drv = driver.RBDISCSIDriver(configuration=config) + drv.set_initialized() + response = self.fake_clients['response'] response['status'] = status response['content-location'] = ( response['content-location'].replace('XX_REPLACE_ME', target_iqn)) body = self.fake_clients['body'] - mock_conf = { - 'get_clients.return_value': (response, body), - 'get_api.return_value': (self.fake_valid_response, None) - } - mock_client = self.setup_mock_client(mock_conf=mock_conf, - config=config) - - with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'), \ - mock.patch.object(driver.RBDISCSIDriver, - '_create_client') as mock_create_client: - mock_create_client.return_value = mock_client - self.driver.do_setup(None) + + m_create_client.return_value.get_clients.return_value = (response, + body) + + with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'): + drv.do_setup(None) if status == '200': - actual_response = self.driver._get_clients() + actual_response = drv._get_clients() self.assertEqual(actual_response, body) else: # we expect an exception - self.assertRaises(exception.VolumeBackendAPIException, - self.driver._get_clients) + self.assertRaisesRegex(exception.VolumeBackendAPIException, + 'Failed to get_clients()', + drv._get_clients) @ddt.data({'status': '200', 'body': {'created': 'someday', @@ -215,32 +216,35 @@ class RBDISCSITestCase(test.TestCase): {'status': '300', 'body': None}) @ddt.unpack - def test__get_config(self, status, body): + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.RBDISCSIDriver.' + '_create_client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.client') + @mock.patch('cinder.volume.drivers.ceph.rbd_iscsi.rbd_iscsi_client') + def test__get_config(self, m_rbd_iscsi_client, m_client, m_create_client, + status, body): + m_create_client.return_value.get_api.return_value = ( + self.fake_valid_response, None) config = self.setup_configuration() config.rbd_iscsi_target_iqn = self.fake_target_iqn + drv = driver.RBDISCSIDriver(configuration=config) + drv.set_initialized() + response = self.fake_clients['response'] response['status'] = status response['content-location'] = ( response['content-location'].replace('XX_REPLACE_ME', self.fake_target_iqn)) - mock_conf = { - 'get_config.return_value': (response, body), - 'get_api.return_value': (self.fake_valid_response, None) - } - mock_client = self.setup_mock_client(mock_conf=mock_conf, - config=config) - - with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'), \ - mock.patch.object(driver.RBDISCSIDriver, - '_create_client') as mock_create_client: - mock_create_client.return_value = mock_client - self.driver.do_setup(None) + m_create_client.return_value.get_config.return_value = (response, body) + + with mock.patch('cinder.volume.drivers.rbd.RBDDriver.do_setup'): + drv.do_setup(None) if status == '200': - actual_response = self.driver._get_config() + actual_response = drv._get_config() self.assertEqual(body, actual_response) else: # we expect an exception - self.assertRaises(exception.VolumeBackendAPIException, - self.driver._get_config) + self.assertRaisesRegex(exception.VolumeBackendAPIException, + 'Failed to get_config()', + drv._get_config) diff --git a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py index 1d1e440cb..018d93694 100644 --- a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py +++ b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py @@ -675,6 +675,11 @@ class HPE3PARBaseDriver(test.TestCase): 'minor': 5, 'revision': 0} + wsapi_version_2023 = {'major': 1, + 'build': 100000050, + 'minor': 10, + 'revision': 0} + # Use this to point to latest version of wsapi wsapi_version_latest = wsapi_version_for_compression @@ -892,28 +897,41 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): mock_client.assert_has_calls(expected) self.assertEqual(self.STATUS_DONE, status) - def test_create_volume(self): + # (i) wsapi version is old/default + # (ii) wsapi version is 2023, then snapCPG isn't required + @ddt.data({'wsapi_version': None}, + {'wsapi_version': HPE3PARBaseDriver.wsapi_version_2023}) + @ddt.unpack + def test_create_volume(self, wsapi_version): # setup_mock_client drive with default configuration # and return the mock HTTP 3PAR client - mock_client = self.setup_driver() + mock_client = self.setup_driver(wsapi_version=wsapi_version) + with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client - self.driver.create_volume(self.volume) + if not wsapi_version: + # (i) old/default + self.driver.create_volume(self.volume) + else: + # (ii) wsapi 2023 + common = self.driver._login() + common.create_volume(self.volume) comment = Comment({ "display_name": "Foo Volume", "type": "OpenStack", "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7", "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7"}) + optional = {'comment': comment, + 'tpvv': True, + 'tdvv': False} + if not wsapi_version: + optional['snapCPG'] = HPE3PAR_CPG_SNAP expected = [ mock.call.createVolume( self.VOLUME_3PAR_NAME, HPE3PAR_CPG, - 2048, { - 'comment': comment, - 'tpvv': True, - 'tdvv': False, - 'snapCPG': HPE3PAR_CPG_SNAP})] + 2048, optional)] mock_client.assert_has_calls(expected) @@ -1255,6 +1273,89 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): mock_client.assert_has_calls(expected) @mock.patch.object(volume_types, 'get_volume_type') + def test_create_volume_replicated_periodic_2023(self, _mock_volume_types): + # setup_mock_client drive with default configuration + # and return the mock HTTP 3PAR client + conf = self.setup_configuration() + self.replication_targets[0]['replication_mode'] = 'periodic' + conf.replication_device = self.replication_targets + mock_client = self.setup_driver(conf, None, self.wsapi_version_2023) + mock_client.getStorageSystemInfo.return_value = ( + {'id': self.CLIENT_ID}) + mock_client.getRemoteCopyGroup.side_effect = ( + hpeexceptions.HTTPNotFound) + mock_client.getCPG.return_value = {'domain': None} + mock_replicated_client = self.setup_driver(conf, None, + self.wsapi_version_2023) + mock_replicated_client.getStorageSystemInfo.return_value = ( + {'id': self.REPLICATION_CLIENT_ID}) + + _mock_volume_types.return_value = { + 'name': 'replicated', + 'extra_specs': { + 'replication_enabled': '<is> True', + 'replication:mode': 'periodic', + 'replication:sync_period': '900', + 'volume_type': self.volume_type_replicated}} + + with mock.patch.object( + hpecommon.HPE3PARCommon, + '_create_client') as mock_create_client, \ + mock.patch.object( + hpecommon.HPE3PARCommon, + '_create_replication_client') as mock_replication_client: + mock_create_client.return_value = mock_client + mock_replication_client.return_value = mock_replicated_client + + common = self.driver._login() + return_model = common.create_volume(self.volume_replicated) + comment = Comment({ + "volume_type_name": "replicated", + "display_name": "Foo Volume", + "name": "volume-d03338a9-9115-48a3-8dfc-35cdfcdc15a7", + "volume_type_id": "be9181f1-4040-46f2-8298-e7532f2bf9db", + "volume_id": "d03338a9-9115-48a3-8dfc-35cdfcdc15a7", + "qos": {}, + "type": "OpenStack"}) + + backend_id = self.replication_targets[0]['backend_id'] + expected = [ + mock.call.createVolume( + self.VOLUME_3PAR_NAME, + HPE3PAR_CPG, + 2048, { + 'comment': comment, + 'tpvv': True, + 'tdvv': False}), + mock.call.getRemoteCopyGroup(self.RCG_3PAR_NAME), + mock.call.getCPG(HPE3PAR_CPG), + mock.call.createRemoteCopyGroup( + self.RCG_3PAR_NAME, + [{'userCPG': HPE3PAR_CPG_REMOTE, + 'targetName': backend_id, + 'mode': PERIODIC_MODE}], + {'localUserCPG': HPE3PAR_CPG}), + mock.call.addVolumeToRemoteCopyGroup( + self.RCG_3PAR_NAME, + self.VOLUME_3PAR_NAME, + [{'secVolumeName': self.VOLUME_3PAR_NAME, + 'targetName': backend_id}], + optional={'volumeAutoCreation': True}), + mock.call.modifyRemoteCopyGroup( + self.RCG_3PAR_NAME, + {'targets': [{'syncPeriod': SYNC_PERIOD, + 'targetName': backend_id}]}), + mock.call.startRemoteCopy(self.RCG_3PAR_NAME)] + mock_client.assert_has_calls( + self.get_id_login + + self.standard_logout + + self.standard_login + + expected) + self.assertEqual({'replication_status': 'enabled', + 'provider_location': self.CLIENT_ID}, + return_model) + + @mock.patch.object(volume_types, 'get_volume_type') def test_create_volume_replicated_sync(self, _mock_volume_types): # setup_mock_client drive with default configuration # and return the mock HTTP 3PAR client @@ -3398,6 +3499,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client @@ -3426,6 +3528,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3449,6 +3552,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] _mock_volume_types.return_value = { 'name': 'gold', 'extra_specs': { @@ -3490,6 +3594,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'comment': comment, 'readOnly': False}), mock.call.getCPG(HPE3PAR_CPG), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3606,6 +3711,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'getVolume.return_value': {} } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] volume_type_hos = copy.deepcopy(self.volume_type_hos) volume_type_hos['extra_specs']['convert_to_base'] = True _mock_volume_types.return_value = volume_type_hos @@ -3635,6 +3741,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3656,6 +3763,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'getVolume.return_value': {} } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] _mock_volume_types.return_value = self.volume_type_hos with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: @@ -3684,6 +3792,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3706,6 +3815,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'getVolume.return_value': {} } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] volume_type_hos = copy.deepcopy(self.volume_type_hos) volume_type_hos['extra_specs']['convert_to_base'] = True _mock_volume_types.return_value = volume_type_hos @@ -3736,6 +3846,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3834,6 +3945,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client @@ -3857,6 +3969,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client @@ -3868,6 +3981,18 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): self.volume, str(new_size)) + def test__convert_to_base_volume_failure(self): + mock_client = self.setup_driver() + mock_client.getVolumeSnapshots.return_value = ( + ['oss-nwJVbXaEQMi0w.xPutFRQw']) + with mock.patch.object(hpecommon.HPE3PARCommon, + '_create_client') as mock_create_client: + mock_create_client.return_value = mock_client + common = self.driver._login() + self.assertRaises(exception.VolumeIsBusy, + common._convert_to_base_volume, + self.volume) + @mock.patch.object(volume_types, 'get_volume_type') def test_extend_volume_replicated(self, _mock_volume_types): # Managed vs. unmanaged and periodic vs. sync are not relevant when @@ -4245,10 +4370,16 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): expected_retype_specs) self.assertEqual(expected_obj, obj) + # (i) wsapi version is old/default + # (ii) wsapi version is 2023, then snapCPG isn't required + @ddt.data({'wsapi_version': None}, + {'wsapi_version': HPE3PARBaseDriver.wsapi_version_2023}) + @ddt.unpack @mock.patch.object(volume_types, 'get_volume_type') - def test_manage_existing_with_no_snap_cpg(self, _mock_volume_types): + def test_manage_existing_with_no_snap_cpg(self, _mock_volume_types, + wsapi_version): _mock_volume_types.return_value = self.volume_type - mock_client = self.setup_driver() + mock_client = self.setup_driver(wsapi_version=wsapi_version) new_comment = Comment({ "display_name": "Foo Volume", @@ -4280,15 +4411,20 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): obj = self.driver.manage_existing(volume, existing_ref) + optional = {'newName': osv_matcher, + 'comment': new_comment} + + if not wsapi_version: + # (i) old/default + # manage_existing() should be setting + # blank snapCPG to the userCPG + optional['snapCPG'] = 'testUserCpg0' + expected_manage = [ mock.call.getVolume(existing_ref['source-name']), mock.call.modifyVolume( existing_ref['source-name'], - {'newName': osv_matcher, - 'comment': new_comment, - # manage_existing() should be setting - # blank snapCPG to the userCPG - 'snapCPG': 'testUserCpg0'}) + optional) ] mock_client.assert_has_calls(self.standard_login + expected_manage) @@ -6052,16 +6188,21 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): mock_client.assert_has_calls(expected) + # (i) wsapi version is old/default + # (ii) wsapi version is 2023, then snapCPG isn't required + @ddt.data({'wsapi_version': None}, + {'wsapi_version': HPE3PARBaseDriver.wsapi_version_2023}) + @ddt.unpack @mock.patch('cinder.volume.drivers.hpe.hpe_3par_common.HPE3PARCommon.' 'get_volume_settings_from_type') @mock.patch('cinder.volume.drivers.hpe.hpe_3par_common.HPE3PARCommon.' 'is_volume_group_snap_type') @mock.patch('cinder.volume.volume_utils.is_group_a_cg_snapshot_type') def test_create_group_from_src_group(self, cg_ss_enable, vol_ss_enable, - typ_info): + typ_info, wsapi_version): cg_ss_enable.return_value = True vol_ss_enable.return_value = True - mock_client = self.setup_driver() + mock_client = self.setup_driver(wsapi_version=wsapi_version) task_id = 1 mock_client.copyVolume.return_value = {'taskid': task_id} mock_client.getStorageSystemInfo.return_value = {'id': self.CLIENT_ID} @@ -6092,6 +6233,10 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): source_grp = self.fake_group_object( grp_id=self.SRC_CONSIS_GROUP_ID) + optional = {'online': True, + 'tpvv': mock.ANY, 'tdvv': mock.ANY} + if not wsapi_version: + optional['snapCPG'] = HPE3PAR_CPG expected = [ mock.call.getCPG(HPE3PAR_CPG), mock.call.createVolumeSet( @@ -6107,17 +6252,25 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): mock.ANY, self.VOLUME_NAME_3PAR, HPE3PAR_CPG, - {'snapCPG': HPE3PAR_CPG, 'online': True, - 'tpvv': mock.ANY, 'tdvv': mock.ANY}), + optional), mock.call.addVolumeToVolumeSet( self.CONSIS_GROUP_NAME, self.VOLUME_NAME_3PAR)] # Create a consistency group from a source consistency group. - self.driver.create_group_from_src( - context.get_admin_context(), group, - [volume], source_group=source_grp, - source_vols=[source_volume]) + if not wsapi_version: + # (i) old/default + self.driver.create_group_from_src( + context.get_admin_context(), group, + [volume], source_group=source_grp, + source_vols=[source_volume]) + else: + # (ii) wsapi 2023 + common = self.driver._login() + common.create_group_from_src( + context.get_admin_context(), group, + [volume], source_group=source_grp, + source_vols=[source_volume]) mock_client.assert_has_calls(expected) diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_data_motion.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_data_motion.py index 910005b90..0ba8a2fe9 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_data_motion.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/utils/test_data_motion.py @@ -1128,7 +1128,7 @@ class NetAppCDOTDataMotionMixinTestCase(test.TestCase): utils.get_backend_configuration.assert_called_once_with( dataontap_fakes.DEST_BACKEND_NAME) - utils.get_client_for_backend.has_calls( + utils.get_client_for_backend.assert_has_calls( [mock.call(dataontap_fakes.DEST_BACKEND_NAME), mock.call(dataontap_fakes.BACKEND_NAME)]) self.mock_src_client.get_cluster_name.assert_called() @@ -1190,7 +1190,7 @@ class NetAppCDOTDataMotionMixinTestCase(test.TestCase): utils.get_backend_configuration.assert_called_once_with( dataontap_fakes.DEST_BACKEND_NAME) - utils.get_client_for_backend.has_calls( + utils.get_client_for_backend.assert_has_calls( [mock.call(dataontap_fakes.DEST_BACKEND_NAME), mock.call(dataontap_fakes.BACKEND_NAME)]) self.mock_src_client.get_cluster_name.assert_called() @@ -1240,7 +1240,7 @@ class NetAppCDOTDataMotionMixinTestCase(test.TestCase): else: utils.get_backend_configuration.assert_called_once_with( dest_backend_name) - utils.get_client_for_backend.has_calls( + utils.get_client_for_backend.assert_has_calls( [mock.call(dest_backend_name), mock.call(dataontap_fakes.BACKEND_NAME)]) self.mock_src_client.get_cluster_name.assert_called() @@ -1281,7 +1281,7 @@ class NetAppCDOTDataMotionMixinTestCase(test.TestCase): utils.get_backend_configuration.assert_called_once_with( dataontap_fakes.DEST_BACKEND_NAME) - utils.get_client_for_backend.has_calls( + utils.get_client_for_backend.assert_has_calls( [mock.call(dataontap_fakes.DEST_BACKEND_NAME), mock.call(dataontap_fakes.BACKEND_NAME)]) self.mock_src_client.get_cluster_name.assert_called() diff --git a/cinder/tests/unit/volume/drivers/test_pure.py b/cinder/tests/unit/volume/drivers/test_pure.py index ff329a478..de59abc13 100644 --- a/cinder/tests/unit/volume/drivers/test_pure.py +++ b/cinder/tests/unit/volume/drivers/test_pure.py @@ -194,14 +194,12 @@ FC_PORTS = [{"name": name, "nqn": None, "portal": None, "wwn": wwn, - "nqn": None, } for name, wwn in zip(FC_PORT_NAMES, FC_WWNS)] AC_FC_PORTS = [{"name": name, "iqn": None, "nqn": None, "portal": None, "wwn": wwn, - "nqn": None, } for name, wwn in zip(FC_PORT_NAMES, AC_FC_WWNS)] NON_ISCSI_PORT = { "name": "ct0.fc1", @@ -4934,14 +4932,54 @@ class PureNVMEDriverTestCase(PureBaseSharedDriverTestCase): self.driver.initialize_connection(vol, multipath_connector) def test_get_target_nvme_ports(self): - self.array.list_ports.return_value = NVME_PORTS + ports = [{'name': 'CT0.ETH4', + 'wwn': None, + 'iqn': None, + 'nqn': TARGET_NQN}, + {'name': 'CT0.ETH5', + 'wwn': None, + 'iqn': TARGET_IQN, + 'nqn': None}, + {'name': 'CT0.ETH20', + 'wwn': None, + 'iqn': None, + 'nqn': TARGET_NQN}, + {'name': 'CT0.FC4', + 'wwn': TARGET_WWN, + 'iqn': None, + 'nqn': TARGET_NQN}] + interfaces = [ + {'name': 'ct0.eth4', 'services': ['nvme-tcp']}, + {'name': 'ct0.eth5', 'services': ['iscsi']}, + {'name': 'ct0.eth20', 'services': ['nvme-roce']}, + {'name': 'ct0.fc4', 'services': ['nvme-fc']} + ] + # Test for the nvme-tcp port + self.driver.configuration.pure_nvme_transport = "tcp" + self.array.get_network_interface.return_value = interfaces[0] + self.array.list_ports.return_value = [ports[0]] ret = self.driver._get_target_nvme_ports(self.array) - self.assertEqual(NVME_PORTS, ret) - - def test_get_target_nvme_ports_with_nvme_and_fc(self): - self.array.list_ports.return_value = NVME_PORTS_WITH + self.assertEqual([ports[0]], ret) + # Test for failure if no NVMe ports + self.array.get_network_interface.return_value = interfaces[1] + self.array.list_ports.return_value = [ports[1]] + self.assertRaises( + pure.PureDriverException, + self.driver._get_target_nvme_ports, + self.array, + ) + # Test for the nvme-roce port + self.driver.configuration.pure_nvme_transport = "roce" + self.array.get_network_interface.return_value = interfaces[2] + self.array.list_ports.return_value = [ports[2]] + ret = self.driver._get_target_nvme_ports(self.array) + self.assertEqual([ports[2]], ret) + # Test for empty dict if only nvme-fc port + self.driver.configuration.pure_nvme_transport = "roce" + self.array.get_network_interface.return_value = interfaces[3] + self.array.list_ports.return_value = [ports[3]] ret = self.driver._get_target_nvme_ports(self.array) - self.assertEqual(NVME_PORTS, ret) + self.assertEqual([], ret) def test_get_target_nvme_ports_with_no_ports(self): # Should raise an exception if there are no ports diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 57dae3af1..f1ffeb89e 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -3361,6 +3361,17 @@ class RBDTestCase(test.TestCase): {'provider_location': "{\"saved_features\":%s}" % image_features}, ret) + @common_mocks + def test_enable_multiattach_no_features(self): + image = self.mock_proxy.return_value.__enter__.return_value + image.features.return_value = 0 + + ret = self.driver._enable_multiattach(self.volume_a) + + image.update_features.assert_not_called() + + self.assertEqual({'provider_location': '{"saved_features":0}'}, ret) + @ddt.data(MULTIATTACH_FULL_FEATURES, MULTIATTACH_REDUCED_FEATURES) @common_mocks def test_disable_multiattach(self, features): @@ -3374,6 +3385,18 @@ class RBDTestCase(test.TestCase): self.assertEqual({'provider_location': None}, ret) + @common_mocks + def test_disable_multiattach_no_features(self): + image = self.mock_proxy.return_value.__enter__.return_value + self.volume_a.provider_location = '{"saved_features": 0}' + image.features.return_value = 0 + + ret = self.driver._disable_multiattach(self.volume_a) + + image.update_features.assert_not_called() + + self.assertEqual({'provider_location': None}, ret) + class ManagedRBDTestCase(test_driver.BaseDriverTestCase): driver_name = "cinder.volume.drivers.rbd.RBDDriver" diff --git a/cinder/tests/unit/volume/drivers/test_remotefs.py b/cinder/tests/unit/volume/drivers/test_remotefs.py index 812550868..db2971fa2 100644 --- a/cinder/tests/unit/volume/drivers/test_remotefs.py +++ b/cinder/tests/unit/volume/drivers/test_remotefs.py @@ -770,6 +770,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase): src_encryption_key_id=None, new_encryption_key_id=None) mock_delete_snapshot.assert_called_once_with( mock_obj_snap.return_value) + mock_obj_snap.return_value.destroy.assert_called_once_with() else: self.assertFalse(mock_create_snapshot.called) diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index 5b4ddb35f..d2dca5a56 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -1771,6 +1771,8 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): image_location = 'someImageLocationStr' image_id = fakes.IMAGE_ID image_meta = mock.MagicMock() + volume_id = str(uuid.uuid4()) + self.mock_cache.get_entry.return_value = {'volume_id': volume_id} volume = fake_volume.fake_volume_obj(self.ctxt, size=1, host='foo@bar#pool') self.mock_driver.clone_image.return_value = (None, False) @@ -1794,6 +1796,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): image_id, image_meta, self.mock_image_service) + mock_handle_bootable.assert_not_called() else: mock_create_from_src.side_effect = NotImplementedError( 'Driver does not support clone') @@ -1807,16 +1810,13 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): mock_create_from_img_dl.assert_called_once() self.assertEqual(mock_create_from_img_dl.return_value, model_update) + mock_handle_bootable.assert_called_once_with(self.ctxt, volume, + image_id=image_id, + image_meta=image_meta) # Ensure cloning was attempted and that it failed - mock_create_from_src.assert_called_once() - with mock.patch( - 'cinder.volume.flows.manager.create_volume.' - 'CreateVolumeFromSpecTask') as volume_manager: - (volume_manager.CreateVolumeFromSpecTask. - _create_from_image_cache_or_download.called_once()) - (volume_manager.CreateVolumeFromSpecTask. - _create_from_image_cache.called_once()) + mock_create_from_src.assert_called_once_with(self.ctxt, volume, + volume_id) @mock.patch('cinder.volume.flows.manager.create_volume.' 'CreateVolumeFromSpecTask.' diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index fa0f1da04..471b3391c 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -2910,8 +2910,8 @@ class VolumeTestCase(base.BaseVolumeTestCase): self.volume.detach_volume(self.context, volume.id, attachment.id) self.volume.delete_volume(self.context, volume) - @mock.patch('cinder.volume.rpcapi.VolumeAPI.extend_volume') - def test_extend_volume_with_volume_type(self, mock_rpc_extend): + @mock.patch('cinder.scheduler.rpcapi.SchedulerAPI.extend_volume') + def test_extend_volume_with_volume_type(self, mock_scheduler_extend): elevated = context.get_admin_context() project_id = self.context.project_id db.volume_type_create(elevated, {'name': 'type', 'extra_specs': {}}) @@ -2929,7 +2929,13 @@ class VolumeTestCase(base.BaseVolumeTestCase): db.volume_update(self.context, volume.id, {'status': 'available'}) volume_api._extend(self.context, volume, 200) - mock_rpc_extend.called_once_with(self.context, volume, 200, mock.ANY) + mock_scheduler_extend.assert_called_once_with( + self.context, volume, 200, mock.ANY, + { + 'volume_properties': volume, + 'volume_type': vol_type, + 'volume_id': volume.id + }) try: usage = db.quota_usage_get(elevated, project_id, 'gigabytes_type') diff --git a/cinder/volume/drivers/dell_emc/powerstore/utils.py b/cinder/volume/drivers/dell_emc/powerstore/utils.py index 52c74a587..dd02fe93c 100644 --- a/cinder/volume/drivers/dell_emc/powerstore/utils.py +++ b/cinder/volume/drivers/dell_emc/powerstore/utils.py @@ -15,12 +15,12 @@ """Utilities for Dell EMC PowerStore Cinder driver.""" -from distutils import version import functools import re from oslo_log import log as logging from oslo_utils import units +from packaging import version from cinder.common import constants from cinder import exception @@ -186,4 +186,4 @@ def is_group_a_cg_snapshot_type(func): def version_gte(ver1, ver2): - return version.LooseVersion(ver1) >= version.LooseVersion(ver2) + return version.parse(ver1) >= version.parse(ver2) diff --git a/cinder/volume/drivers/hpe/hpe_3par_common.py b/cinder/volume/drivers/hpe/hpe_3par_common.py index 971fac3e8..2d9534a16 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_common.py +++ b/cinder/volume/drivers/hpe/hpe_3par_common.py @@ -81,6 +81,7 @@ FLASH_CACHE_API_VERSION = 30201200 COMPRESSION_API_VERSION = 30301215 SRSTATLD_API_VERSION = 30201200 REMOTE_COPY_API_VERSION = 30202290 +API_VERSION_2023 = 100000000 hpe3par_opts = [ cfg.StrOpt('hpe3par_api_url', @@ -300,11 +301,14 @@ class HPE3PARCommon(object): 4.0.16 - In multi host env, fix multi-detach operation. Bug #1958122 4.0.17 - Added get_manageable_volumes and get_manageable_snapshots. Bug #1819903 + 4.0.18 - During conversion of volume to base volume, + error out if it has child snapshot(s). Bug #1994521 + 4.0.19 - Update code to work with new WSAPI (of 2023). Bug #2015746 """ - VERSION = "4.0.17" + VERSION = "4.0.19" stats = {} @@ -704,9 +708,12 @@ class HPE3PARCommon(object): compression = self.get_compression_policy( type_info['hpe3par_keys']) - optional = {'online': True, 'snapCPG': snapcpg, + optional = {'online': True, 'tpvv': tpvv, 'tdvv': tdvv} + if self.API_VERSION < API_VERSION_2023: + optional['snapCPG'] = snapcpg + if compression is not None: optional['compression'] = compression @@ -1004,7 +1011,7 @@ class HPE3PARCommon(object): 'comment': json.dumps(new_comment)} # Ensure that snapCPG is set - if 'snapCPG' not in vol: + if 'snapCPG' not in vol and self.API_VERSION < API_VERSION_2023: new_vals['snapCPG'] = vol['userCPG'] LOG.info("Virtual volume %(disp)s '%(new)s' snapCPG " "is empty so it will be set to: %(cpg)s", @@ -2393,9 +2400,14 @@ class HPE3PARCommon(object): comments['qos'] = qos extras = {'comment': json.dumps(comments), - 'snapCPG': snap_cpg, 'tpvv': tpvv} + LOG.debug("self.API_VERSION: %(version)s", + {'version': self.API_VERSION}) + + if self.API_VERSION < API_VERSION_2023: + extras['snapCPG'] = snap_cpg + # Only set the dedup option if the backend supports it. if self.API_VERSION >= DEDUP_API_VERSION: extras['tdvv'] = tdvv @@ -2466,7 +2478,7 @@ class HPE3PARCommon(object): {'src': src_name, 'dest': dest_name}) optional = {'tpvv': tpvv, 'online': True} - if snap_cpg is not None: + if snap_cpg is not None and self.API_VERSION < API_VERSION_2023: optional['snapCPG'] = snap_cpg if self.API_VERSION >= DEDUP_API_VERSION: @@ -3139,6 +3151,21 @@ class HPE3PARCommon(object): compression = self.get_compression_policy( type_info['hpe3par_keys']) + + # If volume (osv-) has snapshot, while converting the volume + # to base volume (omv-), snapshot cannot be transferred to + # new base volume (omv-) i.e it remain with volume (osv-). + # So error out for such volume. + snap_list = self.client.getVolumeSnapshots(volume_name) + if snap_list: + snap_str = ",".join(snap_list) + msg = (_("Volume %(name)s has dependent snapshots: %(snap)s." + " Either flatten or remove the dependent snapshots:" + " %(snap)s for the conversion of volume %(name)s to" + " succeed." % {'name': volume_name, + 'snap': snap_str})) + raise exception.VolumeIsBusy(message=msg) + # Create a physical copy of the volume task_id = self._copy_volume(volume_name, temp_vol_name, cpg, cpg, type_info['tpvv'], @@ -3162,16 +3189,18 @@ class HPE3PARCommon(object): comment = self._get_3par_vol_comment(volume_name) if comment: self.client.modifyVolume(temp_vol_name, {'comment': comment}) - LOG.debug('Volume rename completed: convert_to_base_volume: ' - 'id=%s.', volume['id']) + LOG.debug('Assigned the comment: convert_to_base_volume: ' + 'id=%s.', volume['id']) - # Delete source volume after the copy is complete + # Delete source volume (osv-) after the copy is complete self.client.deleteVolume(volume_name) LOG.debug('Delete src volume completed: convert_to_base_volume: ' 'id=%s.', volume['id']) - # Rename the new volume to the original name + # Rename the new volume (omv-) to the original name (osv-) self.client.modifyVolume(temp_vol_name, {'newName': volume_name}) + LOG.debug('Volume rename completed: convert_to_base_volume: ' + 'id=%s.', volume['id']) LOG.info('Completed: convert_to_base_volume: ' 'id=%s.', volume['id']) @@ -4358,15 +4387,17 @@ class HPE3PARCommon(object): local_cpg) rcg_target = {'targetName': target['backend_id'], 'mode': replication_mode_num, - 'snapCPG': cpg, 'userCPG': cpg} + if self.API_VERSION < API_VERSION_2023: + rcg_target['snapCPG'] = cpg rcg_targets.append(rcg_target) sync_target = {'targetName': target['backend_id'], 'syncPeriod': replication_sync_period} sync_targets.append(sync_target) - optional = {'localSnapCPG': vol_settings['snap_cpg'], - 'localUserCPG': local_cpg} + optional = {'localUserCPG': local_cpg} + if self.API_VERSION < API_VERSION_2023: + optional['localSnapCPG'] = vol_settings['snap_cpg'] pool = volume_utils.extract_host(volume['host'], level='pool') domain = self.get_domain(pool) if domain: @@ -4381,6 +4412,8 @@ class HPE3PARCommon(object): LOG.error(msg) raise exception.VolumeBackendAPIException(data=msg) + LOG.debug("created rcg %(name)s", {'name': rcg_name}) + # Add volume to remote copy group. rcg_targets = [] for target in self._replication_targets: @@ -5300,7 +5333,11 @@ class ModifyVolumeTask(flow_utils.CinderTask): comment_dict = self._get_new_comment( old_comment, new_vvs, new_qos, new_type_name, new_type_id) - if new_snap_cpg != old_snap_cpg: + LOG.debug("API_VERSION: %(ver_1)s, API_VERSION_2023: %(ver_2)s", + {'ver_1': common.API_VERSION, + 'ver_2': API_VERSION_2023}) + if (new_snap_cpg != old_snap_cpg and + common.API_VERSION < API_VERSION_2023): # Modify the snap_cpg. This will fail with snapshots. LOG.info("Modifying %(volume_name)s snap_cpg from " "%(old_snap_cpg)s to %(new_snap_cpg)s.", diff --git a/cinder/volume/drivers/ibm/ibm_storage/ds8k_connection.py b/cinder/volume/drivers/ibm/ibm_storage/ds8k_connection.py index 36f283b40..dfed6d49f 100644 --- a/cinder/volume/drivers/ibm/ibm_storage/ds8k_connection.py +++ b/cinder/volume/drivers/ibm/ibm_storage/ds8k_connection.py @@ -94,10 +94,7 @@ class DS8KHTTPSConnection(connection.VerifiedHTTPSConnection): # Calls self._set_hostport(), so self.host is # self._tunnel_host below. # - # disable pylint because pylint doesn't support importing - # from six.moves yet. see: - # https://bitbucket.org/logilab/pylint/issue/550/ - self._tunnel() # pylint: disable=E1101 + self._tunnel() # Mark this connection as not reusable self.auto_open = 0 diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index bf38d88f9..fcaecda08 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -111,9 +111,8 @@ PURE_OPTS = [ "IPv4 and IPv6 subnets. This parameter supersedes " "pure_nvme_cidr."), cfg.StrOpt("pure_nvme_transport", default="roce", - choices=['roce'], - help="The NVMe transport layer to be used by the NVMe driver. " - "This only supports RoCE at this time."), + choices=['roce', 'tcp'], + help="The NVMe transport layer to be used by the NVMe driver."), cfg.BoolOpt("pure_eradicate_on_delete", default=False, help="When enabled, all Pure volumes, snapshots, and " @@ -159,6 +158,7 @@ ERR_MSG_NOT_CONNECTED = "is not connected" ERR_MSG_ALREADY_BELONGS = "already belongs to" ERR_MSG_EXISTING_CONNECTIONS = "cannot be deleted due to existing connections" ERR_MSG_ALREADY_IN_USE = "already in use" +ERR_MSG_ARRAY_LIMIT = "limit reached" EXTRA_SPECS_REPL_ENABLED = "replication_enabled" EXTRA_SPECS_REPL_TYPE = "replication_type" @@ -406,6 +406,13 @@ class PureBaseVolumeDriver(san.SanDriver): "unsupported. Please upgrade your backend to " "a supported version.") raise PureDriverException(msg) + if version.parse(array_info["version"]) < version.parse( + '6.4.2' + ) and self._storage_protocol == constants.NVMEOF_TCP: + msg = _("FlashArray Purity version less than 6.4.2 " + "unsupported for NVMe-TCP. Please upgrade your " + "backend to a supported version.") + raise PureDriverException(msg) self._array.array_name = array_info["array_name"] self._array.array_id = array_info["id"] @@ -2418,8 +2425,9 @@ class PureBaseVolumeDriver(san.SanDriver): except purestorage.PureHTTPError as err: with excutils.save_and_reraise_exception() as ctxt: if err.code == 400 and ( - ERR_MSG_ALREADY_EXISTS - in err.text): + ERR_MSG_ALREADY_EXISTS in err.text + or ERR_MSG_ARRAY_LIMIT in err.text + ): ctxt.reraise = False LOG.info("Skipping add array %(target_array)s to pod" " %(pod_name)s since it's already added.", @@ -3217,6 +3225,9 @@ class PureNVMEDriver(PureBaseVolumeDriver, driver.BaseVD): if self.configuration.pure_nvme_transport == "roce": self.transport_type = "rdma" self._storage_protocol = constants.NVMEOF_ROCE + else: + self.transport_type = "tcp" + self._storage_protocol = constants.NVMEOF_TCP def _get_nguid(self, pure_vol_name): """Return the NGUID based on the volume's serial number @@ -3331,14 +3342,24 @@ class PureNVMEDriver(PureBaseVolumeDriver, driver.BaseVD): return props def _get_target_nvme_ports(self, array): - """Return list of nvme-enabled port descriptions.""" + """Return list of correct nvme-enabled port descriptions.""" ports = array.list_ports() + valid_nvme_ports = [] nvme_ports = [port for port in ports if port["nqn"]] + for port in range(0, len(nvme_ports)): + if "ETH" in nvme_ports[port]["name"]: + port_detail = array.get_network_interface( + interface=nvme_ports[port]["name"] + ) + if port_detail["services"][0] == "nvme-" + \ + self.configuration.pure_nvme_transport: + valid_nvme_ports.append(nvme_ports[port]) if not nvme_ports: raise PureDriverException( - reason=_("No nvme-enabled ports on target array.") + reason=_("No %(type)s enabled ports on target array.") % + {"type": self._storage_protocol} ) - return nvme_ports + return valid_nvme_ports @utils.retry(PureRetryableException, retries=HOST_CREATE_MAX_RETRIES) def _connect(self, array, vol_name, connector): diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index 6cc86c2c5..e710fd356 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -968,7 +968,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, with RBDVolumeProxy(self, vol_name) as image: image_features = image.features() change_features = self.MULTIATTACH_EXCLUSIONS & image_features - image.update_features(change_features, False) + if change_features != 0: + image.update_features(change_features, False) return {'provider_location': self._dumps({'saved_features': image_features})} @@ -980,7 +981,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, provider_location = json.loads(volume.provider_location) image_features = provider_location['saved_features'] change_features = self.MULTIATTACH_EXCLUSIONS & image_features - image.update_features(change_features, True) + if change_features != 0: + image.update_features(change_features, True) except IndexError: msg = "Could not find saved image features." raise RBDDriverException(reason=msg) |