python-common: OSD specs: Improve quality of error messages

Fixes: https://tracker.ceph.com/issues/47401

Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
This commit is contained in:
Sebastian Wagner 2021-08-24 12:56:21 +02:00
parent b91f81801a
commit 4142c52d74
No known key found for this signature in database
GPG Key ID: 8D2442807E6979F8
2 changed files with 92 additions and 42 deletions

View File

@ -65,32 +65,30 @@ class DeviceSelection(object):
#: Matches all devices. Can only be used for data devices
self.all = all
self.validate()
def validate(self):
# type: () -> None
def validate(self, name: str) -> None:
props = [self.model, self.vendor, self.size, self.rotational] # type: List[Any]
if self.paths and any(p is not None for p in props):
raise DriveGroupValidationError(
'DeviceSelection: `paths` and other parameters are mutually exclusive')
name,
'device selection: `paths` and other parameters are mutually exclusive')
is_empty = not any(p is not None and p != [] for p in [self.paths] + props)
if not self.all and is_empty:
raise DriveGroupValidationError('DeviceSelection cannot be empty')
raise DriveGroupValidationError(name, 'device selection cannot be empty')
if self.all and not is_empty:
raise DriveGroupValidationError(
'DeviceSelection `all` and other parameters are mutually exclusive. {}'.format(
name,
'device selection: `all` and other parameters are mutually exclusive. {}'.format(
repr(self)))
@classmethod
def from_json(cls, device_spec):
# type: (dict) -> Optional[DeviceSelection]
if not device_spec:
return # type: ignore
return None
for applied_filter in list(device_spec.keys()):
if applied_filter not in cls._supported_filters:
raise DriveGroupValidationError(
"Filtering for <{}> is not supported".format(applied_filter))
raise KeyError(applied_filter)
return cls(**device_spec)
@ -134,8 +132,9 @@ class DriveGroupValidationError(SpecValidationError):
if it was raised in a different mgr module.
"""
def __init__(self, msg: str):
super(DriveGroupValidationError, self).__init__('Failed to validate Drive Group: ' + msg)
def __init__(self, name: str, msg: str):
super(DriveGroupValidationError, self).__init__(
f'Failed to validate OSD spec "{name}": {msg}')
class DriveGroupSpec(ServiceSpec):
@ -254,81 +253,107 @@ class DriveGroupSpec(ServiceSpec):
json_drive_group['placement'] = {'host_pattern': json_drive_group['host_pattern']}
del json_drive_group['host_pattern']
args['service_type'] = json_drive_group.pop('service_type', 'osd')
# service_id was not required in early octopus.
args['service_id'] = json_drive_group.pop('service_id', '')
s_id = args['service_id']
try:
args['placement'] = PlacementSpec.from_json(json_drive_group.pop('placement'))
except KeyError:
args['placement'] = PlacementSpec()
args['service_type'] = json_drive_group.pop('service_type', 'osd')
# service_id was not required in early octopus.
args['service_id'] = json_drive_group.pop('service_id', '')
# spec: was not mandatory in octopus
if 'spec' in json_drive_group:
args.update(cls._drive_group_spec_from_json(json_drive_group.pop('spec')))
args.update(cls._drive_group_spec_from_json(s_id, json_drive_group.pop('spec')))
else:
args.update(cls._drive_group_spec_from_json(json_drive_group))
args.update(cls._drive_group_spec_from_json(s_id, json_drive_group))
args['unmanaged'] = json_drive_group.pop('unmanaged', False)
return cls(**args)
@classmethod
def _drive_group_spec_from_json(cls, json_drive_group: dict) -> dict:
def _drive_group_spec_from_json(cls, name: str, json_drive_group: dict) -> dict:
for applied_filter in list(json_drive_group.keys()):
if applied_filter not in cls._supported_features:
raise DriveGroupValidationError(
"Feature <{}> is not supported".format(applied_filter))
name,
"Feature `{}` is not supported".format(applied_filter))
try:
args = {k: (DeviceSelection.from_json(v) if k.endswith('_devices') else v) for k, v in
def to_selection(key: str, vals: dict) -> Optional[DeviceSelection]:
try:
return DeviceSelection.from_json(vals)
except KeyError as e:
raise DriveGroupValidationError(
f'{name}.{key}',
f"Filtering for `{e.args[0]}` is not supported")
args = {k: (to_selection(k, v) if k.endswith('_devices') else v) for k, v in
json_drive_group.items()}
if not args:
raise DriveGroupValidationError("Didn't find Drivegroup specs")
raise DriveGroupValidationError(name, "Didn't find drive selections")
return args
except (KeyError, TypeError) as e:
raise DriveGroupValidationError(str(e))
raise DriveGroupValidationError(name, str(e))
def validate(self):
# type: () -> None
super(DriveGroupSpec, self).validate()
if not self.service_id:
raise DriveGroupValidationError('service_id is required')
raise DriveGroupValidationError('', 'service_id is required')
if not isinstance(self.placement.host_pattern, str) and \
self.placement.host_pattern is not None:
raise DriveGroupValidationError('host_pattern must be of type string')
raise DriveGroupValidationError(self.service_id, 'host_pattern must be of type string')
if self.data_devices is None:
raise DriveGroupValidationError("`data_devices` element is required.")
raise DriveGroupValidationError(self.service_id, "`data_devices` element is required.")
specs = [self.data_devices, self.db_devices, self.wal_devices, self.journal_devices]
for s in filter(None, specs):
s.validate()
specs_names = "data_devices db_devices wal_devices journal_devices".split()
specs = dict(zip(specs_names, [getattr(self, k) for k in specs_names]))
for k, s in [ks for ks in specs.items() if ks[1] is not None]:
assert s is not None
s.validate(f'{self.service_id}.{k}')
for s in filter(None, [self.db_devices, self.wal_devices, self.journal_devices]):
if s.all:
raise DriveGroupValidationError("`all` is only allowed for data_devices")
raise DriveGroupValidationError(
self.service_id,
"`all` is only allowed for data_devices")
if self.objectstore not in ('bluestore'):
raise DriveGroupValidationError(f"{self.objectstore} is not supported. Must be "
raise DriveGroupValidationError(self.service_id,
f"{self.objectstore} is not supported. Must be "
f"one of ('bluestore')")
if self.block_wal_size is not None and type(self.block_wal_size) not in [int, str]:
raise DriveGroupValidationError('block_wal_size must be of type int or string')
raise DriveGroupValidationError(
self.service_id,
'block_wal_size must be of type int or string')
if self.block_db_size is not None and type(self.block_db_size) not in [int, str]:
raise DriveGroupValidationError('block_db_size must be of type int or string')
raise DriveGroupValidationError(
self.service_id,
'block_db_size must be of type int or string')
if self.journal_size is not None and type(self.journal_size) not in [int, str]:
raise DriveGroupValidationError('journal_size must be of type int or string')
raise DriveGroupValidationError(
self.service_id,
'journal_size must be of type int or string')
if self.filter_logic not in ['AND', 'OR']:
raise DriveGroupValidationError('filter_logic must be either <AND> or <OR>')
raise DriveGroupValidationError(
self.service_id,
'filter_logic must be either <AND> or <OR>')
if self.method not in [None, 'lvm', 'raw']:
raise DriveGroupValidationError('method must be one of None, lvm, raw')
raise DriveGroupValidationError(
self.service_id,
'method must be one of None, lvm, raw')
if self.method == 'raw' and self.objectstore == 'filestore':
raise DriveGroupValidationError('method raw only supports bluestore')
raise DriveGroupValidationError(
self.service_id,
'method raw only supports bluestore')
yaml.add_representer(DriveGroupSpec, DriveGroupSpec.yaml_representer)

View File

@ -39,7 +39,8 @@ def test_DriveGroup(test_input):
''
),
(
'Failed to validate Drive Group: DeviceSelection cannot be empty', """
"Failed to validate Drive Group: OSD spec needs a `placement` key.",
'Failed to validate OSD spec "mydg.data_devices": device selection cannot be empty', """
service_type: osd
service_id: mydg
placement:
@ -49,7 +50,7 @@ data_devices:
"""
),
(
'Failed to validate Drive Group: filter_logic must be either <AND> or <OR>', """
'Failed to validate OSD spec "mydg": filter_logic must be either <AND> or <OR>', """
service_type: osd
service_id: mydg
placement:
@ -60,7 +61,7 @@ filter_logic: XOR
"""
),
(
'Failed to validate Drive Group: `data_devices` element is required.', """
'Failed to validate OSD spec "mydg": `data_devices` element is required.', """
service_type: osd
service_id: mydg
placement:
@ -68,6 +69,29 @@ placement:
spec:
db_devices:
model: model
"""
),
(
'Failed to validate OSD spec "mydg.db_devices": Filtering for `unknown_key` is not supported', """
service_type: osd
service_id: mydg
placement:
host_pattern: '*'
spec:
db_devices:
unknown_key: 1
"""
),
(
'Failed to validate OSD spec "mydg": Feature `unknown_key` is not supported', """
service_type: osd
service_id: mydg
placement:
host_pattern: '*'
spec:
db_devices:
all: true
unknown_key: 1
"""
),
])
@ -95,7 +119,8 @@ def test_drive_selection():
assert spec.data_devices.paths[0].path == '/dev/sda'
with pytest.raises(DriveGroupValidationError, match='exclusive'):
DeviceSelection(paths=['/dev/sda'], rotational=False)
ds = DeviceSelection(paths=['/dev/sda'], rotational=False)
ds.validate('')
def test_ceph_volume_command_0():