From: Sebastian Wagner Date: Tue, 24 Aug 2021 12:57:27 +0000 (+0200) Subject: python-common: Improve DriveSelection error messages X-Git-Tag: v16.2.8~251^2~5 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=5992b8078f8857f507147d110f016de919be41af;p=ceph.git python-common: Improve DriveSelection error messages Fixes: https://tracker.ceph.com/issues/50685 Signed-off-by: Sebastian Wagner (cherry picked from commit 74f29b97ea3331d43391cd40fe843104a2c15c3d) --- diff --git a/src/python-common/ceph/deployment/drive_selection/matchers.py b/src/python-common/ceph/deployment/drive_selection/matchers.py index 18775c424f1a4..8203796659b3c 100644 --- a/src/python-common/ceph/deployment/drive_selection/matchers.py +++ b/src/python-common/ceph/deployment/drive_selection/matchers.py @@ -10,6 +10,10 @@ import logging logger = logging.getLogger(__name__) +class _MatchInvalid(Exception): + pass + + # pylint: disable=too-few-public-methods class Matcher(object): """ The base class to all Matchers @@ -70,7 +74,7 @@ class Matcher(object): if disk_value: return disk_value[0] else: - raise Exception("No value found for {} or {}".format( + raise _MatchInvalid("No value found for {} or {}".format( self.key, self.fallback_key)) def compare(self, disk): @@ -256,7 +260,7 @@ class SizeMatcher(Matcher): """ suffix = suffix.upper() if suffix not in cls.supported_suffixes: - raise ValueError("Unit '{}' not supported".format(suffix)) + raise _MatchInvalid("Unit '{}' not supported".format(suffix)) return dict(zip( cls.SUFFIXES[1], cls.SUFFIXES[0], @@ -327,7 +331,7 @@ class SizeMatcher(Matcher): self.exact = self._get_k_v(exact.group()) if not self.low and not self.high and not self.exact: - raise Exception("Couldn't parse {}".format(self.value)) + raise _MatchInvalid("Couldn't parse {}".format(self.value)) @staticmethod # pylint: disable=inconsistent-return-statements @@ -404,5 +408,5 @@ class SizeMatcher(Matcher): logger.debug("Disk didn't match for 'exact' filter") else: logger.debug("Neither high, low, nor exact was given") - raise Exception("No filters applied") + raise _MatchInvalid("No filters applied") return False diff --git a/src/python-common/ceph/deployment/drive_selection/selector.py b/src/python-common/ceph/deployment/drive_selection/selector.py index 70ff651447893..8aed3d19794f5 100644 --- a/src/python-common/ceph/deployment/drive_selection/selector.py +++ b/src/python-common/ceph/deployment/drive_selection/selector.py @@ -1,18 +1,27 @@ import logging -try: - from typing import List, Optional, Dict -except ImportError: - pass +from typing import List, Optional, Dict, Callable from ..inventory import Device -from ..drive_group import DriveGroupSpec, DeviceSelection +from ..drive_group import DriveGroupSpec, DeviceSelection, DriveGroupValidationError from .filter import FilterGenerator +from .matchers import _MatchInvalid logger = logging.getLogger(__name__) +def to_dg_exception(f: Callable) -> Callable[['DriveSelection', str, + Optional['DeviceSelection']], + List['Device']]: + def wrapper(self: 'DriveSelection', name: str, ds: Optional['DeviceSelection']) -> List[Device]: + try: + return f(self, ds) + except _MatchInvalid as e: + raise DriveGroupValidationError(f'{self.spec.service_id}.{name}', e.args[0]) + return wrapper + + class DriveSelection(object): def __init__(self, spec, # type: DriveGroupSpec @@ -23,10 +32,10 @@ class DriveSelection(object): self.spec = spec self.existing_daemons = existing_daemons or 0 - self._data = self.assign_devices(self.spec.data_devices) - self._wal = self.assign_devices(self.spec.wal_devices) - self._db = self.assign_devices(self.spec.db_devices) - self._journal = self.assign_devices(self.spec.journal_devices) + self._data = self.assign_devices('data_devices', self.spec.data_devices) + self._wal = self.assign_devices('wal_devices', self.spec.wal_devices) + self._db = self.assign_devices('db_devices', self.spec.db_devices) + self._journal = self.assign_devices('journal_devices', self.spec.journal_devices) def data_devices(self): # type: () -> List[Device] @@ -79,6 +88,7 @@ class DriveSelection(object): raise Exception( "Disk {} doesn't have a 'path' identifier".format(disk)) + @to_dg_exception def assign_devices(self, device_filter): # type: (Optional[DeviceSelection]) -> List[Device] """ Assign drives based on used filters diff --git a/src/python-common/ceph/tests/test_disk_selector.py b/src/python-common/ceph/tests/test_disk_selector.py index ab72c431edd36..3f154c4d77b09 100644 --- a/src/python-common/ceph/tests/test_disk_selector.py +++ b/src/python-common/ceph/tests/test_disk_selector.py @@ -1,6 +1,7 @@ # flake8: noqa import pytest +from ceph.deployment.drive_selection.matchers import _MatchInvalid from ceph.deployment.inventory import Devices, Device from ceph.deployment.drive_group import DriveGroupSpec, DeviceSelection, \ @@ -142,7 +143,7 @@ class TestSizeMatcher(object): def test_to_byte_PB(self): """ Expect to raise """ - with pytest.raises(ValueError): + with pytest.raises(_MatchInvalid): drive_selection.SizeMatcher('size', '10P').to_byte(('10', 'PB')) assert 'Unit \'P\' is not supported' @@ -269,7 +270,7 @@ class TestSizeMatcher(object): def test_normalize_suffix_raises(self): - with pytest.raises(ValueError): + with pytest.raises(_MatchInvalid): drive_selection.SizeMatcher('10P', 'size')._normalize_suffix("P") pytest.fail("Unit 'P' not supported") @@ -559,3 +560,14 @@ class TestDriveSelection(object): sel = drive_selection.DriveSelection(spec, inventory) assert [d.path for d in sel.data_devices()] == expected_data assert [d.path for d in sel.db_devices()] == expected_db + + def test_disk_selection_raise(self): + spec = DriveGroupSpec( + placement=PlacementSpec(host_pattern='*'), + service_id='foobar', + data_devices=DeviceSelection(size='wrong'), + ) + inventory = _mk_inventory(_mk_device(rotational=True)*2) + m = 'Failed to validate OSD spec "foobar.data_devices": No filters applied' + with pytest.raises(DriveGroupValidationError, match=m): + drive_selection.DriveSelection(spec, inventory) \ No newline at end of file