mgr/cephadm: mon: Don't show traceback for user errors

E.g. when complaining about a missing netowork.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index ca00adbeeb8d1e0f66fc757ecf639d4495d40887..fcf5ff10d0cb5ee85721158282b71a6f339f9ca6 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -1869,13 +1869,12 @@ class CephadmOrchestrator(MgrModule, orchestrator.OrchestratorClientMixin):
                                    keyring=keyring,
                                    extra_config=extra_config)
 
+    @async_completion
     def add_mon(self, spec):
         # type: (orchestrator.ServiceSpec) -> orchestrator.Completion
 
         # current support requires a network to be specified
-        for host, network, _ in spec.placement.hosts:
-            if not network:
-                raise RuntimeError("Host '{}' is missing a network spec".format(host))
+        orchestrator.servicespec_validate_hosts_have_network_spec(spec)
 
         def add_mons(daemons):
             for _, _, name in spec.placement.hosts:
@@ -1894,6 +1893,7 @@ class CephadmOrchestrator(MgrModule, orchestrator.OrchestratorClientMixin):
 
         return self._get_daemons('mon').then(add_mons)
 
+    @async_completion
     def apply_mon(self, spec):
         # type: (orchestrator.ServiceSpec) -> orchestrator.Completion
         """
@@ -1924,9 +1924,7 @@ class CephadmOrchestrator(MgrModule, orchestrator.OrchestratorClientMixin):
         [self._require_hosts(host.hostname) for host in spec.placement.hosts]
 
         # current support requires a network to be specified
-        for host, network, _ in spec.placement.hosts:
-            if not network:
-                raise RuntimeError("Host '{}' is missing a network spec".format(host))
+        orchestrator.servicespec_validate_hosts_have_network_spec(spec)
 
         def update_mons_with_daemons(daemons):
             for _, _, name in spec.placement.hosts:

diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index cf9b1bfe3c23d6206533dea9d986e573e011fb7c..e702b5ce38d6a212a40542d9c239fb6589e11034 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -113,6 +113,11 @@ class TestCephadm(object):
             c = cephadm_module.apply_mon(ServiceSpec(placement=ps))
             assert wait(cephadm_module, c) == ["Deployed mon.a on host 'test'"]
 
+            with pytest.raises(OrchestratorError, match="is missing a network spec"):
+                ps = PlacementSpec(hosts=['test'], count=1)
+                c = cephadm_module.apply_mon(ServiceSpec(placement=ps))
+                wait(cephadm_module, c)
+
     @mock.patch("cephadm.module.CephadmOrchestrator._run_cephadm", _run_cephadm('[]'))
     @mock.patch("cephadm.module.CephadmOrchestrator.send_command")
     @mock.patch("cephadm.module.CephadmOrchestrator.mon_command", mon_command)

diff --git a/src/pybind/mgr/orchestrator/__init__.py b/src/pybind/mgr/orchestrator/__init__.py
index 857d4b5ffc0c37dd238963587310705d8723d465..61cf50e0ff6bf8a41341bd7b12abee4f4ef8b5f4 100644 (file)
--- a/src/pybind/mgr/orchestrator/__init__.py
+++ b/src/pybind/mgr/orchestrator/__init__.py
@@ -9,6 +9,7 @@ from ._interface import \
     Orchestrator, OrchestratorClientMixin, \
     OrchestratorValidationError, OrchestratorError, NoOrchestrator, \
     ServiceSpec, NFSServiceSpec, RGWSpec, HostPlacementSpec, \
+    servicespec_validate_add, servicespec_validate_hosts_have_network_spec, \
     ServiceDescription, InventoryFilter, PlacementSpec,  HostSpec, \
     DaemonDescription, \
     InventoryNode, DeviceLightLoc, \

diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py
index e6915b9c0a349b66bba911679b63b352f7fb508c..8863c62e865af905c741ee0841b9c3525b3103fd 100644 (file)
--- a/src/pybind/mgr/orchestrator/_interface.py
+++ b/src/pybind/mgr/orchestrator/_interface.py
@@ -1366,9 +1366,28 @@ class ServiceSpec(object):
         else:
             self.count = 1
 
-    def validate_add(self):
-        if not self.name:
-            raise OrchestratorValidationError('Cannot add Service: Name required')
+
+def servicespec_validate_add(self: ServiceSpec):
+    # This must not be a method of ServiceSpec, otherwise you'll hunt
+    # sub-interpreter affinity bugs.
+    if not self.name:
+        raise OrchestratorValidationError('Cannot add Service: Name required')
+
+
+def servicespec_validate_hosts_have_network_spec(self: ServiceSpec):
+    # This must not be a method of ServiceSpec, otherwise you'll hunt
+    # sub-interpreter affinity bugs.
+    if not self.placement.hosts:
+        raise OrchestratorValidationError('Service specification: no hosts provided')
+
+    for host, network, _ in self.placement.hosts:
+        if not network:
+            m = "Host '{host}' is missing a network spec\nE.g. {host}:1.2.3.0/24".format(
+                host=host)
+            logger.error(
+                f'validate_hosts_have_network_spec: id(OrchestratorValidationError)={id(OrchestratorValidationError)}')
+            raise OrchestratorValidationError(m)
+
 
 
 class NFSServiceSpec(ServiceSpec):
@@ -1382,8 +1401,8 @@ class NFSServiceSpec(ServiceSpec):
         self.namespace = namespace
 
     def validate_add(self):
-        super(NFSServiceSpec, self).validate_add()
-
+        servicespec_validate_add(self)
+        
         if not self.pool:
             raise OrchestratorValidationError('Cannot add NFS: No Pool specified')
 

diff --git a/src/pybind/mgr/tests/test_orchestrator.py b/src/pybind/mgr/tests/test_orchestrator.py
index acd1b567dc27f44b0a4d0ed4312a9d176cfc9ecd..532fdd5b8bd70a6758c42d31064e674de47610d2 100644 (file)
--- a/src/pybind/mgr/tests/test_orchestrator.py
+++ b/src/pybind/mgr/tests/test_orchestrator.py
@@ -6,7 +6,8 @@ from tests import mock
 import pytest
 
 from ceph.deployment import inventory
-from orchestrator import raise_if_exception, RGWSpec, Completion, ProgressReference
+from orchestrator import raise_if_exception, RGWSpec, Completion, ProgressReference, \
+    servicespec_validate_add
 from orchestrator import InventoryNode, ServiceDescription
 from orchestrator import OrchestratorValidationError
 from orchestrator import HostPlacementSpec
@@ -110,7 +111,7 @@ def test_rgwspec():
     """
     example = json.loads(test_rgwspec.__doc__.strip())
     spec = RGWSpec.from_json(example)
-    assert spec.validate_add() is None
+    assert servicespec_validate_add(spec) is None
 
 
 def test_promise():