mgr/cephadm: adding daemon_name as argument to nvmeof get bundle API

When cephadm-signed are in use, we know to know exacly which nvmeof daemon is
being used so we get the correct certificates for this daemon in
particular

Fixes: https://tracker.ceph.com/issues/74377
Signed-off-by: Redouane Kachach <rkachach@ibm.com>

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 590763e8ae60cf33ae6e1dfcb636fc95fb7166a7..dbcd42a5ab6626e3a4f8dfc7fcac4bb35b90a278 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -3490,9 +3490,9 @@ Then run the following:
         return self.cert_mgr.key_ls(include_cephadm_generated_keys)
 
     @handle_orch_error
-    def get_nvmeof_tls_bundle(self, service_name: str) -> Dict[str, str]:
+    def get_nvmeof_tls_bundle(self, service_name: str, daemon_name: str) -> Dict[str, str]:
         nvmeof_svc = cast(NvmeofService, service_registry.get_service('nvmeof'))
-        tls_bundle = nvmeof_svc.get_nvmeof_tls_bundle(service_name)
+        tls_bundle = nvmeof_svc.get_nvmeof_tls_bundle(service_name, daemon_name)
         return tls_bundle._asdict() if tls_bundle else {}
 
     @handle_orch_error

diff --git a/src/pybind/mgr/cephadm/services/nvmeof.py b/src/pybind/mgr/cephadm/services/nvmeof.py
index fa8ec1741434efde7cacecdcbcc2a05427e04c9a..8ea15bec9d5695082f6628427b07d314d3e21843 100644 (file)
--- a/src/pybind/mgr/cephadm/services/nvmeof.py
+++ b/src/pybind/mgr/cephadm/services/nvmeof.py
@@ -368,7 +368,7 @@ class NvmeofService(CephService):
         ]
         return blocking_daemon_hosts
 
-    def _pick_running_daemon_host_for_service(self, service_name: str) -> Optional[str]:
+    def _get_daemon_hostname(self, service_name: str, daemon_name: str) -> Optional[str]:
         """
         Resolve a deterministic host for a service when HOST-scoped objects are needed.
         Picks the first RUNNING daemon host from the orchestrator cache.
@@ -381,17 +381,12 @@ class NvmeofService(CephService):
 
         for dd in dds:
             # dd.hostname is the short host name used for HOST-scoped certmgr objects
-            if dd.status == DaemonDescriptionStatus.running and dd.hostname:
-                return dd.hostname
-
-        # fallback: any host if nothing is RUNNING
-        for dd in dds:
-            if dd.hostname:
+            if dd.name == daemon_name:
                 return dd.hostname
 
         return None
 
-    def get_nvmeof_tls_bundle(self, service_name: str) -> Optional[NvmeofTLSBundle]:
+    def get_nvmeof_tls_bundle(self, service_name: str, daemon_name: str) -> Optional[NvmeofTLSBundle]:
         """
         Deterministic NVMeoF TLS bundle retrieval based on the NVMeoF spec's certificate_source.
 
@@ -443,7 +438,7 @@ class NvmeofService(CephService):
 
         # -------- CEPHADM_SIGNED --------
         elif cert_source == CertificateSource.CEPHADM_SIGNED.value:
-            hostname = self._pick_running_daemon_host_for_service(service_name)
+            hostname = self._get_daemon_hostname(service_name, daemon_name)
             if not hostname:
                 logger.error(f"certificate_source=cephadm-signed for '{service_name}' but no hostname could be resolved")
                 return None

diff --git a/src/pybind/mgr/cephadm/tests/services/test_nvmeof.py b/src/pybind/mgr/cephadm/tests/services/test_nvmeof.py
index 326421a71b660b80cd7180de1528d9ea4bfd43ed..88a1cc1458c31766f20d380634e6ad42b982a02a 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/services/test_nvmeof.py
+++ b/src/pybind/mgr/cephadm/tests/services/test_nvmeof.py
@@ -704,6 +704,9 @@ class TestNvmeofTLSBundle:
         if hasattr(cephadm_module.spec_store, 'spec_created'):
             cephadm_module.spec_store.spec_created[spec.service_name()] = datetime_now()
 
+    def _daemon_name(self, spec: NvmeofServiceSpec) -> str:
+        return f'{spec.service_name()}.test-daemon'
+
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
     def test_get_nvmeof_tls_bundle_ssl_disabled(self, cephadm_module: CephadmOrchestrator):
         """Test that SSL disabled returns empty bundle"""
@@ -716,7 +719,9 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
-        bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        bundle = nvmeof_service.get_nvmeof_tls_bundle(
+            spec.service_name(), self._daemon_name(spec)
+        )
 
         assert bundle is not None
         assert bundle.server_cert == ''
@@ -744,7 +749,9 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
-        bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        bundle = nvmeof_service.get_nvmeof_tls_bundle(
+            spec.service_name(), self._daemon_name(spec)
+        )
 
         assert bundle is not None
         assert bundle.server_cert == server_cert
@@ -778,7 +785,9 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
-        bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        bundle = nvmeof_service.get_nvmeof_tls_bundle(
+            spec.service_name(), self._daemon_name(spec)
+        )
 
         assert bundle is not None
         assert bundle.server_cert == server_cert
@@ -804,6 +813,7 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
+        daemon_name = self._daemon_name(spec)
 
         # Mock cert_mgr.get_cert and get_key for SERVICE-scoped lookups
         def _get_cert(name, service_name=None, host=None):
@@ -818,7 +828,7 @@ class TestNvmeofTLSBundle:
 
         with patch.object(cephadm_module.cert_mgr, 'get_cert', side_effect=_get_cert), \
              patch.object(cephadm_module.cert_mgr, 'get_key', side_effect=_get_key):
-            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name(), daemon_name)
 
         assert bundle is not None
         assert bundle.server_cert == server_cert
@@ -847,6 +857,7 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
+        daemon_name = self._daemon_name(spec)
 
         def _get_cert(name, service_name=None, host=None):
             if name == nvmeof_service.cert_name:
@@ -866,7 +877,7 @@ class TestNvmeofTLSBundle:
 
         with patch.object(cephadm_module.cert_mgr, 'get_cert', side_effect=_get_cert), \
              patch.object(cephadm_module.cert_mgr, 'get_key', side_effect=_get_key):
-            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name(), daemon_name)
 
         assert bundle is not None
         assert bundle.server_cert == server_cert
@@ -875,11 +886,11 @@ class TestNvmeofTLSBundle:
         assert bundle.client_key == client_key
         assert bundle.ca_cert == ca_cert
 
-    @patch("cephadm.services.nvmeof.NvmeofService._pick_running_daemon_host_for_service")
+    @patch("cephadm.services.nvmeof.NvmeofService._get_daemon_hostname")
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
-    def test_get_nvmeof_tls_bundle_cephadm_signed_server_only(self, _pick_host, cephadm_module: CephadmOrchestrator):
+    def test_get_nvmeof_tls_bundle_cephadm_signed_server_only(self, _get_daemon_hostname, cephadm_module: CephadmOrchestrator):
         """Test CEPHADM_SIGNED certificate source without mTLS"""
-        _pick_host.return_value = 'test-host'
+        _get_daemon_hostname.return_value = 'test-host'
 
         spec = NvmeofServiceSpec(
             service_id='test.group',
@@ -902,8 +913,13 @@ class TestNvmeofTLSBundle:
             return server_creds
 
         nvmeof_service = NvmeofService(cephadm_module)
-        with patch.object(cephadm_module.cert_mgr, 'get_self_signed_tls_credentials', side_effect=_get_self_signed):
-            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        daemon_name = self._daemon_name(spec)
+        with patch.object(
+            cephadm_module.cert_mgr,
+            'get_self_signed_tls_credentials',
+            side_effect=_get_self_signed,
+        ):
+            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name(), daemon_name)
 
         assert bundle is not None
         assert bundle.server_cert == ceph_generated_cert
@@ -912,11 +928,11 @@ class TestNvmeofTLSBundle:
         assert bundle.client_cert == ''
         assert bundle.client_key == ''
 
-    @patch("cephadm.services.nvmeof.NvmeofService._pick_running_daemon_host_for_service")
+    @patch("cephadm.services.nvmeof.NvmeofService._get_daemon_hostname")
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
-    def test_get_nvmeof_tls_bundle_cephadm_signed_with_mtls(self, _pick_host, cephadm_module: CephadmOrchestrator):
+    def test_get_nvmeof_tls_bundle_cephadm_signed_with_mtls(self, _get_daemon_hostname, cephadm_module: CephadmOrchestrator):
         """Test CEPHADM_SIGNED certificate source with mTLS enabled"""
-        _pick_host.return_value = 'test-host'
+        _get_daemon_hostname.return_value = 'test-host'
 
         spec = NvmeofServiceSpec(
             service_id='test.group',
@@ -946,8 +962,13 @@ class TestNvmeofTLSBundle:
             return server_creds
 
         nvmeof_service = NvmeofService(cephadm_module)
-        with patch.object(cephadm_module.cert_mgr, 'get_self_signed_tls_credentials', side_effect=_get_self_signed):
-            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        daemon_name = self._daemon_name(spec)
+        with patch.object(
+            cephadm_module.cert_mgr,
+            'get_self_signed_tls_credentials',
+            side_effect=_get_self_signed,
+        ):
+            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name(), daemon_name)
 
         assert bundle is not None
         assert bundle.server_cert == ceph_generated_cert
@@ -957,11 +978,11 @@ class TestNvmeofTLSBundle:
         # API returns the CA from server_creds
         assert bundle.ca_cert == cephadm_root_ca
 
-    @patch("cephadm.services.nvmeof.NvmeofService._pick_running_daemon_host_for_service")
+    @patch("cephadm.services.nvmeof.NvmeofService._get_daemon_hostname")
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
-    def test_get_nvmeof_tls_bundle_cephadm_signed_no_hostname(self, _pick_host, cephadm_module: CephadmOrchestrator):
+    def test_get_nvmeof_tls_bundle_cephadm_signed_no_hostname(self, _get_daemon_hostname, cephadm_module: CephadmOrchestrator):
         """Test CEPHADM_SIGNED returns None when no hostname can be resolved"""
-        _pick_host.return_value = None
+        _get_daemon_hostname.return_value = None
 
         spec = NvmeofServiceSpec(
             service_id='test.group',
@@ -973,14 +994,18 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
-        bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        bundle = nvmeof_service.get_nvmeof_tls_bundle(
+            spec.service_name(), self._daemon_name(spec)
+        )
         assert bundle is None
 
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
     def test_get_nvmeof_tls_bundle_service_not_found(self, cephadm_module: CephadmOrchestrator):
         """Test that None is returned when service doesn't exist"""
         nvmeof_service = NvmeofService(cephadm_module)
-        bundle = nvmeof_service.get_nvmeof_tls_bundle('nvmeof.nonexistent')
+        bundle = nvmeof_service.get_nvmeof_tls_bundle(
+            'nvmeof.nonexistent', 'nvmeof.nonexistent.test-daemon'
+        )
         assert bundle is None
 
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
@@ -996,7 +1021,9 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
-        bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+        bundle = nvmeof_service.get_nvmeof_tls_bundle(
+            spec.service_name(), self._daemon_name(spec)
+        )
         assert bundle is None
 
     @patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('{}'))
@@ -1013,11 +1040,12 @@ class TestNvmeofTLSBundle:
         self._store_spec(cephadm_module, spec)
 
         nvmeof_service = NvmeofService(cephadm_module)
+        daemon_name = self._daemon_name(spec)
 
         # Mock get_cert and get_key to return None for all lookups
         with patch.object(cephadm_module.cert_mgr, 'get_cert', return_value=None), \
              patch.object(cephadm_module.cert_mgr, 'get_key', return_value=None):
-            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name())
+            bundle = nvmeof_service.get_nvmeof_tls_bundle(spec.service_name(), daemon_name)
 
         assert bundle is not None
         assert bundle.server_cert == ''

diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py
index 14f108dfc7fcf9a032bfc9181c03b3d9fc98fa81..de58b0c9935ef235a86f714979ea748d2285c3d3 100644 (file)
--- a/src/pybind/mgr/orchestrator/_interface.py
+++ b/src/pybind/mgr/orchestrator/_interface.py
@@ -539,7 +539,7 @@ class Orchestrator(object):
     def cert_store_key_ls(self, include_cephadm_generated_keys: bool = False) -> OrchResult[Dict[str, Any]]:
         raise NotImplementedError()
 
-    def get_nvmeof_tls_bundle(self, service_name: str) -> OrchResult[Dict[str, str]]:
+    def get_nvmeof_tls_bundle(self, service_name: str, daemon_name: str) -> OrchResult[Dict[str, str]]:
         raise NotImplementedError()
 
     def cert_store_get_cert(