mgr/cephadm: addressing reviewer comments

Signed-off-by: Redouane Kachach <rkachach@ibm.com>

diff --git a/src/pybind/mgr/cephadm/cert_mgr.py b/src/pybind/mgr/cephadm/cert_mgr.py
index 2977916568e2e5d2b2ed3b5e2a467511aeb86208..aabc587db175f35d3ca98988af279db51e6db113 100644 (file)
--- a/src/pybind/mgr/cephadm/cert_mgr.py
+++ b/src/pybind/mgr/cephadm/cert_mgr.py
@@ -42,7 +42,7 @@ class CertInfo:
         return self.is_valid and not self.is_close_to_expiration
 
     def get_status_description(self) -> str:
-        cert_source = 'user-made' if self.user_made else 'self-signed'
+        cert_source = 'user-made' if self.user_made else 'cephadm-signed'
         cert_target = f' ({self.target})' if self.target else ''
         cert_details = f"'{self.cert_name}{cert_target}' ({cert_source})"
         if not self.is_valid:
@@ -66,7 +66,7 @@ class CertMgr:
     It tracks known certificates and private keys, associates them with services, and ensures
     their validity. If certificates are close to expiration or invalid, depending on the configuration
     (governed by the mgr/cephadm/certificate_automated_rotation_enabled parameter), CertMgr generates
-    warnings or attempts renewal for self-signed certificates.
+    warnings or attempts renewal for cephadm-signed certificates.
 
     Additionally, CertMgr provides methods for certificate management, including retrieving, saving,
     and removing certificates and keys, as well as reporting certificate health status in case of issues.
@@ -385,8 +385,8 @@ class CertMgr:
                                f'error: {cert_info.error_info}')
 
         # Reaching this point means either certificates are not present or they are
-        # invalid self-signed certificates. Either way, we will just generate new ones.
-        logger.info(f'Generating cephadm self-signed certificates for {cert_name}/{key_name}')
+        # invalid cephadm-signed certificates. Either way, we will just generate new ones.
+        logger.info(f'Generating cephadm-signed certificates for {cert_name}/{key_name}')
         cert, pkey = self.generate_cert(host_fqdns, host_ips)
         self.mgr.cert_mgr.save_cert(cert_name, cert, host=target_host, service_name=target_service)
         self.mgr.cert_mgr.save_key(key_name, pkey, host=target_host, service_name=target_service)
@@ -434,7 +434,7 @@ class CertMgr:
 
     def _renew_self_signed_certificate(self, cert_info: CertInfo, cert_obj: Cert) -> bool:
         try:
-            logger.info(f'Renewing self-signed certificate for {cert_info.cert_name}')
+            logger.info(f'Renewing cephadm-signed certificate for {cert_info.cert_name}')
             new_cert, new_key = self.ssl_certs.renew_cert(cert_obj.cert, self.mgr.certificate_duration_days)
             service_name, host = self.cert_store.determine_tlsobject_target(cert_info.cert_name, cert_info.target)
             self.cert_store.save_tlsobject(cert_info.cert_name, new_cert, service_name=service_name, host=host)
@@ -442,7 +442,7 @@ class CertMgr:
             self.key_store.save_tlsobject(key_name, new_key, service_name=service_name, host=host)
             return True
         except SSLConfigException as e:
-            logger.error(f'Error while trying to renew self-signed certificate for {cert_info.cert_name}: {e}')
+            logger.error(f'Error while trying to renew cephadm-signed certificate for {cert_info.cert_name}: {e}')
             return False
 
     def check_services_certificates(self, fix_issues: bool = False) -> Tuple[List[str], List[CertInfo]]:
@@ -466,7 +466,7 @@ class CertMgr:
             if not self.mgr.certificate_automated_rotation_enabled or cert_obj.user_made:
                 return False
 
-            # This is a self-signed certificate, let's try to fix it
+            # This is a cephadm-signed certificate, let's try to fix it
             if not cert_info.is_valid:
                 # Remove the invalid certificate to force regeneration
                 service_name, host = self.cert_store.determine_tlsobject_target(cert_info.cert_name, cert_info.target)

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 2822697dd59d1e6beea748009232f1e84dccf579..d3b027ce1fd50f53e4ba7bd61f6da989bff9057c 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -55,6 +55,7 @@ from mgr_module import (
     MgrModule,
     HandleCommandResult,
     Option,
+    OptionLevel,
     NotifyType,
     MonCommandFailed,
 )
@@ -412,6 +413,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         ),
         Option(
             'certificate_check_debug_mode',
+            level=OptionLevel.DEV,
             type='bool',
             default=False,
             desc='FOR TESTING ONLY: This flag forces the certificate check instead of waiting for certificate_check_period.',

diff --git a/src/pybind/mgr/cephadm/services/mgmt_gateway.py b/src/pybind/mgr/cephadm/services/mgmt_gateway.py
index d21478333ded5c2e5dc2c9ac041349c307c8351a..1ca7bb7485578f4086456dbd645e94ee825f62a0 100644 (file)
--- a/src/pybind/mgr/cephadm/services/mgmt_gateway.py
+++ b/src/pybind/mgr/cephadm/services/mgmt_gateway.py
@@ -162,7 +162,7 @@ class MgmtGatewayService(CephadmService):
 
         return daemon_config, sorted(MgmtGatewayService.get_dependencies(self.mgr))
 
-    def pre_remove(self, daemon: DaemonDescription) -> None:
+    def post_remove(self, daemon: DaemonDescription, is_failed_deploy: bool) -> None:
         """
         Called before mgmt-gateway daemon is removed.
         """

diff --git a/src/pybind/mgr/cephadm/services/oauth2_proxy.py b/src/pybind/mgr/cephadm/services/oauth2_proxy.py
index 372383bdb6a811f71d95d460842dd6846108bfdd..1b77d0faafd4b6ecf9fb7b9f2c3aab7aeee3cb69 100644 (file)
--- a/src/pybind/mgr/cephadm/services/oauth2_proxy.py
+++ b/src/pybind/mgr/cephadm/services/oauth2_proxy.py
@@ -89,7 +89,7 @@ class OAuth2ProxyService(CephadmService):
 
         return daemon_config, []
 
-    def pre_remove(self, daemon: DaemonDescription) -> None:
+    def post_remove(self, daemon: DaemonDescription, is_failed_deploy: bool) -> None:
         """
         Called before mgmt-gateway daemon is removed.
         """

diff --git a/src/pybind/mgr/cephadm/tests/test_certmgr.py b/src/pybind/mgr/cephadm/tests/test_certmgr.py
index 6d20791a488205a9acdcb0f7245204ebef0125a3..38accc16a7d533cf33c577ffd5527118dcd61391 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_certmgr.py
+++ b/src/pybind/mgr/cephadm/tests/test_certmgr.py
@@ -715,7 +715,7 @@ class TestCertMgr(object):
 
     @mock.patch("cephadm.module.CephadmOrchestrator.set_store")
     def test_certificate_renewal_for_self_signed(self, _set_store, cephadm_module: CephadmOrchestrator):
-        """ Test that self-signed certificates close to expiration are renewed """
+        """ Test that cephadm-signed certificates close to expiration are renewed """
         cert_mgr = cephadm_module.cert_mgr
 
         # for services with host scope
@@ -758,7 +758,7 @@ class TestCertMgr(object):
                                            'Detected 2 cephadm certificate(s) issues: 1 invalid, 1 expired',
                                            2,
                                            ["Certificate 'test_service_1 (target_1)' (user-made) has expired",
-                                            "Certificate 'test_service_2 (target_2)' (self-signed) is not valid (error: invalid format)"])
+                                            "Certificate 'test_service_2 (target_2)' (cephadm-signed) is not valid (error: invalid format)"])
 
         # Test in case of appending new errors we also report previous ones
         problematic_certs = [
@@ -770,8 +770,8 @@ class TestCertMgr(object):
                                            'Detected 3 cephadm certificate(s) issues: 1 invalid, 1 expired, 1 expiring',
                                            3,
                                            ["Certificate 'test_service_1 (target_1)' (user-made) has expired",
-                                            "Certificate 'test_service_2 (target_2)' (self-signed) is not valid (error: invalid format)",
-                                            "Certificate 'test_service_3 (target_3)' (self-signed) is about to expire (remaining days: 0)"])
+                                            "Certificate 'test_service_2 (target_2)' (cephadm-signed) is not valid (error: invalid format)",
+                                            "Certificate 'test_service_3 (target_3)' (cephadm-signed) is about to expire (remaining days: 0)"])
 
     @mock.patch("cephadm.module.CephadmOrchestrator.set_store")
     def test_health_warning_on_bad_certificates(self, _set_store, cephadm_module: CephadmOrchestrator):

diff --git a/src/pybind/mgr/cephadm/tlsobject_store.py b/src/pybind/mgr/cephadm/tlsobject_store.py
index 52f26f7a648dc2caa43c73a6c26894a3e0f301da..83ff9e14a9cf613d20bb2442d901ec2668616ba2 100644 (file)
--- a/src/pybind/mgr/cephadm/tlsobject_store.py
+++ b/src/pybind/mgr/cephadm/tlsobject_store.py
@@ -143,7 +143,7 @@ class TLSObjectStore():
         for k, v in self.mgr.get_store_prefix(self.store_prefix).items():
             entity = k[len(self.store_prefix):]
             if entity not in self.known_entities:
-                logger.warning(f"TLSObjectStore: Discarding unkown entity '{entity}'")
+                logger.warning(f"TLSObjectStore: Discarding unknown entity '{entity}'")
                 continue
             entity_targets = json.loads(v)
             if entity in self.per_service_name_tlsobjects or entity in self.per_host_tlsobjects: