From: Shweta Bhosale <Shweta.Bhosale1@ibm.com>
Date: Mon, 10 Mar 2025 10:54:40 +0000 (+0530)
Subject: cephadm: Enable logroate.timer service in bootstrap process
X-Git-Tag: testing/wip-vshankar-testing-20250327.072724-debug~26^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f72a7f6e206d0befabf6f80c4d1ccbce282c67dc;p=ceph-ci.git

cephadm: Enable logroate.timer service in bootstrap process

Fixes: https://tracker.ceph.com/issues/70373
Signed-off-by: Shweta Bhosale <Shweta.Bhosale1@ibm.com>
---

diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py
index 6dc1d5cdfa0..770537d6f06 100755
--- a/src/cephadm/cephadm.py
+++ b/src/cephadm/cephadm.py
@@ -134,13 +134,13 @@ from cephadmlib.net_utils import (
 )
 from cephadmlib.locking import FileLock
 from cephadmlib.daemon_identity import DaemonIdentity, DaemonSubIdentity
-from cephadmlib.packagers import create_packager, Packager
+from cephadmlib.packagers import create_packager
 from cephadmlib.logging import (
     cephadm_init_logging,
     Highlight,
     LogDestination,
 )
-from cephadmlib.systemd import check_unit, check_units, terminate_service
+from cephadmlib.systemd import check_unit, check_units, terminate_service, enable_service
 from cephadmlib import systemd_unit
 from cephadmlib import runscripts
 from cephadmlib.container_types import (
@@ -2946,6 +2946,13 @@ def command_bootstrap(ctx):
     if getattr(ctx, 'custom_prometheus_alerts', None):
         cli(['orch', 'prometheus', 'set-custom-alerts', '-i', '/etc/ceph/custom_alerts.yml'])
 
+    # Enable logrotate.timer
+    _, _, installed = check_unit(ctx, 'logrotate')
+    if not installed:
+        logger.warning('Log rotation will not occur because the logrotate service is not installed. Please install it to enable log rotation.')
+    else:
+        logger.info('enable logrotate.timer service')
+        enable_service(ctx, 'logrotate.timer')
     return ctx.error_code
 
 ##################################
@@ -4385,7 +4392,7 @@ def _rm_cluster(ctx: CephadmContext, keep_logs: bool, zap_osds: bool) -> None:
 
 
 def check_time_sync(
-    ctx: CephadmContext, enabler: Optional[Packager] = None
+    ctx: CephadmContext
 ) -> bool:
     units = [
         'chrony.service',  # 18.04 (at least)
@@ -4397,7 +4404,7 @@ def check_time_sync(
         'openntpd.service',  # ubuntu / debian
         'timemaster.service',  # linuxptp on ubuntu/debian
     ]
-    if not check_units(ctx, units, enabler):
+    if not check_units(ctx, units):
         logger.warning('No time sync service is running; checked for %s' % units)
         return False
     return True
@@ -4465,7 +4472,7 @@ def command_prepare_host(ctx: CephadmContext) -> None:
         pkg.install(['chrony'])
         # check again, and this time try to enable
         # the service
-        check_time_sync(ctx, enabler=pkg)
+        check_time_sync(ctx)
 
     if 'expect_hostname' in ctx and ctx.expect_hostname and ctx.expect_hostname != get_hostname():
         logger.warning('Adjusting hostname from %s -> %s...' % (get_hostname(), ctx.expect_hostname))
diff --git a/src/cephadm/cephadmlib/packagers.py b/src/cephadm/cephadmlib/packagers.py
index 9187b718411..df015771650 100644
--- a/src/cephadm/cephadmlib/packagers.py
+++ b/src/cephadm/cephadmlib/packagers.py
@@ -118,12 +118,6 @@ class Packager(object):
         else:
             return 'https://download.ceph.com/keys/autobuild.gpg', 'autobuild'
 
-    def enable_service(self, service: str) -> None:
-        """
-        Start and enable the service (typically using systemd).
-        """
-        call_throws(self.ctx, ['systemctl', 'enable', '--now', service])
-
 
 class Apt(Packager):
     DISTRO_NAMES = {
diff --git a/src/cephadm/cephadmlib/systemd.py b/src/cephadm/cephadmlib/systemd.py
index 1956957d457..1d2668a4803 100644
--- a/src/cephadm/cephadmlib/systemd.py
+++ b/src/cephadm/cephadmlib/systemd.py
@@ -2,11 +2,10 @@
 
 import logging
 
-from typing import Tuple, List, Optional
+from typing import Tuple, List
 
 from .context import CephadmContext
 from .call_wrappers import call, CallVerbosity
-from .packagers import Packager
 
 logger = logging.getLogger()
 
@@ -55,18 +54,15 @@ def check_unit(ctx: CephadmContext, unit_name: str) -> Tuple[bool, str, bool]:
     return (enabled, state, installed)
 
 
-def check_units(
-    ctx: CephadmContext, units: List[str], enabler: Optional[Packager] = None
-) -> bool:
+def check_units(ctx: CephadmContext, units: List[str]) -> bool:
     for u in units:
         (enabled, state, installed) = check_unit(ctx, u)
         if enabled and state == 'running':
             logger.info('Unit %s is enabled and running' % u)
             return True
-        if enabler is not None:
-            if installed:
-                logger.info('Enabling unit %s' % u)
-                enabler.enable_service(u)
+        if installed:
+            logger.info('Enabling unit %s' % u)
+            enable_service(ctx, u)
     return False
 
 
@@ -86,3 +82,14 @@ def terminate_service(ctx: CephadmContext, service_name: str) -> None:
         ['systemctl', 'disable', service_name],
         verbosity=CallVerbosity.DEBUG,
     )
+
+
+def enable_service(ctx: CephadmContext, service_name: str) -> None:
+    """
+    Start and enable the service (typically using systemd).
+    """
+    call(
+        ctx,
+        ['systemctl', 'enable', '--now', service_name],
+        verbosity=CallVerbosity.DEBUG,
+    )
diff --git a/src/cephadm/tests/test_util_funcs.py b/src/cephadm/tests/test_util_funcs.py
index 92872b196f3..d2aa39aca4b 100644
--- a/src/cephadm/tests/test_util_funcs.py
+++ b/src/cephadm/tests/test_util_funcs.py
@@ -351,6 +351,8 @@ def _mk_fake_call(enabled, active):
             if isinstance(active, Exception):
                 raise active
             return active
+        if "enable" in cmd:
+            return enabled
         raise ValueError("should not get here")
 
     return _fake_call
@@ -420,28 +422,8 @@ def test_check_unit(enabled_out, active_out, expected):
     assert (enabled, state, installed) == expected
 
 
-class FakeEnabler:
-    def __init__(self, should_be_called):
-        self._should_be_called = should_be_called
-        self._services = []
-
-    def enable_service(self, service):
-        self._services.append(service)
-
-    def check_expected(self):
-        if not self._should_be_called:
-            assert not self._services
-            return
-        # there are currently seven chron/chrony type services that
-        # cephadm looks for. Make sure it probed for each of them
-        # or more in case someone adds to the list.
-        assert len(self._services) >= 7
-        assert "chrony.service" in self._services
-        assert "ntp.service" in self._services
-
-
 @pytest.mark.parametrize(
-    "call_fn, enabler, expected",
+    "call_fn, expected",
     [
         # Test that time sync services are not enabled
         (
@@ -449,7 +431,6 @@ class FakeEnabler:
                 enabled=("", "", 1),
                 active=("", "", 1),
             ),
-            None,
             False,
         ),
         # Test that time sync service is enabled
@@ -458,7 +439,6 @@ class FakeEnabler:
                 enabled=("", "", 0),
                 active=("active", "", 0),
             ),
-            None,
             True,
         ),
         # Test that time sync is not enabled, and try to enable them.
@@ -470,7 +450,6 @@ class FakeEnabler:
                 enabled=("disabled", "", 1),
                 active=("", "", 1),
             ),
-            FakeEnabler(True),
             False,
         ),
         # Test that time sync is enabled, with an enabler passed which
@@ -480,22 +459,19 @@ class FakeEnabler:
                 enabled=("", "", 0),
                 active=("active", "", 0),
             ),
-            FakeEnabler(False),
             True,
         ),
     ],
 )
-def test_check_time_sync(call_fn, enabler, expected):
+def test_check_time_sync(call_fn, expected):
     """The check_time_sync call actually checks if a time synchronization service
     is enabled. It is also the only consumer of check_units.
     """
     with with_cephadm_ctx([]) as ctx:
         with mock.patch('cephadmlib.systemd.call') as _call:
             _call.side_effect = call_fn
-            result = _cephadm.check_time_sync(ctx, enabler=enabler)
+            result = _cephadm.check_time_sync(ctx)
             assert result == expected
-            if enabler is not None:
-                enabler.check_expected()
 
 
 @pytest.mark.parametrize(