diff options
24 files changed, 284 insertions, 264 deletions
diff --git a/.zuul.yaml b/.zuul.yaml index 52bdf40a7..e15853006 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -14,7 +14,7 @@ - cinder-mypy - cinder-tox-bandit-baseline: voting: false - - openstack-tox-functional-py38: + - openstack-tox-functional-py39: irrelevant-files: &functional-irrelevant-files - ^.*\.rst$ - ^cinder/locale/.*$ @@ -58,6 +58,9 @@ irrelevant-files: *gate-irrelevant-files - cinder-tempest-plugin-lvm-lio-barbican: irrelevant-files: *gate-irrelevant-files + - cinder-tempest-plugin-lvm-lio-barbican-fips: + voting: false + irrelevant-files: *gate-irrelevant-files - cinder-grenade-mn-sub-volbak: irrelevant-files: *gate-irrelevant-files - cinder-tempest-lvm-multibackend: @@ -68,6 +71,9 @@ irrelevant-files: *gate-irrelevant-files - devstack-plugin-nfs-tempest-full: irrelevant-files: *gate-irrelevant-files + - devstack-plugin-nfs-tempest-full-fips: + voting: false + irrelevant-files: *gate-irrelevant-files - tempest-slow-py3: irrelevant-files: *gate-irrelevant-files - tempest-integrated-storage: @@ -169,6 +175,9 @@ - "{{ ansible_user_dir }}/{{ zuul.projects['opendev.org/openstack/cinderlib'].src_dir }}" devstack_localrc: CEPH_MIN_CLIENT_VERSION: "mimic" + # NOTE: if jobs are having memory problems, may want + # to turn this on (currently defaults to false): + # MYSQL_REDUCE_MEMORY: true devstack_local_conf: test-config: $TEMPEST_CONFIG: @@ -176,6 +185,17 @@ volume_revert: True - job: + # this depends on some ceph admin setup which is not yet complete + # TODO(alee) enable this test when ceph admin work is complete. + name: cinder-plugin-ceph-tempest-fips + parent: cinder-plugin-ceph-tempest + nodeset: devstack-single-node-centos-9-stream + pre-run: playbooks/enable-fips.yaml + vars: + configure_swap_size: 4096 + nslookup_target: 'opendev.org' + +- job: name: cinder-plugin-ceph-tempest-mn-aa parent: devstack-plugin-ceph-multinode-tempest-py3 roles: @@ -191,6 +211,9 @@ - "{{ ansible_user_dir }}/{{ zuul.projects['opendev.org/openstack/cinderlib'].src_dir }}" devstack_localrc: TEMPEST_VOLUME_REVERT_TO_SNAPSHOT: True + # NOTE: if jobs are having memory problems, may want + # to turn this on (currently defaults to false): + # MYSQL_REDUCE_MEMORY: true devstack_local_conf: post-config: $CINDER_CONF: diff --git a/bindep.txt b/bindep.txt index d32d02680..6311a1885 100644 --- a/bindep.txt +++ b/bindep.txt @@ -29,6 +29,7 @@ postgresql postgresql-client [platform:dpkg] postgresql-devel [platform:rpm] postgresql-server [platform:rpm] +python3-devel [platform:rpm test] libpq-dev [platform:dpkg] thin-provisioning-tools [platform:debian] libxml2-dev [platform:dpkg test] 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/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..7916792fa 100644 --- a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py +++ b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py @@ -3398,6 +3398,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 +3427,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 +3451,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 +3493,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 +3610,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 +3640,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 +3662,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 +3691,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 +3714,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 +3745,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 +3844,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 +3868,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 +3880,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 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_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/hpe/hpe_3par_common.py b/cinder/volume/drivers/hpe/hpe_3par_common.py index 971fac3e8..94c346b37 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_common.py +++ b/cinder/volume/drivers/hpe/hpe_3par_common.py @@ -300,11 +300,13 @@ 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 """ - VERSION = "4.0.17" + VERSION = "4.0.18" stats = {} @@ -3139,6 +3141,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 +3179,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']) diff --git a/doc/source/configuration/block-storage/drivers/dell-emc-powerflex-driver.rst b/doc/source/configuration/block-storage/drivers/dell-emc-powerflex-driver.rst index 0f1a07272..adace17a8 100644 --- a/doc/source/configuration/block-storage/drivers/dell-emc-powerflex-driver.rst +++ b/doc/source/configuration/block-storage/drivers/dell-emc-powerflex-driver.rst @@ -41,6 +41,8 @@ compatible: * PowerFlex 3.6.0 +* PowerFlex 4.0.x + Please consult the :ref:`powerflex_docs` to determine supported operating systems for each version of PowerFlex or VxFlex OS. diff --git a/doc/source/contributor/high_availability.rst b/doc/source/contributor/high_availability.rst index a75d2c085..dcb46b32c 100644 --- a/doc/source/contributor/high_availability.rst +++ b/doc/source/contributor/high_availability.rst @@ -585,7 +585,7 @@ the lock, and `external=True` to make sure that file locks are being used. The name parameter provided for these locks must be a literal string value. There is no kind of templating support. -Example from `cinder/volume/targest/lio.py`: +Example from `cinder/volume/targets/lio.py`: .. code-block:: python diff --git a/playbooks/enable-fips.yaml b/playbooks/enable-fips.yaml new file mode 100644 index 000000000..bc1dc04ea --- /dev/null +++ b/playbooks/enable-fips.yaml @@ -0,0 +1,3 @@ +- hosts: all + roles: + - enable-fips diff --git a/releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml b/releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml new file mode 100644 index 000000000..e087e3353 --- /dev/null +++ b/releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + HPE 3PAR driver `Bug #1994521 <https://bugs.launchpad.net/cinder/+bug/1994521>`_: + Fixed: While performing a delete snapshot (s1) operation, the volumes (v2) + dependent on the snapshot (s1) are converted to base volumes. This + operation fails if these dependent volumes (v2) have their own dependent + snapshots (s2). The errors during the failure were vague and not helpful. + With this release, we added conditions to fail this operation early and + also added useful error message. + diff --git a/tools/test-setup.sh b/tools/test-setup.sh index 5b986ced3..fced9be5e 100755 --- a/tools/test-setup.sh +++ b/tools/test-setup.sh @@ -15,6 +15,47 @@ DB_ROOT_PW=${MYSQL_ROOT_PW:-insecure_slave} DB_USER=openstack_citest DB_PW=openstack_citest +function is_rhel7 { + [ -f /usr/bin/yum ] && \ + cat /etc/*release | grep -q -e "Red Hat" -e "CentOS" -e "CloudLinux" && \ + cat /etc/*release | grep -q 'release 7' +} + +function is_rhel8 { + [ -f /usr/bin/dnf ] && \ + cat /etc/*release | grep -q -e "Red Hat" -e "CentOS" -e "CloudLinux" && \ + cat /etc/*release | grep -q 'release 8' +} + +function is_rhel9 { + [ -f /usr/bin/dnf ] && \ + cat /etc/*release | grep -q -e "Red Hat" -e "CentOS" -e "CloudLinux" && \ + cat /etc/*release | grep -q 'release 9' +} + +function set_conf_line { # file regex value + sudo sh -c "grep -q -e '$2' $1 && \ + sed -i 's|$2|$3|g' $1 || \ + echo '$3' >> $1" +} + +if is_rhel7 || is_rhel8 || is_rhel9; then + # mysql needs to be started on centos/rhel + sudo systemctl restart mariadb.service + + # postgres setup for centos + sudo postgresql-setup --initdb + PG_CONF=/var/lib/pgsql/data/postgresql.conf + set_conf_line $PG_CONF '^password_encryption =.*' 'password_encryption = scram-sha-256' + + PG_HBA=/var/lib/pgsql/data/pg_hba.conf + set_conf_line $PG_HBA '^local[ \t]*all[ \t]*all.*' 'local all all peer' + set_conf_line $PG_HBA '^host[ \t]*all[ \t]*all[ \t]*127.0.0.1\/32.*' 'host all all 127.0.0.1/32 scram-sha-256' + set_conf_line $PG_HBA '^host[ \t]*all[ \t]*all[ \t]*::1\/128.*' 'host all all ::1/128 scram-sha-256' + + sudo systemctl restart postgresql.service +fi + sudo -H mysqladmin -u root password $DB_ROOT_PW # It's best practice to remove anonymous users from the database. If |