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 <sewagner@redhat.com>
(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 5b421d09c53e23164d1e987e5c7f089573665691..3afddd69b689a0178b3ef8f2bff4f385a9601c93 100644 (file)
--- 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 = '<NFSServiceSpec for service_name=nfs>'
+    res = '<NFSServiceSpec for service_name=nfs.foo>'
     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 0844768c67427f1d7cc2e56e22d4158c69493314..823959eea9d02e7f9ca6fb3ef3aec96d93f53a60 100644 (file)
--- 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 "<unnamed>"
         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', '<unnamed>')
         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 a59d7a2229724c617af7f951c80094fc90d30d13..e1935f0e9bf66f3a2e323a5f83dcee7432dd2269 100644 (file)
--- 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 7e1bbcd0658af629d04a9434029c1072e8b4c389..dfc9bed35c057f730dd95e905fd5c0da98c16578 100644 (file)
--- 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