From: Michael Fritch <mfritch@suse.com>
Date: Mon, 27 Apr 2020 17:41:50 +0000 (-0600)
Subject: mgr/orch: allow '.' chars in the service_id
X-Git-Tag: v16.1.0~2421^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0bd6d46af68ad25d54ab7b3dfd3f55731fe810cf;p=ceph.git

mgr/orch: allow '.' chars in the service_id

service_id can contain a '.' char (mds, nfs, iscsi)

Fixes: https://tracker.ceph.com/issues/45293
Signed-off-by: Michael Fritch <mfritch@suse.com>
---

diff --git a/src/pybind/mgr/cephadm/tests/test_spec.py b/src/pybind/mgr/cephadm/tests/test_spec.py
index de5eb1530bb2..3d0826391e0f 100644
--- a/src/pybind/mgr/cephadm/tests/test_spec.py
+++ b/src/pybind/mgr/cephadm/tests/test_spec.py
@@ -2,7 +2,9 @@ import json
 
 import pytest
 
-from ceph.deployment.service_spec import ServiceSpec, RGWSpec, PlacementSpec
+from ceph.deployment.service_spec import ServiceSpec, NFSServiceSpec, RGWSpec, \
+    ServiceSpecValidationError, IscsiServiceSpec, PlacementSpec
+
 from orchestrator import DaemonDescription, OrchestratorError
 
 
@@ -224,8 +226,8 @@ def test_spec_octopus():
 
 @pytest.mark.parametrize("spec,dd,valid",
 [
+    # https://tracker.ceph.com/issues/44934
     (
-        # https://tracker.ceph.com/issues/44934
         RGWSpec(
             rgw_realm="default-rgw-realm",
             rgw_zone="eu-central-1",
@@ -280,8 +282,125 @@ def test_spec_octopus():
         ),
         False
     ),
+
+    # https://tracker.ceph.com/issues/45293
+    (
+        ServiceSpec(
+            service_type='mds',
+            service_id="a",
+        ),
+        DaemonDescription(
+            daemon_type='mds',
+            daemon_id="a.host1.abc123",
+            hostname="host1",
+        ),
+        True
+    ),
+    (
+        # '.' char in service_id
+        ServiceSpec(
+            service_type='mds',
+            service_id="a.b.c",
+        ),
+        DaemonDescription(
+            daemon_type='mds',
+            daemon_id="a.b.c.host1.abc123",
+            hostname="host1",
+        ),
+        True
+    ),
+
+    # https://tracker.ceph.com/issues/45293
+    (
+        NFSServiceSpec(
+            service_id="a",
+        ),
+        DaemonDescription(
+            daemon_type='nfs',
+            daemon_id="a.host1",
+            hostname="host1",
+        ),
+        True
+    ),
+    (
+        # service_id contains a '.' char
+        NFSServiceSpec(
+            service_id="a.b.c",
+        ),
+        DaemonDescription(
+            daemon_type='nfs',
+            daemon_id="a.b.c.host1",
+            hostname="host1",
+        ),
+        True
+    ),
+    (
+        # trailing chars after hostname
+        NFSServiceSpec(
+            service_id="a.b.c",
+        ),
+        DaemonDescription(
+            daemon_type='nfs',
+            daemon_id="a.b.c.host1.abc123",
+            hostname="host1",
+        ),
+        True
+    ),
+    (
+        # chars after hostname without '.'
+        NFSServiceSpec(
+            service_id="a",
+        ),
+        DaemonDescription(
+            daemon_type='nfs',
+            daemon_id="a.host1abc123",
+            hostname="host1",
+        ),
+        False
+    ),
+    (
+        # chars before hostname without '.'
+        NFSServiceSpec(
+            service_id="a",
+        ),
+        DaemonDescription(
+            daemon_type='nfs',
+            daemon_id="a.abc123",
+            hostname="host1",
+        ),
+        False
+    ),
+
+    # https://tracker.ceph.com/issues/45293
+    (
+        IscsiServiceSpec(
+            service_type='iscsi',
+            service_id="a",
+        ),
+        DaemonDescription(
+            daemon_type='iscsi',
+            daemon_id="a.host1.abc123",
+            hostname="host1",
+        ),
+        True
+    ),
+    (
+        # '.' char in service_id
+        IscsiServiceSpec(
+            service_type='iscsi',
+            service_id="a.b.c",
+        ),
+        DaemonDescription(
+            daemon_type='iscsi',
+            daemon_id="a.b.c.host1.abc123",
+            hostname="host1",
+        ),
+        True
+    ),
 ])
-def test_rgw_service_name(spec: RGWSpec, dd: DaemonDescription, valid):
+def test_daemon_description_service_name(spec: ServiceSpec,
+                                         dd: DaemonDescription,
+                                         valid: bool):
     if valid:
         assert spec.service_name() == dd.service_name()
     else:
diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py
index d6d94930de60..ee482adee61e 100644
--- a/src/pybind/mgr/orchestrator/_interface.py
+++ b/src/pybind/mgr/orchestrator/_interface.py
@@ -4,15 +4,18 @@ ceph-mgr orchestrator interface
 
 Please see the ceph-mgr module developer's guide for more information.
 """
+
+import copy
+import datetime
+import errno
 import logging
 import pickle
+import re
 import time
+import uuid
+
 from collections import namedtuple
 from functools import wraps
-import uuid
-import datetime
-import copy
-import errno
 
 from ceph.deployment import inventory
 from ceph.deployment.service_spec import ServiceSpec, NFSServiceSpec, RGWSpec, \
@@ -1292,6 +1295,15 @@ class DaemonDescription(object):
         return False
 
     def service_id(self):
+        def _match():
+            if self.hostname:
+                # daemon_id == "service_id.hostname"
+                # daemon_id == "service_id.hostname.random"
+                p = re.compile(r'(.*)\.%s(\.{1}\w+)?$' % (self.hostname))
+                m = p.match(self.daemon_id)
+                if m:
+                    return m.group(1)
+
         if self.daemon_type == 'rgw':
             if self.hostname and self.hostname in self.daemon_id:
                 pre, post_ = self.daemon_id.split(self.hostname)
@@ -1303,9 +1315,15 @@ class DaemonDescription(object):
                     return '.'.join(v[0:2])
                 # subcluster or fqdn? undecidable.
                 raise OrchestratorError(f"DaemonDescription: Cannot calculate service_id: {v}")
+
         if self.daemon_type in ['mds', 'nfs', 'iscsi']:
-            return self.daemon_id.split('.')[0]
-        return self.daemon_type
+            service_id = _match()
+            if service_id:
+                return service_id
+            raise OrchestratorError("DaemonDescription: Cannot calculate service_id: " \
+                    f"daemon_id='{self.daemon_id}' hostname='{self.hostname}'")
+
+        return self.daemon_id
 
     def service_name(self):
         if self.daemon_type in ['rgw', 'mds', 'nfs', 'iscsi']: