From a81f9f158e32fc2d79b7cd242f5c1b21e927864a Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 4 Sep 2024 11:39:02 -0700 Subject: [PATCH 1/7] kdump: Introduce controller and model Introduce the KernelCrashDumpsController and KernelCrashDumpsModel, a non-interactive controller-model pair for setting the kernel-crash-dumps curthooks config. Practically, we introduce a new autoinstall section with the same format as the relevant curthooks config to selectively enable or disable kernel crash dumps on the target system. The default configuration is `{ "enabled": null }` which will let curtin perform dynamic enablement. --- subiquity/models/kernel_crash_dumps.py | 31 +++++++++ .../models/tests/test_kernel_crash_dumps.py | 33 +++++++++ .../server/controllers/kernel_crash_dumps.py | 49 +++++++++++++ .../tests/test_kernel_crash_dumps.py | 68 +++++++++++++++++++ 4 files changed, 181 insertions(+) create mode 100644 subiquity/models/kernel_crash_dumps.py create mode 100644 subiquity/models/tests/test_kernel_crash_dumps.py create mode 100644 subiquity/server/controllers/kernel_crash_dumps.py create mode 100644 subiquity/server/controllers/tests/test_kernel_crash_dumps.py diff --git a/subiquity/models/kernel_crash_dumps.py b/subiquity/models/kernel_crash_dumps.py new file mode 100644 index 000000000..99217ca5c --- /dev/null +++ b/subiquity/models/kernel_crash_dumps.py @@ -0,0 +1,31 @@ +# Copyright 2024 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, version 3. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import logging +from typing import Any + +log = logging.getLogger("subiquity.models.kernel_crash_dumps") + + +class KernelCrashDumpsModel: + # Set to True/False via autoinstall. Defaults to None to let curtin know + # to do dynamic enablement based on release, arch, etc. + enabled: bool | None = None + + def render(self) -> dict[str, Any]: + return { + "kernel-crash-dumps": { + "enabled": self.enabled, + }, + } diff --git a/subiquity/models/tests/test_kernel_crash_dumps.py b/subiquity/models/tests/test_kernel_crash_dumps.py new file mode 100644 index 000000000..376fe7868 --- /dev/null +++ b/subiquity/models/tests/test_kernel_crash_dumps.py @@ -0,0 +1,33 @@ +# Copyright 2024 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, version 3. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from subiquity.models.kernel_crash_dumps import KernelCrashDumpsModel +from subiquitycore.tests import SubiTestCase + + +class TestKernelCrashDumpsModel(SubiTestCase): + def setUp(self): + self.model = KernelCrashDumpsModel() + + def test_automatic_decision(self): + """Test the curtin config for curtin automatic enablement.""" + expected = {"kernel-crash-dumps": {"enabled": None}} + self.assertEqual(expected, self.model.render()) + + def test_render_formatting(self): + """Test the curtin config populates with correct formatting.""" + config = {} + self.model.enabled = config["enabled"] = True + expected = {"kernel-crash-dumps": config} + self.assertEqual(expected, self.model.render()) diff --git a/subiquity/server/controllers/kernel_crash_dumps.py b/subiquity/server/controllers/kernel_crash_dumps.py new file mode 100644 index 000000000..756020621 --- /dev/null +++ b/subiquity/server/controllers/kernel_crash_dumps.py @@ -0,0 +1,49 @@ +# Copyright 2024 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, version 3. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import logging +from typing import TypedDict + +from subiquity.server.controller import NonInteractiveController + +log = logging.getLogger("subiquity.server.controllers.kernel_crash_dumps") + + +class KernelCrashDumpsConfig(TypedDict, total=True): + enabled: bool | None + + +class KernelCrashDumpsController(NonInteractiveController): + model_name = "kernel_crash_dumps" + autoinstall_key = "kernel-crash-dumps" + autoinstall_schema = { + "type": "object", + "properties": { + "enabled": {"type": ["boolean", "null"]}, + }, + "required": ["enabled"], + "additionalProperties": False, + } + + def load_autoinstall_data(self, data: KernelCrashDumpsConfig | None) -> None: + if data is None: + return + self.model.enabled = data["enabled"] + + def make_autoinstall(self) -> dict[str, KernelCrashDumpsConfig]: + # Automatic determination implies no autoinstall + if self.model.enabled is None: + return {} + + return {"enabled": self.model.enabled} diff --git a/subiquity/server/controllers/tests/test_kernel_crash_dumps.py b/subiquity/server/controllers/tests/test_kernel_crash_dumps.py new file mode 100644 index 000000000..48baeda87 --- /dev/null +++ b/subiquity/server/controllers/tests/test_kernel_crash_dumps.py @@ -0,0 +1,68 @@ +# Copyright 2024 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, version 3. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import jsonschema +from jsonschema.validators import validator_for + +from subiquity.models.kernel_crash_dumps import KernelCrashDumpsModel +from subiquity.server.autoinstall import AutoinstallValidationError +from subiquity.server.controllers.kernel_crash_dumps import KernelCrashDumpsController +from subiquitycore.tests import SubiTestCase +from subiquitycore.tests.mocks import make_app +from subiquitycore.tests.parameterized import parameterized + + +class TestKernelCrashDumpsSchema(SubiTestCase): + def test_valid_schema(self): + """Test that the expected autoinstall JSON schema is valid""" + + JsonValidator: jsonschema.protocols.Validator = validator_for( + KernelCrashDumpsController.autoinstall_schema + ) + + JsonValidator.check_schema(KernelCrashDumpsController.autoinstall_schema) + + +class TestKernelCrashDumpsAutoinstall(SubiTestCase): + def setUp(self): + app = make_app() + self.controller = KernelCrashDumpsController(app) + self.controller.model = KernelCrashDumpsModel() + + @parameterized.expand( + ( + # (config, valid) + # Valid configs + ({"enabled": True}, True), + ({"enabled": False}, True), + ({"enabled": None}, True), + # Invalid configs + ({}, False), + ) + ) + def test_valid_configs(self, config, valid): + """Test autoinstall config validation behavior.""" + if valid: + self.controller.validate_autoinstall(config) + else: + with self.assertRaises(AutoinstallValidationError): + self.controller.validate_autoinstall(config) + + def test_make_autoinstall__default_empty(self): + self.assertEqual(self.controller.make_autoinstall(), {}) + + def test_make_autoinstall__non_default_format(self): + self.controller.model.enabled = False + expected = {"enabled": False} + self.assertEqual(self.controller.make_autoinstall(), expected) From f9d28f1105e06e84c4c46035e36ec582b0e7fd33 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 4 Sep 2024 11:40:20 -0700 Subject: [PATCH 2/7] kdump: enable controller and model The needed wiring to make subiquity use the new kernel crash dumps related controller and model. --- subiquity/models/subiquity.py | 2 ++ subiquity/server/controllers/__init__.py | 2 ++ subiquity/server/server.py | 2 ++ 3 files changed, 6 insertions(+) diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py index e5256fe9c..5d567ef04 100644 --- a/subiquity/models/subiquity.py +++ b/subiquity/models/subiquity.py @@ -52,6 +52,7 @@ def SchemaProblem(x, y): from .identity import IdentityModel from .integrity import IntegrityModel from .kernel import KernelModel +from .kernel_crash_dumps import KernelCrashDumpsModel from .keyboard import KeyboardModel from .locale import LocaleModel from .mirror import MirrorModel @@ -198,6 +199,7 @@ def __init__( self.identity = IdentityModel() self.integrity = IntegrityModel() self.kernel = KernelModel() + self.kernel_crash_dumps = KernelCrashDumpsModel() self.keyboard = KeyboardModel(self.root) self.locale = LocaleModel(self.chroot_prefix) self.mirror = MirrorModel() diff --git a/subiquity/server/controllers/__init__.py b/subiquity/server/controllers/__init__.py index 8508d9bc7..f7c481a18 100644 --- a/subiquity/server/controllers/__init__.py +++ b/subiquity/server/controllers/__init__.py @@ -23,6 +23,7 @@ from .install import InstallController from .integrity import IntegrityController from .kernel import KernelController +from .kernel_crash_dumps import KernelCrashDumpsController from .keyboard import KeyboardController from .locale import LocaleController from .mirror import MirrorController @@ -54,6 +55,7 @@ "IntegrityController", "InstallController", "KernelController", + "KernelCrashDumpsController", "KeyboardController", "LateController", "LocaleController", diff --git a/subiquity/server/server.py b/subiquity/server/server.py index 0e67fb8ff..8ac8b0058 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -200,6 +200,7 @@ def get_installer_password_from_cloudinit_log(): "debconf_selections", "filesystem", "kernel", + "kernel_crash_dumps", "keyboard", "source", }, @@ -258,6 +259,7 @@ class SubiquityServer(Application): "Locale", "Refresh", "Kernel", + "KernelCrashDumps", "Integrity", "Keyboard", "Zdev", From 3c90bb92410cfd72666ee161cc863f028207cc8c Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 4 Sep 2024 11:41:50 -0700 Subject: [PATCH 3/7] ci: update autoinstall-schema with kdump schema --- autoinstall-schema.json | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/autoinstall-schema.json b/autoinstall-schema.json index e16195ac3..c9febb68a 100644 --- a/autoinstall-schema.json +++ b/autoinstall-schema.json @@ -103,6 +103,21 @@ } ] }, + "kernel-crash-dumps": { + "type": "object", + "properties": { + "enabled": { + "type": [ + "boolean", + "null" + ] + } + }, + "required": [ + "enabled" + ], + "additionalProperties": false + }, "keyboard": { "type": "object", "properties": { From 0f47bb6a27dcb7597eacf8a721898e429afb1145 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 4 Sep 2024 12:16:35 -0700 Subject: [PATCH 4/7] ci: allow null compare in check-yaml-fields script Allow check for `None` type in check-yaml-fields.py script. It wasn't clear from the original commit - 2ddde49a4 - if we purposefully didn't want to compare `null` in the script, or it was just a typo. This will now let us assert a field should be `null` instead of some other value, and fail properly when it is not. --- scripts/check-yaml-fields.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/check-yaml-fields.py b/scripts/check-yaml-fields.py index 821b53c29..73e902fc9 100644 --- a/scripts/check-yaml-fields.py +++ b/scripts/check-yaml-fields.py @@ -27,8 +27,7 @@ def main(): v = v[index] if expected is None: print(v) - else: - assert v == expected, "{!r} != {!r}".format(v, expected) + assert v == expected, "{!r} != {!r}".format(v, expected) main() From 1a0f51169d51314a188da2f97087b84340ca5840 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 4 Sep 2024 12:17:44 -0700 Subject: [PATCH 5/7] ci: add kernel-crash-dumps checks and examples - Add an example kernel-crash-dumps section to the most-options.yaml example file and check the curthook is updated correctly. - Add a check in the autoinstall-simple case that asserts the default value for the kernel-crash-dumps curthook config. --- examples/autoinstall/most-options.yaml | 2 ++ scripts/runtests.sh | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/examples/autoinstall/most-options.yaml b/examples/autoinstall/most-options.yaml index 5fa2055b9..34b0216c7 100644 --- a/examples/autoinstall/most-options.yaml +++ b/examples/autoinstall/most-options.yaml @@ -69,3 +69,5 @@ storage: - {type: format, fstype: ext4, volume: raid-system, preserve: false, id: format-system} - {type: mount, device: format-system, path: /, id: mount-system} - {type: mount, device: format-boot, path: /boot, id: mount-boot, options: 'errors=remount-ro'} +kernel-crash-dumps: + enabled: false diff --git a/scripts/runtests.sh b/scripts/runtests.sh index f20a95d5a..da282be31 100755 --- a/scripts/runtests.sh +++ b/scripts/runtests.sh @@ -171,6 +171,8 @@ python3 scripts/check-yaml-fields.py <(python3 scripts/check-yaml-fields.py $tmp timezone='"Pacific/Guam"' \ ubuntu_advantage.token='"C1NWcZTHLteJXGVMM6YhvHDpGrhyy7"' \ 'snap.commands=[snap install --channel=3.2/stable etcd]' +python3 scripts/check-yaml-fields.py "$tmpdir"/var/log/installer/curtin-install/subiquity-curthooks.conf \ + kernel-crash-dumps.enabled=false grep -q 'finish: subiquity/Install/install/postinstall/install_package1: SUCCESS: installing package1' \ $tmpdir/subiquity-server-debug.log grep -q 'finish: subiquity/Install/install/postinstall/install_package2: SUCCESS: installing package2' \ @@ -193,6 +195,8 @@ validate python3 scripts/check-yaml-fields.py "$tmpdir"/var/log/installer/autoinstall-user-data \ 'autoinstall.source.id="ubuntu-server-minimal"' grep -q 'finish: subiquity/Install/install/postinstall/run_unattended_upgrades: SUCCESS: downloading and installing security updates' $tmpdir/subiquity-server-debug.log +python3 scripts/check-yaml-fields.py "$tmpdir"/var/log/installer/curtin-install/subiquity-curthooks.conf \ + kernel-crash-dumps.enabled=null clean testname=autoinstall-hybrid From be8d1b15c71d38cfa9f92474167c15050d116c64 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 4 Sep 2024 11:56:19 -0700 Subject: [PATCH 6/7] reference: add kernel-crash-dumps section --- doc/.custom_wordlist.txt | 1 + doc/reference/autoinstall-reference.rst | 49 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/doc/.custom_wordlist.txt b/doc/.custom_wordlist.txt index 9a01c2840..d95e6f6d3 100644 --- a/doc/.custom_wordlist.txt +++ b/doc/.custom_wordlist.txt @@ -55,6 +55,7 @@ debconf debian dir el +enablement flavor geoip geolocation diff --git a/doc/reference/autoinstall-reference.rst b/doc/reference/autoinstall-reference.rst index cadacef95..917528ee0 100644 --- a/doc/reference/autoinstall-reference.rst +++ b/doc/reference/autoinstall-reference.rst @@ -1190,6 +1190,55 @@ Example: kernel: flavour: hwe +.. _ai-kernel-crash-dumps: + +kernel-crash-dumps +~~~~~~~~~~~~~~~~~~ + +* **type:** mapping, see below +* **default:** see below +* **can be interactive:** no + +Toggle kernel crash dumps enablement. + +In 24.10 and later, the default configuration will result in dynamic enablement of kernel crash dumps on the installed system using the ``kdump-tools`` package. On amd64, arm64, and s390x systems, if the system is detected to meet the minimum requirements for kernel crash dumps then they will be enabled. Otherwise, they will be disabled. More details about the minimum system requirements can be found in the following document (doesn't exist yet). + +In pre-24.10, the default configuration will result in kernel crash dumps being disabled. + +Default configuration: + +.. code-block:: yaml + + autoinstall: + # In 24.10 and later, allow kernel crash dumps to be enabled dynamically. + # In pre-24.10, kernel crash dumps will be disabled. + kernel-crash-dumps: + enabled: null + +enabled +^^^^^^^ + +* **type:** boolean or null +* **default:** ``null`` + +Specify a boolean value to enable or disable kernel crash dumps. Set to ``null`` (default) to allow dynamic enablement. + +If kernel crash dumps are to be disabled, whether determined dynamically or manually requested, the ``kdump-tools`` package will not be uninstalled but will be configured to ensure it is inactive in the target system. + +Examples: + +.. code-block:: yaml + + autoinstall: + # Enable kernel crash dumps. + kernel-crash-dumps: + enabled: true + + autoinstall: + # Disable kernel crash dumps. + kernel-crash-dumps: + enabled: false + .. _ai-timezone: timezone From d3429d759185a2f356f0e08545b052cb37549fa0 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Fri, 13 Sep 2024 17:44:14 -0700 Subject: [PATCH 7/7] snapcraft: rev curtin for kernel crash dumps --- snapcraft.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snapcraft.yaml b/snapcraft.yaml index e210c8332..c3acef378 100644 --- a/snapcraft.yaml +++ b/snapcraft.yaml @@ -80,7 +80,7 @@ parts: source: https://git.launchpad.net/curtin source-type: git - source-commit: 364b719e189708255ff63b95078bc4f6bcf70540 + source-commit: 40cae5c60fa9f4c495c7f61cde28862175f93ce2 override-pull: | craftctl default