python-common: Improve DriveSelection error messages

Fixes: https://tracker.ceph.com/issues/50685
Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
(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 18775c424f1a4f2abeeb1fd43b55955ec569669c..8203796659b3cad5bcf06e15269c13a2329a82f2 100644 (file)
--- 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 70ff65144789315d7905a8abdb366aa18996a249..8aed3d19794f5bd12167377c000b7a190d43d7ba 100644 (file)
--- 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 ab72c431edd36b187533a6084957a484cc2167f0..3f154c4d77b098145f67cf535503ef9a927ab768 100644 (file)
--- 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