Merge pull request ceph#54812 from adk3798/osd-spec-needs-id

mgr/orchestrator: block OSD specs with no service id

Reviewed-by: Guillaume Abrioux <[email protected]>

Author: adk3798

doc/cephadm/services/osd.rst

@@ -454,6 +454,15 @@ configurations, without knowing the specifics of device names and paths.
 Service specifications make it possible to define a yaml or json file that can
 be used to reduce the amount of manual work involved in creating OSDs.
 
+.. note::
+    It is recommended that advanced OSD specs include the ``service_id`` field
+    set. The plain ``osd`` service with no service id is where OSDs created
+    using ``ceph orch daemon add`` or ``ceph orch apply osd --all-available-devices``
+    are placed. Not including a ``service_id`` in your OSD spec would mix
+    the OSDs from your spec with those OSDs and potentially overwrite services
+    specs created by cephadm to track them. Newer versions of cephadm will even
+    block creation of advanced OSD specs without the service_id present
+
 For example, instead of running the following command:
 
 .. prompt:: bash [monitor.1]#

src/pybind/mgr/orchestrator/module.py

@@ -1372,6 +1372,21 @@ def apply_misc(self,
                         except KeyError:
                             raise SpecValidationError(f'Invalid config option {k} in spec')
 
+                # There is a general "osd" service with no service id, but we use
+                # that to dump osds created individually with "ceph orch daemon add osd"
+                # and those made with "ceph orch apply osd --all-available-devices"
+                # For actual user created OSD specs, we should promote users having a
+                # service id so it doesn't get mixed in with those other OSDs. This
+                # check is being done in this spot in particular as this is the only
+                # place we can 100% differentiate between an actual user created OSD
+                # spec and a spec we made ourselves to cover the all-available-devices case
+                if (
+                    isinstance(spec, DriveGroupSpec)
+                    and spec.service_type == 'osd'
+                    and not spec.service_id
+                ):
+                    raise SpecValidationError('Please provide the service_id field in your OSD spec')
+
                 if dry_run and not isinstance(spec, HostSpec):
                     spec.preview_only = dry_run