From 77b3080808ffb90f6f0aebfeb3d495c9a5dcae7a Mon Sep 17 00:00:00 2001
From: Zack Cerza <zack@redhat.com>
Date: Mon, 13 Jun 2022 11:12:23 -0600
Subject: [PATCH] cephadm: Make sysctl conf writing idempotent

By checking the system's actual settings against our desired settings
before writing out the config file.

Fixes: https://tracker.ceph.com/issues/56026

Signed-off-by: Zack Cerza <zack@redhat.com>
---
 src/cephadm/cephadm               | 32 ++++++++++++++++++++++-
 src/cephadm/tests/test_cephadm.py | 42 ++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/cephadm/cephadm b/src/cephadm/cephadm
index a238087a00edd..f6a625d1db599 100755
--- a/src/cephadm/cephadm
+++ b/src/cephadm/cephadm
@@ -3532,7 +3532,7 @@ def install_sysctl(ctx: CephadmContext, fsid: str, daemon_type: str) -> None:
             f.write('\n'.join(lines))
 
     conf = Path(ctx.sysctl_dir).joinpath(f'90-ceph-{fsid}-{daemon_type}.conf')
-    lines: Optional[List] = None
+    lines: List = []
 
     if daemon_type == 'osd':
         lines = OSD.get_sysctl_settings()
@@ -3540,6 +3540,7 @@ def install_sysctl(ctx: CephadmContext, fsid: str, daemon_type: str) -> None:
         lines = HAproxy.get_sysctl_settings()
     elif daemon_type == 'keepalived':
         lines = Keepalived.get_sysctl_settings()
+    lines = filter_sysctl_settings(ctx, lines)
 
     # apply the sysctl settings
     if lines:
@@ -3548,6 +3549,35 @@ def install_sysctl(ctx: CephadmContext, fsid: str, daemon_type: str) -> None:
         call_throws(ctx, ['sysctl', '--system'])
 
 
+def sysctl_get(ctx: CephadmContext, variable: str) -> Union[str, None]:
+    """
+    Read a sysctl setting by executing 'sysctl -b {variable}'
+    """
+    out, err, code = call(ctx, ['sysctl', '-b', variable])
+    return out or None
+
+
+def filter_sysctl_settings(ctx: CephadmContext, lines: List[str]) -> List[str]:
+    """
+    Given a list of sysctl settings, examine the system's current configuration
+    and return those which are not currently set as described.
+    """
+    def test_setting(desired_line: str) -> bool:
+        # Remove any comments
+        comment_start = desired_line.find('#')
+        if comment_start != -1:
+            desired_line = desired_line[:comment_start]
+        desired_line = desired_line.strip()
+        if not desired_line or desired_line.isspace():
+            return False
+        setting, desired_value = map(lambda s: s.strip(), desired_line.split('='))
+        if not setting or not desired_value:
+            return False
+        actual_value = sysctl_get(ctx, setting)
+        return desired_value != actual_value
+    return list(filter(test_setting, lines))
+
+
 def migrate_sysctl_dir(ctx: CephadmContext, fsid: str) -> None:
     """
     Cephadm once used '/usr/lib/sysctl.d' for storing sysctl configuration.
diff --git a/src/cephadm/tests/test_cephadm.py b/src/cephadm/tests/test_cephadm.py
index 604d023449254..58701e43db6ff 100644
--- a/src/cephadm/tests/test_cephadm.py
+++ b/src/cephadm/tests/test_cephadm.py
@@ -2323,4 +2323,44 @@ class TestNetworkValidation:
 
         # invalid IPv6 and valid subnets list
         with pytest.raises(Exception):
-            rc = cd.ip_in_sublets('fe80:2030:31:24', 'fe80::/64')
\ No newline at end of file
+            rc = cd.ip_in_sublets('fe80:2030:31:24', 'fe80::/64')
+
+
+class TestSysctl:
+    @mock.patch('cephadm.sysctl_get')
+    def test_filter_sysctl_settings(self, sysctl_get):
+        ctx = cd.CephadmContext()
+        input = [
+            # comment-only lines should be ignored
+            "# just a comment",
+            # As should whitespace-only lines",
+            "   \t ",
+            "   =  \t  ",
+            # inline comments are stripped when querying
+            "something = value # inline comment",
+            "fs.aio-max-nr = 1048576",
+            "kernel.pid_max = 4194304",
+            "vm.lowmem_reserve_ratio = 256\t256\t32\t0\t0",
+            "  vm.max_map_count       =            65530    ",
+            "  vm.max_map_count       =            65530    ",
+        ]
+        sysctl_get.side_effect = [
+            "value",
+            "1",
+            "4194304",
+            "256\t256\t32\t0\t0",
+            "65530",
+            "something else",
+        ]
+        result = cd.filter_sysctl_settings(ctx, input)
+        assert len(sysctl_get.call_args_list) == 6
+        assert sysctl_get.call_args_list[0].args[1] == "something"
+        assert sysctl_get.call_args_list[1].args[1] == "fs.aio-max-nr"
+        assert sysctl_get.call_args_list[2].args[1] == "kernel.pid_max"
+        assert sysctl_get.call_args_list[3].args[1] == "vm.lowmem_reserve_ratio"
+        assert sysctl_get.call_args_list[4].args[1] == "vm.max_map_count"
+        assert sysctl_get.call_args_list[5].args[1] == "vm.max_map_count"
+        assert result == [
+            "fs.aio-max-nr = 1048576",
+            "  vm.max_map_count       =            65530    ",
+        ]
-- 
2.47.3