curtin-dev team mailing list archive

[Dan Bungert <mp+473954@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:23.1-kdump into curtin:release/23.1.

Commit message:
Introduce the kernel-crash-dumps builtin hook

Introduce a new hook for builtin_curthooks to configure kernel crash
dumps on the target system. The configuration key is
`kernel-crash-dumps` and adds the following new section to the
curthooks configuration:

    kernel-crash-dumps:
        enabled: bool | None

By default, `enabled` is `None` will cause kernel crash dumps
to be dynamically enabled on the target system if the enablement
script provided by kdump-tools is found in:

/usr/share/kdump-tools/kdump_set_default

The enablement script will inspect the system and either enable or
disable kernel crash dumps. Users can also specify `True` or `False`
to unconditionally enable or disable kernel crash dumps, respectively.

(cherry picked from commit 40cae5c60fa9f4c495c7f61cde28862175f93ce2)



Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/473954
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:23.1-kdump into curtin:release/23.1.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 6fb5f3d..da4eda8 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -3,6 +3,7 @@
 import copy
 import glob
 import os
+import pathlib
 import platform
 import re
 import sys
@@ -21,6 +22,7 @@ from curtin import paths
 from curtin import swap
 from curtin import util
 from curtin import version as curtin_version
+from curtin import kernel_crash_dumps
 from curtin.block import deps as bdeps
 from curtin.distro import DISTROS
 from curtin.net import deps as ndeps
@@ -1480,6 +1482,29 @@ def configure_mdadm(cfg, state_etcd, target, osfamily=DISTROS.debian):
                 data=None, target=target)
 
 
+def configure_kernel_crash_dumps(cfg, target: pathlib.Path) -> None:
+    """Configure kernel crash dumps on target system.
+
+    kernel-crash-dumps:
+        enabled: bool | None
+
+    If `enabled` is `None` then kernel crash dumps will be dynamically enabled
+    if both the kdump-tools package is installed on the target system and it
+    also provides the expected enablement script (Starting in 24.10).
+    """
+    kdump_cfg = cfg.get("kernel-crash-dumps", {})
+
+    enabled: bool = kdump_cfg.get("enabled")
+    automatic = enabled is None
+
+    if automatic:
+        kernel_crash_dumps.automatic_detect(target)
+    elif enabled:
+        kernel_crash_dumps.manual_enable(target)
+    else:
+        kernel_crash_dumps.manual_disable(target)
+
+
 def handle_cloudconfig(cfg, base_dir=None):
     """write cloud-init configuration files into base_dir.
 
@@ -1851,6 +1876,15 @@ def builtin_curthooks(cfg, target, state):
     if os.path.isdir(udev_rules_d):
         copy_dname_rules(udev_rules_d, target)
 
+    # Setup kernel crash dumps
+    with events.ReportEventStack(
+        name=stack_prefix + "/configuring-kernel-crash-dumps",
+        reporting_enabled=True,
+        level="INFO",
+        description="configuring kernel crash dumps settings",
+    ):
+        configure_kernel_crash_dumps(cfg, pathlib.Path(target))
+
     with events.ReportEventStack(
             name=stack_prefix + '/updating-initramfs-configuration',
             reporting_enabled=True, level="INFO",
diff --git a/curtin/kernel_crash_dumps.py b/curtin/kernel_crash_dumps.py
new file mode 100644
index 0000000..9c7ef4d
--- /dev/null
+++ b/curtin/kernel_crash_dumps.py
@@ -0,0 +1,70 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from pathlib import Path
+
+from curtin import distro
+from curtin.log import LOG
+from curtin.util import ChrootableTarget, ProcessExecutionError
+
+ENABLEMENT_SCRIPT = "/usr/share/kdump-tools/kdump_set_default"
+
+
+def ensure_kdump_installed(target: Path) -> None:
+    """Ensure kdump-tools installed on target system and install it if not.
+
+    kdump-tools is theoretically part of the base-install in >=24.10
+    but we may need to dynamically install it if manual enablement is
+    requested.
+    """
+    if "kdump-tools" not in distro.get_installed_packages(str(target)):
+        distro.install_packages(["kdump-tools"], target=str(target))
+
+
+def detection_script_available(target: Path) -> bool:
+    """Detect existence of the enablement script.
+
+    Enablement script will only be found on targets where kdump-tools is
+    pre-installed and it's a version which contains the script.
+    """
+    path = target / ENABLEMENT_SCRIPT[1:]
+    if path.exists():
+        LOG.debug("kernel-crash-dumps enablement script found.")
+        return True
+    else:
+        LOG.debug("kernel-crash-dumps enablement script missing.")
+        return False
+
+
+def manual_enable(target: Path) -> None:
+    """Manually enable kernel crash dumps with kdump-tools on target."""
+    ensure_kdump_installed(target)
+    try:
+        with ChrootableTarget(str(target)) as in_target:
+            in_target.subp([ENABLEMENT_SCRIPT, "true"])
+    except ProcessExecutionError as exc:
+        # Likely the enablement script hasn't been SRU'd
+        # Let's not block the install on this.
+        LOG.warning(
+            "Unable to run kernel-crash-dumps enablement script: %s",
+            exc,
+        )
+
+
+def manual_disable(target: Path) -> None:
+    """Manually disable kernel crash dumps with kdump-tools on target."""
+    if "kdump-tools" in distro.get_installed_packages(str(target)):
+        with ChrootableTarget(str(target)) as in_target:
+            in_target.subp([ENABLEMENT_SCRIPT, "false"])
+
+
+def automatic_detect(target: Path) -> None:
+    """Perform conditional enablement with kdump-tools on target.
+
+    Uses the enablement script provided by kdump-tools to detect
+    system criteria and either enable or disable kernel crash dumps
+    on the target system. The script is not run if it's not found.
+    """
+    if detection_script_available(target):
+        LOG.debug("Running conditional enablement script...")
+        with ChrootableTarget(str(target)) as in_target:
+            in_target.subp([ENABLEMENT_SCRIPT])
diff --git a/doc/topics/config.rst b/doc/topics/config.rst
index c870446..35ba863 100644
--- a/doc/topics/config.rst
+++ b/doc/topics/config.rst
@@ -21,6 +21,7 @@ Curtin's top level config keys are as follows:
 - http_proxy (``http_proxy``)
 - install (``install``)
 - kernel (``kernel``)
+- kernel-crash-dumps (``kernel-crash-dumps``)
 - kexec (``kexec``)
 - multipath (``multipath``)
 - network (``network``)
@@ -439,6 +440,51 @@ Specify the exact package to install in the target OS.
       - xenial:
         - 4.4.0: -my-custom-kernel    
 
+kernel-crash-dumps
+~~~~~~~~~~~~~~~~~~
+Configure how Curtin will configure kernel crash dumps in the target system
+using the ``kdump-tools`` package. If ``kernel-crash-dumps`` is not configured,
+Curtin will attempt to use ``kdump-tools`` to enable kernel crash dumps on the
+target machine if certain criteria are met. This requires ``kdump-tools`` to be
+installed in the target system before the hook is ran, which will be run
+during execution of the hook to determine if the system meets the minimum
+requirements based on criteria such as architecture, number of cores,
+disk space, and available memory. The hook will not install ``kdump-tools``
+by default.
+
+**enabled**: *<boolean or None: default None>*
+
+Enable, disable, or allow ``kdump-tools`` to detect whether kernel crash
+dumps should be enabled or disabled on the target system for values of
+``true``, ``false``, and ``None``, respectively.
+
+If ``enabled`` is set to ``true``, Curtin will install the ``kdump-tools``
+package if it is not installed already and then enable kernel crash dumps in
+the target system unconditionally.
+
+If ``enabled`` is set to ``false``, Curtin will ensure kernel crash dumps are
+disabled in the target system but it **will not uninstall the package**.
+
+If ``enabled`` is set to ``null``, Curtin will check that ``kdump-tools``
+is installed in the target system and provides the automatic detection
+capability, and if so, will invoke ``kdump-tools`` to detect if the system
+meets the minimum criteria and enable or disable kernel crash dumps
+accordingly.
+
+**Examples**::
+
+  # Default: dynamically enable kernel crash dumps if kdump-tools is installed.
+  # on the target system.
+  kernel-crash-dumps:
+    enabled: null
+
+  # Unconditionally enable kernel-crash-dumps.
+  kernel-crash-dumps:
+    enabled: true
+
+  # Unconditionally disable kernel-crash-dumps.
+  kernel-crash-dumps:
+    enabled: false
 
 kexec
 ~~~~~
diff --git a/tests/unittests/test_kernel_crash_dumps.py b/tests/unittests/test_kernel_crash_dumps.py
new file mode 100644
index 0000000..f1c69e9
--- /dev/null
+++ b/tests/unittests/test_kernel_crash_dumps.py
@@ -0,0 +1,174 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+from parameterized import parameterized
+
+from curtin.commands.curthooks import configure_kernel_crash_dumps
+from curtin.kernel_crash_dumps import (ENABLEMENT_SCRIPT, automatic_detect,
+                                       detection_script_available,
+                                       ensure_kdump_installed, manual_disable,
+                                       manual_enable)
+from tests.unittests.helpers import CiTestCase
+
+
+@patch("curtin.kernel_crash_dumps.manual_disable")
+@patch("curtin.kernel_crash_dumps.manual_enable")
+@patch("curtin.kernel_crash_dumps.automatic_detect")
+class TestKernelCrashDumpsCurthook(CiTestCase):
+
+    @parameterized.expand(
+        (
+            ({"kernel-crash-dumps": {}},),
+            ({"kernel-crash-dumps": {"enabled": None}},),
+        )
+    )
+    def test_config__automatic(
+        self,
+        auto_mock,
+        enable_mock,
+        disable_mock,
+        config,
+    ):
+        """Test expected automatic configs."""
+
+        configure_kernel_crash_dumps(config, "/target")
+        auto_mock.assert_called_once()
+        enable_mock.assert_not_called()
+        disable_mock.assert_not_called()
+
+    def test_config__manual_enable(
+        self,
+        auto_mock,
+        enable_mock,
+        disable_mock,
+    ):
+        """Test expected automatic configs."""
+        config = {"kernel-crash-dumps": {"enabled": True}}
+        configure_kernel_crash_dumps(config, "/target")
+        auto_mock.assert_not_called()
+        enable_mock.assert_called_once()
+        disable_mock.assert_not_called()
+
+    def test_config__manual_disable(
+        self,
+        auto_mock,
+        enable_mock,
+        disable_mock,
+    ):
+        """Test expected automatic configs."""
+        config = {"kernel-crash-dumps": {"enabled": False}}
+        configure_kernel_crash_dumps(config, "/target")
+        auto_mock.assert_not_called()
+        enable_mock.assert_not_called()
+        disable_mock.assert_called_once()
+
+
+class TestKernelCrashDumpsUtilities(CiTestCase):
+
+    @parameterized.expand(
+        (
+            (True, True),
+            (False, False),
+        )
+    )
+    def test_detection_script_available(self, preinstalled, expected):
+        """Test detection_script_available checks for script path."""
+
+        with patch(
+            "curtin.kernel_crash_dumps.Path.exists",
+            return_value=preinstalled,
+        ):
+            self.assertEqual(detection_script_available(Path("")), expected)
+
+    @parameterized.expand(
+        (
+            (True,),
+            (False,),
+        )
+    )
+    def test_ensure_kdump_installed(self, preinstalled):
+        """Test detection of preinstall and install of kdump-tools."""
+
+        target = Path("/target")
+        with (
+            patch(
+                "curtin.distro.get_installed_packages",
+                return_value=["kdump-tools" if preinstalled else ""],
+            )
+        ):
+            with patch("curtin.distro.install_packages") as do_install:
+                ensure_kdump_installed(target)
+
+        if preinstalled:
+            do_install.assert_not_called()
+        else:
+            do_install.assert_called_with(["kdump-tools"], target=str(target))
+
+    def test_manual_enable(self):
+        """Test manual enablement logic."""
+        target = Path("/target")
+        with patch(
+            "curtin.kernel_crash_dumps.ensure_kdump_installed",
+        ) as ensure_mock:
+            with patch(
+                "curtin.kernel_crash_dumps.ChrootableTarget",
+                new=MagicMock(),
+            ) as chroot_mock:
+                manual_enable(target)
+        ensure_mock.assert_called_once()
+        subp_mock = chroot_mock.return_value.__enter__.return_value.subp
+        subp_mock.assert_called_with(
+            [ENABLEMENT_SCRIPT, "true"],
+        )
+
+    @parameterized.expand(
+        (
+            (True,),
+            (False,),
+        )
+    )
+    def test_manual_disable(self, preinstalled):
+        """Test manual disable logic."""
+        target = Path("/target")
+        with patch(
+            "curtin.distro.get_installed_packages",
+            return_value=["kdump-tools" if preinstalled else ""],
+        ):
+            with patch(
+                "curtin.kernel_crash_dumps.ChrootableTarget",
+                new=MagicMock(),
+            ) as chroot_mock:
+                manual_disable(target)
+
+        subp_mock = chroot_mock.return_value.__enter__.return_value.subp
+        if preinstalled:
+            subp_mock.assert_called_with([ENABLEMENT_SCRIPT, "false"])
+        else:
+            subp_mock.assert_not_called()
+
+    @parameterized.expand(
+        (
+            (True,),
+            (False,),
+        )
+    )
+    def test_automatic_detect(self, wants_enablement):
+        """Test automatic enablement logic."""
+        target = Path("/target")
+        with patch(
+            "curtin.kernel_crash_dumps.detection_script_available",
+            return_value=wants_enablement,
+        ):
+            with patch(
+                "curtin.kernel_crash_dumps.ChrootableTarget",
+                new=MagicMock(),
+            ) as chroot_mock:
+                automatic_detect(target)
+
+        subp_mock = chroot_mock.return_value.__enter__.return_value.subp
+        if wants_enablement:
+            subp_mock.assert_called_with([ENABLEMENT_SCRIPT])
+        else:
+            subp_mock.assert_not_called()