From: Sebastian Wagner Date: Wed, 1 Sep 2021 13:36:01 +0000 (+0200) Subject: pyhton-common: DriveGroupSpec: Allow unnamed OSD specs X-Git-Tag: v16.2.8~251^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=65471f8ad727f3bd4730cb03e56aa861c4c3e081;p=ceph.git pyhton-common: DriveGroupSpec: Allow unnamed OSD specs Cause it never actually worked as expected. Remove duplicated service_id check, cause it's already verified by parent method. Fixes: https://tracker.ceph.com/issues/46253 Signed-off-by: Sebastian Wagner (cherry picked from commit 8b567e132d75711179febac126c5ec8a250b8952) Conflicts: src/python-common/ceph/deployment/service_spec.py --- diff --git a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py index 5b421d09c53..3afddd69b68 100644 --- a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py +++ b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py @@ -74,11 +74,11 @@ def test_daemon_description(): def test_apply(): to = _TestOrchestrator('', 0, 0) completion = to.apply([ - ServiceSpec(service_type='nfs'), - ServiceSpec(service_type='nfs'), - ServiceSpec(service_type='nfs'), + ServiceSpec(service_type='nfs', service_id='foo'), + ServiceSpec(service_type='nfs', service_id='foo'), + ServiceSpec(service_type='nfs', service_id='foo'), ]) - res = '' + res = '' assert completion.result == [res, res, res] diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py index 0844768c674..823959eea9d 100644 --- a/src/python-common/ceph/deployment/drive_group.py +++ b/src/python-common/ceph/deployment/drive_group.py @@ -126,7 +126,8 @@ class DriveGroupValidationError(SpecValidationError): if it was raised in a different mgr module. """ - def __init__(self, name: str, msg: str): + def __init__(self, name: Optional[str], msg: str): + name = name or "" super(DriveGroupValidationError, self).__init__( f'Failed to validate OSD spec "{name}": {msg}') @@ -241,9 +242,7 @@ class DriveGroupSpec(ServiceSpec): 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'] + s_id = args.get('service_id', '') try: args['placement'] = PlacementSpec.from_json(json_drive_group.pop('placement')) except KeyError: @@ -288,9 +287,6 @@ class DriveGroupSpec(ServiceSpec): # type: () -> None super(DriveGroupSpec, self).validate() - if not self.service_id: - raise DriveGroupValidationError('', 'service_id is required') - if not isinstance(self.placement.host_pattern, str) and \ self.placement.host_pattern is not None: raise DriveGroupValidationError(self.service_id, 'host_pattern must be of type string') diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index a59d7a22297..e1935f0e9bf 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -431,7 +431,7 @@ class ServiceSpec(object): KNOWN_SERVICE_TYPES = 'alertmanager crash grafana iscsi mds mgr mon nfs ' \ 'node-exporter osd prometheus rbd-mirror rgw ' \ 'container cephadm-exporter ingress cephfs-mirror snmp-gateway'.split() - REQUIRES_SERVICE_ID = 'iscsi mds nfs osd rgw container ingress '.split() + REQUIRES_SERVICE_ID = 'iscsi mds nfs rgw container ingress '.split() MANAGED_CONFIG_OPTIONS = [ 'mds_join_fs', ] @@ -498,7 +498,7 @@ class ServiceSpec(object): #: ``container``, ``ingress`` self.service_id = None - if self.service_type in self.REQUIRES_SERVICE_ID: + if self.service_type in self.REQUIRES_SERVICE_ID or self.service_type == 'osd': self.service_id = service_id #: If set to ``true``, the orchestrator will not deploy nor remove @@ -658,15 +658,17 @@ class ServiceSpec(object): if not self.service_type: raise SpecValidationError('Cannot add Service: type required') - if self.service_type in self.REQUIRES_SERVICE_ID: - if not self.service_id: + if self.service_type != 'osd': + if self.service_type in self.REQUIRES_SERVICE_ID and not self.service_id: raise SpecValidationError('Cannot add Service: id required') + if self.service_type not in self.REQUIRES_SERVICE_ID and self.service_id: + raise SpecValidationError( + f'Service of type \'{self.service_type}\' should not contain a service id') + + if self.service_id: if not re.match('^[a-zA-Z0-9_.-]+$', self.service_id): raise SpecValidationError('Service id contains invalid characters, ' 'only [a-zA-Z0-9_.-] allowed') - elif self.service_id: - raise SpecValidationError( - f'Service of type \'{self.service_type}\' should not contain a service id') if self.placement is not None: self.placement.validate() diff --git a/src/python-common/ceph/tests/test_drive_group.py b/src/python-common/ceph/tests/test_drive_group.py index 7e1bbcd0658..dfc9bed35c0 100644 --- a/src/python-common/ceph/tests/test_drive_group.py +++ b/src/python-common/ceph/tests/test_drive_group.py @@ -39,7 +39,12 @@ def test_DriveGroup(test_input): '' ), ( - "Failed to validate Drive Group: OSD spec needs a `placement` key.", + 'Failed to validate OSD spec "": `placement` key required', + """data_devices: + all: True +""" + ), + ( 'Failed to validate OSD spec "mydg.data_devices": device selection cannot be empty', """ service_type: osd service_id: mydg