From 3057445fc968afec5fafd632e523d9a0caaef2c7 Mon Sep 17 00:00:00 2001 From: Brian Rosmaita Date: Wed, 17 Apr 2024 22:45:52 -0400 Subject: [PATCH] Remove psutil requirement Follow up on a TODO in image_utils to replace use of psutil with the standard library shutil, which makes it no longer a requirement for the main cinder code. The only driver using psutil is Quobyte, so make it a driver extra requirement, and make some minor adjustments to the quobyte driver and tests so that the driver will check for the availability of psutil and the unit tests can operate without it being present. Change-Id: Icd1a4e59317fe1db0c5419b9aaeb85464b7be863 --- cinder/image/image_utils.py | 6 +- .../tests/unit/volume/drivers/test_quobyte.py | 56 +++++++++++++------ cinder/volume/drivers/quobyte.py | 9 ++- driver-requirements.txt | 3 + ...obyte-extra-requires-8dc1761859da923a.yaml | 9 +++ requirements.txt | 1 - setup.cfg | 3 + 7 files changed, 63 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/quobyte-extra-requires-8dc1761859da923a.yaml diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 84e478f7ce2..4a4bfe3fdb2 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -31,6 +31,7 @@ import io import math import os import re +import shutil import tempfile from typing import ContextManager, Generator, Optional @@ -45,7 +46,6 @@ from oslo_utils import fileutils from oslo_utils import imageutils from oslo_utils import timeutils from oslo_utils import units -import psutil from cinder import context from cinder import exception @@ -1099,12 +1099,10 @@ def check_virtual_size(virtual_size: float, def check_available_space(dest: str, image_size: int, image_id: str) -> None: - # TODO(e0ne): replace psutil with shutil.disk_usage when we drop - # Python 2.7 support. if not os.path.isdir(dest): dest = os.path.dirname(dest) - free_space = psutil.disk_usage(dest).free + free_space = shutil.disk_usage(dest).free if free_space <= image_size: msg = ('There is no space on %(dest_dir)s to convert image. ' 'Requested: %(image_size)s, available: %(free_space)s.' diff --git a/cinder/tests/unit/volume/drivers/test_quobyte.py b/cinder/tests/unit/volume/drivers/test_quobyte.py index c66ba2d8c5b..1dc350d6529 100644 --- a/cinder/tests/unit/volume/drivers/test_quobyte.py +++ b/cinder/tests/unit/volume/drivers/test_quobyte.py @@ -27,7 +27,6 @@ from oslo_concurrency import processutils as putils from oslo_utils import fileutils from oslo_utils import imageutils from oslo_utils import units -import psutil from cinder import context from cinder import db @@ -439,14 +438,15 @@ class QuobyteDriverTestCase(test.TestCase): any_order=False) mock_validate.assert_called_once_with(self.TEST_MNT_POINT) - @mock.patch.object(psutil, "disk_partitions") + @mock.patch.object(quobyte, 'psutil') def test_mount_quobyte_should_reraise_already_mounted_error(self, - part_mock): + ps_mock): """test_mount_quobyte_should_reraise_already_mounted_error Like test_mount_quobyte_should_suppress_already_mounted_error but with ensure=False. """ + part_mock = ps_mock.disk_partitions part_mock.return_value = [] # no quobyte@ devices with mock.patch.object(self._driver, '_execute') as mock_execute, \ mock.patch('oslo_utils.fileutils.ensure_tree') as mock_mkdir, \ @@ -623,7 +623,17 @@ class QuobyteDriverTestCase(test.TestCase): qb_snso_mock.assert_called_once_with(is_new_cinder_install=mock.ANY) self.assertFalse(drv.configuration.quobyte_overlay_volumes) - def test_check_for_setup_error_throws_quobyte_volume_url_not_set(self): + @mock.patch.object(quobyte, 'psutil', new=None) + def test_check_for_setup_error_throws_psutil_missing(self): + """check_for_setup_error raises if psutil not installed.""" + drv = self._driver + e = self.assertRaises(exception.VolumeDriverException, + drv.check_for_setup_error) + self.assertIn("psutil", str(e)) + + @mock.patch.object(quobyte, 'psutil') + def test_check_for_setup_error_throws_quobyte_volume_url_not_set( + self, mock_psutil): """check_for_setup_error throws if 'quobyte_volume_url' is not set.""" drv = self._driver @@ -633,7 +643,9 @@ class QuobyteDriverTestCase(test.TestCase): 'no Quobyte volume configured', drv.check_for_setup_error) - def test_check_for_setup_error_throws_client_not_installed(self): + @mock.patch.object(quobyte, 'psutil') + def test_check_for_setup_error_throws_client_not_installed( + self, mock_psutil): """check_for_setup_error throws if client is not installed.""" drv = self._driver drv._execute = mock.Mock(side_effect=OSError @@ -646,7 +658,9 @@ class QuobyteDriverTestCase(test.TestCase): check_exit_code=False, run_as_root=False) - def test_check_for_setup_error_throws_client_not_executable(self): + @mock.patch.object(quobyte, 'psutil') + def test_check_for_setup_error_throws_client_not_executable( + self, mock_psutil): """check_for_setup_error throws if client cannot be executed.""" drv = self._driver @@ -1469,9 +1483,10 @@ class QuobyteDriverTestCase(test.TestCase): drv.configuration.nas_secure_file_permissions) self.assertFalse(drv._execute_as_root) - @mock.patch.object(psutil, "disk_partitions") + @mock.patch.object(quobyte, 'psutil') @mock.patch.object(os, "stat") - def test_validate_volume_all_good_prefix_val(self, stat_mock, part_mock): + def test_validate_volume_all_good_prefix_val(self, stat_mock, ps_mock): + part_mock = ps_mock.disk_partitions part_mock.return_value = self.get_mock_partitions() drv = self._driver @@ -1488,9 +1503,10 @@ class QuobyteDriverTestCase(test.TestCase): stat_mock.assert_called_once_with(self.TEST_MNT_POINT) part_mock.assert_called_once_with(all=True) - @mock.patch.object(psutil, "disk_partitions") + @mock.patch.object(quobyte, 'psutil') @mock.patch.object(os, "stat") - def test_validate_volume_all_good_subtype_val(self, stat_mock, part_mock): + def test_validate_volume_all_good_subtype_val(self, stat_mock, ps_mock): + part_mock = ps_mock.disk_partitions part_mock.return_value = self.get_mock_partitions() part_mock.return_value[0].device = "not_quobyte" part_mock.return_value[0].fstype = "fuse.quobyte" @@ -1509,9 +1525,10 @@ class QuobyteDriverTestCase(test.TestCase): stat_mock.assert_called_once_with(self.TEST_MNT_POINT) part_mock.assert_called_once_with(all=True) - @mock.patch.object(psutil, "disk_partitions") + @mock.patch.object(quobyte, 'psutil') @mock.patch.object(os, "stat") - def test_validate_volume_mount_not_working(self, stat_mock, part_mock): + def test_validate_volume_mount_not_working(self, stat_mock, ps_mock): + part_mock = ps_mock.disk_partitions part_mock.return_value = self.get_mock_partitions() drv = self._driver @@ -1527,8 +1544,9 @@ class QuobyteDriverTestCase(test.TestCase): stat_mock.assert_called_once_with(self.TEST_MNT_POINT) part_mock.assert_called_once_with(all=True) - @mock.patch.object(psutil, "disk_partitions") - def test_validate_volume_no_mtab_entry(self, part_mock): + @mock.patch.object(quobyte, 'psutil') + def test_validate_volume_no_mtab_entry(self, ps_mock): + part_mock = ps_mock.disk_partitions part_mock.return_value = [] # no quobyte@ devices msg = ("Volume driver reported an error: " "No matching Quobyte mount entry for %(mpt)s" @@ -1541,8 +1559,9 @@ class QuobyteDriverTestCase(test.TestCase): self._driver._validate_volume, self.TEST_MNT_POINT) - @mock.patch.object(psutil, "disk_partitions") - def test_validate_volume_wrong_mount_type(self, part_mock): + @mock.patch.object(quobyte, 'psutil') + def test_validate_volume_wrong_mount_type(self, ps_mock): + part_mock = ps_mock.disk_partitions mypart = mock.Mock() mypart.device = "not-quobyte" mypart.mountpoint = self.TEST_MNT_POINT @@ -1560,8 +1579,9 @@ class QuobyteDriverTestCase(test.TestCase): self.TEST_MNT_POINT) part_mock.assert_called_once_with(all=True) - @mock.patch.object(psutil, "disk_partitions") - def test_validate_volume_stale_mount(self, part_mock): + @mock.patch.object(quobyte, 'psutil') + def test_validate_volume_stale_mount(self, ps_mock): + part_mock = ps_mock.disk_partitions part_mock.return_value = self.get_mock_partitions() drv = self._driver diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index db8c9f562ac..9c5962dd3ab 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -23,7 +23,10 @@ from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging from oslo_utils import fileutils -import psutil +try: + import psutil +except ImportError: + psutil = None from cinder import compute from cinder import coordination @@ -273,6 +276,10 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed): "setting will be ignored.") def check_for_setup_error(self): + if psutil is None: + msg = _("The python 'psutil' module is required by this driver.") + LOG.error(msg) + raise exception.VolumeDriverException(msg) if not self.configuration.quobyte_volume_url: msg = (_("There's no Quobyte volume configured (%s). Example:" " quobyte:///") % diff --git a/driver-requirements.txt b/driver-requirements.txt index 8cff370ed5e..2b5613ef513 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -53,3 +53,6 @@ websocket-client>=1.3.2 # LGPLv2+ # LINSTOR python-linstor>=1.7.0 # LGPLv3 + +# Quobyte +psutil>=5.7.2 # BSD diff --git a/releasenotes/notes/quobyte-extra-requires-8dc1761859da923a.yaml b/releasenotes/notes/quobyte-extra-requires-8dc1761859da923a.yaml new file mode 100644 index 00000000000..1e31c75ef19 --- /dev/null +++ b/releasenotes/notes/quobyte-extra-requires-8dc1761859da923a.yaml @@ -0,0 +1,9 @@ +--- +upgrade: + - | + Quobyte driver: The python ``psutil`` module is no longer a requirement for + cinder, so it may need to be installed separately if you are using Quobyte. + This can be done by specifying ``quobyte`` extras if installing via pip + (for example, ``pip install cinder[quobyte]``) or by installing ``psutil`` + from the package appropriate for your operating system and ensuring that + it is accessible to the cinder volume service. diff --git a/requirements.txt b/requirements.txt index 4bc51d9a505..bf5fb9205de 100644 --- a/requirements.txt +++ b/requirements.txt @@ -34,7 +34,6 @@ packaging>=20.4 paramiko>=2.7.2 # LGPLv2.1+ Paste>=3.4.3 # MIT PasteDeploy>=2.1.0 # MIT -psutil>=5.7.2 # BSD pyparsing>=2.4.7 # MIT python-barbicanclient>=5.0.1 # Apache-2.0 python-glanceclient>=3.2.2 # Apache-2.0 diff --git a/setup.cfg b/setup.cfg index 1f6fadeb347..064044f2734 100644 --- a/setup.cfg +++ b/setup.cfg @@ -92,6 +92,7 @@ all = dfs-sdk>=1.2.25 # Apache-2.0 rbd-iscsi-client>=0.1.8 # Apache-2.0 python-linstor>=1.7.0 # LGPLv3 + psutil>=5.7.2 # BSD datacore = websocket-client>=1.3.2 # LGPLv2+ powermax = @@ -123,6 +124,8 @@ rbd_iscsi = rbd-iscsi-client>=0.1.8 # Apache-2.0 linstor = python-linstor>=1.7.0 # LGPLv3 +quobyte = + psutil>=5.7.2 # BSD [mypy]