Repository: osbuild/osbuild
Pull request #1586 (2024-02-14 10:48:57)

stages/dracut: Add functionality to build initoverlayfs with dracut
As an alternative to just initramfs. Upstream initoverlayfs project:

https://github.com/containers/initoverlayfs

achilleas-k: Nice and clean but I'd love to see a test for this.  Looks good, but I have no idea if it works :laughing: 

ericcurtin: > Nice and clean but I'd love to see a test for this. Looks good, but I have no idea if it works 😆

We will be testing thing as part of our sample-images CI builds once this is made available:

https://gitlab.com/CentOS/automotive/sample-images

mvo5: Fwiw, I would love to see a small test for this functionality, I created 
[0001-stages-dracut-add-small-unittest-for-initoverlayfs.patch.txt](https://github.com/osbuild/osbuild/files/14328863/0001-stages-dracut-add-small-unittest-for-initoverlayfs.patch.txt)

that validates that the option is running the expected thing. Ideally also a test in `test/run/test_stages.py:test_dracut`, I will look into this.
mvo5: I wrote a small "stage" test for this now that does an end-to-end type test (it will need tweaks but is a useful starting point) for the feature (see attached patch) 

[0002-test-add-stage-integration-test-for-initoverlayfs-fe.patch.txt](https://github.com/osbuild/osbuild/files/14329239/0002-test-add-stage-integration-test-for-initoverlayfs-fe.patch.txt)

Unfortunately this fails during the creation of the image with an error that `--no-hostonly`  is unknown:
```
org.osbuild.dracut: e30a0aaa9351c35b3a70d8529fe33de68265af135daef250517b34296953684c {
  "kernel": [
    "6.6.4-200.fc39.x86_64"
  ],
  "initoverlayfs": true
}
Building initramfs for 6.6.4-200.fc39.x86_64
+ [[ 8 -gt 0 ]]
+ case $1 in
+ echo 'Unknown option --no-hostonly'
Unknown option --no-hostonly
+ exit 1
Traceback (most recent call last):
  File "/run/osbuild/bin/org.osbuild.dracut", line 228, in <module>
    r = main(args["tree"], args["options"])
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/run/osbuild/bin/org.osbuild.dracut", line 217, in main
    subprocess.run(["/usr/sbin/chroot", tree, bin,
  File "/usr/lib64/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/usr/sbin/chroot', '/run/osbuild/tree', '/usr/bin/initoverlayfs-install', '--no-hostonly', '--kver', '6.6.4-200.fc39.x86_64', '--force', '-v', '--show-modules', '--no-early-microcode', '--reproducible']' returned non-zero exit status 1.

⏱  Duration: 0s

-- END ---------------------------------
FAILED

```
so the way our dracut stage drives dracut seems to not quite align with the way initoverlayfs-install expects it?
ericcurtin: > I wrote a small "stage" test for this now that does an end-to-end type test (it will need tweaks but is a useful starting point) for the feature (see attached patch)
> 
> [0002-test-add-stage-integration-test-for-initoverlayfs-fe.patch.txt](https://github.com/osbuild/osbuild/files/14329239/0002-test-add-stage-integration-test-for-initoverlayfs-fe.patch.txt)
> 
> Unfortunately this fails during the creation of the image with an error that `--no-hostonly` is unknown:
> 
> ```
> org.osbuild.dracut: e30a0aaa9351c35b3a70d8529fe33de68265af135daef250517b34296953684c {
>   "kernel": [
>     "6.6.4-200.fc39.x86_64"
>   ],
>   "initoverlayfs": true
> }
> Building initramfs for 6.6.4-200.fc39.x86_64
> + [[ 8 -gt 0 ]]
> + case $1 in
> + echo 'Unknown option --no-hostonly'
> Unknown option --no-hostonly
> + exit 1
> Traceback (most recent call last):
>   File "/run/osbuild/bin/org.osbuild.dracut", line 228, in <module>
>     r = main(args["tree"], args["options"])
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/run/osbuild/bin/org.osbuild.dracut", line 217, in main
>     subprocess.run(["/usr/sbin/chroot", tree, bin,
>   File "/usr/lib64/python3.12/subprocess.py", line 571, in run
>     raise CalledProcessError(retcode, process.args,
> subprocess.CalledProcessError: Command '['/usr/sbin/chroot', '/run/osbuild/tree', '/usr/bin/initoverlayfs-install', '--no-hostonly', '--kver', '6.6.4-200.fc39.x86_64', '--force', '-v', '--show-modules', '--no-early-microcode', '--reproducible']' returned non-zero exit status 1.
> 
> ⏱  Duration: 0s
> 
> -- END ---------------------------------
> FAILED
> ```
> 
> so the way our dracut stage drives dracut seems to not quite align with the way initoverlayfs-install expects it?

This package, the Fedora 39 rpm, is not up to date with upstream, it's expected to fail.

ericcurtin: We are probably not going to backport either. I would suggest merging this as it's blocking other MRs and add the test at a later point once the corresponding changes have propagated to Fedora.

This change will not break any existing osbuild scripts as initoverlayfs defaults to false.

ericcurtin: > > This package, the Fedora 39 rpm, is not up to date with upstream
> 
> Does that mean it's already available in F39? We can just update the repos used in the test to newer snapshots (we made new ones on Friday).

Integration of initoverlayfs with Fedora will probably be a Fedora 41 kind of thing... But we am currently using it with osbuild via the Automotive SIG osbuild manifests...

ondrejbudai: @mvo5 isn't the test doomed to fail given that `--no-hostonly` is not yet available in Fedora? Or am I missing something?
ericcurtin: Yeah it would just fail, I don't see the harm in this MR, the option is defaulted to False, so people who are gonna use this have to explicitly set it to True (the people with the correct rpms)...
mvo5: > @mvo5 isn't the test doomed to fail given that `--no-hostonly` is not yet available in Fedora? Or am I missing something?

Yeah, I was just wanted to get confirmation from someone more experienced that we are okay with merging this even though we don't have a full end-to-end test (yet) :)
ondrejbudai: I think I'm fine with that given that this is a new technology, not yet available downstream. We can of course pull the package from somewhere else, but that feels expensive... I think that not blocking this initiative is more important to me. I agree though that this test SHOULD be added when the package lands in a "mainstream" repository.
ericcurtin: I changed a variable name to initfs_bin , think pylint didn't like the bin variable name

Patch:
diff --git a/README.md b/README.md
index 42862848..1776abd7 100644
--- a/README.md
+++ b/README.md
@@ -29,7 +29,7 @@ of the pipeline description, and more.
 
 ### Contributing
 
-Please refer to the [developer guide](https://www.osbuild.org/guides/developer-guide/index.html) to learn about our workflow, code style and more.
+Please refer to the [developer guide](https://osbuild.org/docs/developer-guide/index) to learn about our workflow, code style and more.
 
 ## Requirements
 
diff --git a/stages/org.osbuild.dracut b/stages/org.osbuild.dracut
index a45b097e..475970a0 100755
--- a/stages/org.osbuild.dracut
+++ b/stages/org.osbuild.dracut
@@ -120,6 +120,11 @@ SCHEMA = """
     "description": "Create reproducible images.",
     "type": "boolean"
   },
+  "initoverlayfs": {
+    "description": "Use initoverlayfs rather than initramfs, requires initoverlayfs rpm to be installed",
+    "type": "boolean",
+    "default": false
+  },
   "extra": {
     "description": "Extra arguments to directly pass to dracut",
     "type": "array",
@@ -153,6 +158,7 @@ def main(tree, options):
     install = options.get("install", [])
     early_microcode = options.get("early_microcode", False)
     reproducible = options.get("reproducible", True)
+    initoverlayfs = options.get("initoverlayfs", False)
     extra = options.get("extra", [])
 
     # initrds may have already been created, force the recreation
@@ -204,8 +210,11 @@ def main(tree, options):
     for kver in kernels:
         print(f"Building initramfs for {kver}", file=sys.stderr)
 
-        subprocess.run(["/usr/sbin/chroot", tree,
-                        "/usr/bin/dracut",
+        initfs_bin = "/usr/bin/dracut"
+        if initoverlayfs:
+            initfs_bin = "/usr/bin/initoverlayfs-install"
+
+        subprocess.run(["/usr/sbin/chroot", tree, initfs_bin,
                         "--no-hostonly",
                         "--kver", kver]
                        + opts,
diff --git a/stages/org.osbuild.grub2 b/stages/org.osbuild.grub2
index 72e6e1a0..5fb784fe 100755
--- a/stages/org.osbuild.grub2
+++ b/stages/org.osbuild.grub2
@@ -202,11 +202,24 @@ SCHEMA = """
         "description": "Default boot entry",
         "type": "string"
       },
+      "disable_recovery": {
+        "type": "boolean"
+      },
+      "disable_submenu": {
+        "type": "boolean"
+      },
+      "distributor": {
+        "description": "Name of the distributor",
+        "type": "string"
+      },
+      "terminal": {
+        "$ref": "#/definitions/terminal"
+      },
       "terminal_input": {
-         "$ref": "#/definitions/terminal"
+        "$ref": "#/definitions/terminal"
       },
       "terminal_output": {
-         "$ref": "#/definitions/terminal"
+        "$ref": "#/definitions/terminal"
       },
       "timeout": {
         "description": "Timeout in seconds",
@@ -214,6 +227,10 @@ SCHEMA = """
         "minimum": 0,
         "default": 0
       },
+      "timeout_style": {
+        "type": "string",
+        "enum": ["hidden", "menu", "countdown"]
+      },
       "serial": {
         "description": "The command to configure the serial console",
         "type": "string"
@@ -371,10 +388,15 @@ class GrubConfig:
         self.ignition = False
         self.greenboot = False
         self.kernel_opts = ""
+        self.disable_recovery = None
+        self.disable_submenu = None
+        self.distributor = None
         self.serial = ""
+        self.terminal = None
         self.terminal_input = None
         self.terminal_output = None
         self.timeout = 0
+        self.timeout_style = None
 
     @property
     def grubfs(self):
@@ -472,9 +494,22 @@ class GrubConfig:
             "GRUB_ENABLE_BLSCFG=true\n"
         )
 
+        if self.disable_recovery is not None:
+            data += f"GRUB_DISABLE_RECOVERY={str(self.disable_recovery).lower()}\n"
+
+        if self.disable_submenu is not None:
+            data += f"GRUB_DISABLE_SUBMENU={str(self.disable_submenu).lower()}\n"
+
+        if self.distributor:
+            data += f'GRUB_DISTRIBUTOR="{self.distributor}"\n'
+
         if self.serial:
             data += f'GRUB_SERIAL_COMMAND="{self.serial}"\n'
 
+        if self.terminal:
+            val = " ".join(self.terminal)
+            data += f'GRUB_TERMINAL="{val}"\n'
+
         if self.terminal_input:
             val = " ".join(self.terminal_input)
             data += f'GRUB_TERMINAL_INPUT="{val}"\n'
@@ -483,6 +518,9 @@ class GrubConfig:
             val = " ".join(self.terminal_output)
             data += f'GRUB_TERMINAL_OUTPUT="{val}"\n'
 
+        if self.timeout_style:
+            data += f'GRUB_TIMEOUT_STYLE={self.timeout_style}\n'
+
         if self.default_entry is not None:
             data += f'GRUB_DEFAULT={self.default_entry}\n'
 
@@ -525,10 +563,15 @@ def main(tree, options):
     config.ignition = ignition
     config.greenboot = options.get("greenboot", False)
     config.kernel_opts = kernel_opts
+    config.disable_recovery = cfg.get("disable_recovery")
+    config.disable_submenu = cfg.get("disable_submenu")
+    config.distributor = cfg.get("distributor")
     config.serial = cfg.get("serial")
+    config.terminal = cfg.get("terminal")
     config.terminal_input = cfg.get("terminal_input")
     config.terminal_output = cfg.get("terminal_output")
     config.timeout = cfg.get("timeout", 0)
+    config.timeout_style = cfg.get("timeout_style")
     config.default_entry = cfg.get("default")
 
     # Create the configuration file that determines how grub.cfg is generated.
diff --git a/stages/org.osbuild.grub2.legacy b/stages/org.osbuild.grub2.legacy
index 16e676ff..56b7e997 100755
--- a/stages/org.osbuild.grub2.legacy
+++ b/stages/org.osbuild.grub2.legacy
@@ -174,15 +174,26 @@ SCHEMA = """
         "description": "Additional kernel command line options",
         "type": "string"
       },
+      "disable_recovery": {
+        "type": "boolean",
+        "default": true
+      },
+      "disable_submenu": {
+        "type": "boolean",
+        "default": true
+      },
       "distributor": {
         "description": "Name of the distributor",
         "type": "string"
       },
+      "terminal": {
+        "$ref": "#/definitions/terminal"
+      },
       "terminal_input": {
-         "$ref": "#/definitions/terminal"
+        "$ref": "#/definitions/terminal"
       },
       "terminal_output": {
-         "$ref": "#/definitions/terminal"
+        "$ref": "#/definitions/terminal"
       },
       "timeout": {
         "description": "Timeout in seconds",
@@ -190,6 +201,11 @@ SCHEMA = """
         "minimum": 0,
         "default": 0
       },
+      "timeout_style": {
+        "type": "string",
+        "enum": ["hidden", "menu", "countdown"],
+        "default": "countdown"
+      },
       "serial": {
         "description": "The command to configure the serial console",
         "type": "string"
@@ -331,6 +347,7 @@ class GrubEntry:
         return entry
 
 
+# pylint: disable=too-many-instance-attributes
 class GrubConfig:
     def __init__(self, bios, rootfs, bootfs):
         self.bios = bios
@@ -338,11 +355,15 @@ class GrubConfig:
         self.bootfs = bootfs
         self.entries = []
         self.cmdline = ""
+        self.disable_recovery = True
+        self.disable_submenu = True
         self.distributor = ""
         self.serial = ""
+        self.terminal = None
         self.terminal_input = None
         self.terminal_output = None
         self.timeout = 0
+        self.timeout_style = "countdown"
 
     @property
     def grubfs(self):
@@ -432,9 +453,9 @@ class GrubConfig:
         data = (
             f"GRUB_TIMEOUT={self.timeout}\n"
             f'GRUB_CMDLINE_LINUX="{self.cmdline}"\n'
-            "GRUB_DISABLE_SUBMENU=true\n"
-            "GRUB_DISABLE_RECOVERY=true\n"
-            "GRUB_TIMEOUT_STYLE=countdown\n"
+            f"GRUB_DISABLE_SUBMENU={str(self.disable_submenu).lower()}\n"
+            f"GRUB_DISABLE_RECOVERY={str(self.disable_recovery).lower()}\n"
+            f"GRUB_TIMEOUT_STYLE={self.timeout_style}\n"
             "GRUB_DEFAULT=saved\n"
         )
 
@@ -444,6 +465,10 @@ class GrubConfig:
         if self.serial:
             data += f'GRUB_SERIAL_COMMAND="{self.serial}"\n'
 
+        if self.terminal:
+            val = " ".join(self.terminal)
+            data += f'GRUB_TERMINAL="{val}"\n'
+
         if self.terminal_input:
             val = " ".join(self.terminal_input)
             data += f'GRUB_TERMINAL_INPUT="{val}"\n'
@@ -467,11 +492,15 @@ def main(tree, options):
     cfg = options.get("config", {})
     config = GrubConfig(bios, root_fs, boot_fs)
     config.cmdline = cfg.get("cmdline", "")
+    config.disable_recovery = cfg.get("disable_recovery", True)
+    config.disable_submenu = cfg.get("disable_submenu", True)
     config.distributor = cfg.get("distributor")
     config.serial = cfg.get("serial")
+    config.terminal = cfg.get("terminal")
     config.terminal_input = cfg.get("terminal_input")
     config.terminal_output = cfg.get("terminal_output")
     config.timeout = cfg.get("timeout", 0)
+    config.timeout_style = cfg.get("timeout_style", "countdown")
     config.entries = [GrubEntry.from_json(e) for e in options["entries"]]
 
     # Create the configuration file that determines how grub.cfg is generated.
diff --git a/tools/check-snapshots b/tools/check-snapshots
index 969b1762..2b5fbe3b 100755
--- a/tools/check-snapshots
+++ b/tools/check-snapshots
@@ -23,7 +23,8 @@ def fetch_snapshots_api(url, timeout=SNAPSHOTS_TIMEOUT):
     start = time.time()
     try:
         r = requests.get(url, timeout=timeout)
-    except BaseException:
+    except BaseException as e:  # pylint: disable=broad-exception-caught
+        print(f"Problem downloading {url}: {e}")
         return None
     elapsed = time.time() - start
     if r.status_code != 200:
@@ -90,7 +91,8 @@ def check_baseurl(repo, snapshots):
     return invalid, newer
 
 
-def check_snapshot_urls(urls, snapshots, skip=["test/data/assemblers", "test/data/manifests", "test/data/stages"],
+# pylint: disable=too-many-branches
+def check_snapshot_urls(urls, snapshots, skip=("test/data/assemblers", "test/data/manifests", "test/data/stages"),
                         errors_only=False):
     """check the urls against the current list of snapshots
 
@@ -133,11 +135,11 @@ def check_snapshot_urls(urls, snapshots, skip=["test/data/assemblers", "test/dat
                     messages[f] = {"newer": [newer], "invalid": []}
 
     # Print the messages for each file
-    for f in messages:
+    for f, msgs in messages.items():
         print(f"{f}:")
-        for msg in messages[f]["invalid"]:
+        for msg in msgs["invalid"]:
             print(f"    ERROR: {msg}")
-        for msg in messages[f]["newer"]:
+        for msg in msgs["newer"]:
             print(f"    NEWER: {msg}")
 
     return ret
@@ -167,7 +169,7 @@ def main():
         try:
             with open(args.cache, encoding="utf8") as f:
                 snapshots = json.load(f)
-        except BaseException:
+        except FileNotFoundError:
             print(f"No snapshots cache found at {args.cache}")
             sys.exit(1)
     else:
diff --git a/tools/osbuild-depsolve-dnf5 b/tools/osbuild-depsolve-dnf5
index 0246aca5..903adfc1 100755
--- a/tools/osbuild-depsolve-dnf5
+++ b/tools/osbuild-depsolve-dnf5
@@ -11,6 +11,7 @@ import json
 import os
 import sys
 import tempfile
+import traceback
 from datetime import datetime
 
 import libdnf5 as dnf5
@@ -20,7 +21,7 @@ from libdnf5.common import QueryCmp_EQ as EQ
 from libdnf5.common import QueryCmp_GLOB as GLOB
 
 
-# XXX - Temporarily lifted from dnf.rpm module
+# XXX - Temporarily lifted from dnf.rpm module  # pylint: disable=fixme
 def _invert(dct):
     return {v: k for k in dct for v in dct[k]}
 
@@ -140,7 +141,7 @@ class Solver():
 
             self.base.get_repo_sack().update_and_load_enabled_repos(load_system=False)
         except RuntimeError as e:
-            raise RepoError(e)
+            raise RepoError(e) from e
 
     # pylint: disable=too-many-branches
     def _dnfrepo(self, desc, exclude_pkgs=None):
@@ -420,8 +421,7 @@ def solve(request, cache_dir):
                 "kind": "RepoError",
                 "reason": f"There was a problem reading a repository: {e}"
             }
-        except Exception as e:
-            import traceback
+        except Exception as e:  # pylint: disable=broad-exception-caught
             printe("error traceback")
             return None, {
                 "kind": type(e).__name__,
diff --git a/tools/osbuild-mpp b/tools/osbuild-mpp
index 5eab453e..56513038 100755
--- a/tools/osbuild-mpp
+++ b/tools/osbuild-mpp
@@ -692,7 +692,7 @@ class DepSolver:
             if not result.scheme:
                 path = basedir.joinpath(baseurl)
                 return path.resolve().as_uri()
-        except BaseException:  # pylint: disable=bare-except
+        except BaseException:  # pylint: disable=broad-exception-caught
             pass
 
         return baseurl
@@ -1008,7 +1008,7 @@ class Image:
                 try:
                     table.write_to(loopimage)
                 finally:
-                    subprocess.run(["losetup", "-d", loopimage])
+                    subprocess.run(["losetup", "-d", loopimage], check=True)
 
         return cls(size, table)
 
@@ -1215,7 +1215,7 @@ class ManifestFile:
             pipeline_name = self.get_pipeline_name(parent_node)
             self._process_stage(node, pipeline_name)
 
-    # pylint: disable=too-many-branches
+    # pylint: disable=too-many-branches,too-many-statements
     def _process_format(self, node):
         def _is_format(node):
             if not isinstance(node, dict):
@@ -1245,7 +1245,7 @@ class ManifestFile:
                 res = False
                 try:
                     res = eval(code, dict(local_vars))
-                except Exception as e:
+                except Exception as e:  # pylint: disable=broad-exception-caught
                     print(f"In {self.path}: Failed to evaluate mpp-if of:\n  {code}")
                     print(f"Error: {e}")
                     sys.exit(1)
@@ -1266,7 +1266,7 @@ class ManifestFile:
                 # Note, we copy local_vars here to avoid eval modifying it
                 try:
                     res = eval(code, dict(local_vars))
-                except Exception as e:
+                except Exception as e:  # pylint: disable=broad-exception-caught
                     print(f"In {self.path}: Failed to mpp-eval:\n  {code}")
                     print(f"Error: {e}")
                     sys.exit(1)
@@ -1286,7 +1286,7 @@ class ManifestFile:
             # Note, we copy local_vars here to avoid eval modifying it
             try:
                 res = eval(f'f\'\'\'{format_string}\'\'\'', dict(local_vars))
-            except Exception as e:
+            except Exception as e:  # pylint: disable=broad-exception-caught
                 print(f"In {self.path}: Failed to format string:\n  {format_string}")
                 print(f"Error: {e}")
                 sys.exit(1)
@@ -1373,7 +1373,7 @@ class ManifestFile:
     def _process_container(self, _stage):
         raise NotImplementedError()
 
-    def _process_ostree(self, _stage):
+    def _process_ostree_commits(self, _stage):
         raise NotImplementedError()
 
 
@@ -1788,7 +1788,7 @@ def main():
     args = parser.parse_args(sys.argv[1:])
 
     defaults = {
-        "arch": rpm.expandMacro("%{_arch}")
+        "arch": rpm.expandMacro("%{_arch}")  # pylint: disable=no-member
     }
 
     # Override variables from the main of imported files
diff --git a/tox.ini b/tox.ini
index 1067472f..731d042f 100644
--- a/tox.ini
+++ b/tox.ini
@@ -22,9 +22,10 @@ deps =
     pykickstart
     # required by pykickstart but not pulled in automatically :/
     requests
+    tox-backticks
 
 setenv =
-    LINTABLES = osbuild/ assemblers/* devices/* inputs/* mounts/* runners/* sources/* stages/*.* stages/test/*.py test/ tools/
+    LINTABLES = osbuild/ assemblers/* devices/* inputs/* mounts/* runners/* sources/* stages/*.* stages/test/*.py test/ `find ./tools ! -name "*.sh" -type f`
     TYPEABLES = osbuild
     TYPEABLES_STRICT = ./osbuild/main_cli.py
 


Test patch:
diff --git a/stages/test/test_grub2.py b/stages/test/test_grub2.py
index f3197baf..91a8d987 100644
--- a/stages/test/test_grub2.py
+++ b/stages/test/test_grub2.py
@@ -1,5 +1,9 @@
 #!/usr/bin/python3
 
+import os.path
+
+import pytest
+
 from osbuild.testutil import make_fake_tree
 
 STAGE_NAME = "org.osbuild.grub2"
@@ -26,3 +30,80 @@ def test_grub2_copy_efi_data(tmp_path, stage_module):
     stage_module.main(fake_tree, test_options)
     assert (fake_tree / "boot/efi/EFI/fedora/a.shim").exists()
     assert (fake_tree / "boot/efi/EFI/BOOT/BOOTX64.EFI").exists()
+
+
+# Test that the /etc/default/grub file is created with the correct content
+@pytest.mark.parametrize("test_data,kernel_opts,expected_conf", [
+    # default
+    ({}, "", """GRUB_CMDLINE_LINUX=""
+GRUB_TIMEOUT=0
+GRUB_ENABLE_BLSCFG=true
+"""),
+    # custom
+    ({
+        "default": "0",
+        "timeout": 10,
+        "terminal_input": ["console"],
+        "terminal_output": ["serial"],
+        "serial": "serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1",
+    }, "", """GRUB_CMDLINE_LINUX=""
+GRUB_TIMEOUT=10
+GRUB_ENABLE_BLSCFG=true
+GRUB_SERIAL_COMMAND="serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1"
+GRUB_TERMINAL_INPUT="console"
+GRUB_TERMINAL_OUTPUT="serial"
+GRUB_DEFAULT=0
+"""),
+    # custom (Azure)
+    ({
+        "default": "saved",
+        "disable_submenu": True,
+        "disable_recovery": True,
+        "distributor": "$(sed 's, release .*$,,g' /etc/system-release)",
+        "serial": "serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1",
+        "terminal": ["serial", "console"],
+        "terminal_output": ["console"],
+        "timeout": 10,
+        "timeout_style": "countdown",
+    },
+        "loglevel=3 crashkernel=auto console=tty1 console=ttyS0 earlyprintk=ttyS0 rootdelay=300",
+        """GRUB_CMDLINE_LINUX="loglevel=3 crashkernel=auto console=tty1 console=ttyS0 earlyprintk=ttyS0 rootdelay=300"
+GRUB_TIMEOUT=10
+GRUB_ENABLE_BLSCFG=true
+GRUB_DISABLE_RECOVERY=true
+GRUB_DISABLE_SUBMENU=true
+GRUB_DISTRIBUTOR="$(sed 's, release .*$,,g' /etc/system-release)"
+GRUB_SERIAL_COMMAND="serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1"
+GRUB_TERMINAL="serial console"
+GRUB_TERMINAL_OUTPUT="console"
+GRUB_TIMEOUT_STYLE=countdown
+GRUB_DEFAULT=saved
+"""),
+])
+def test_grub2_default_conf(tmp_path, stage_module, test_data, kernel_opts, expected_conf):
+    treedir = tmp_path / "tree"
+    confpath = treedir / "etc/default/grub"
+    confpath.parent.mkdir(parents=True, exist_ok=True)
+
+    options = {
+        "rootfs": {
+            "label": "root"
+        },
+        "entries": [
+            {
+                "id": "fff",
+                "kernel": "4.18",
+                "product": {
+                    "name": "Fedora",
+                    "version": "40"
+                }
+            }
+        ]
+    }
+
+    options["config"] = test_data
+    options["kernel_opts"] = kernel_opts
+    stage_module.main(treedir, options)
+
+    assert os.path.exists(confpath)
+    assert confpath.read_text() == expected_conf
diff --git a/stages/test/test_grub2_legacy.py b/stages/test/test_grub2_legacy.py
new file mode 100644
index 00000000..5e0b346c
--- /dev/null
+++ b/stages/test/test_grub2_legacy.py
@@ -0,0 +1,81 @@
+#!/usr/bin/python3
+
+import os.path
+
+import pytest
+
+STAGE_NAME = "org.osbuild.grub2.legacy"
+
+
+# Test that the /etc/default/grub file is created with the correct content
+@pytest.mark.parametrize("test_data,expected_conf", [
+    # default
+    ({}, """GRUB_TIMEOUT=0
+GRUB_CMDLINE_LINUX=""
+GRUB_DISABLE_SUBMENU=true
+GRUB_DISABLE_RECOVERY=true
+GRUB_TIMEOUT_STYLE=countdown
+GRUB_DEFAULT=saved
+"""),
+    # custom
+    ({
+        "disable_submenu": False,
+        "disable_recovery": False,
+        "timeout": 10,
+        "timeout_style": "hidden",
+    }, """GRUB_TIMEOUT=10
+GRUB_CMDLINE_LINUX=""
+GRUB_DISABLE_SUBMENU=false
+GRUB_DISABLE_RECOVERY=false
+GRUB_TIMEOUT_STYLE=hidden
+GRUB_DEFAULT=saved
+"""),
+    # custom (Azure)
+    ({
+        "cmdline": "loglevel=3 crashkernel=auto console=tty1 console=ttyS0 earlyprintk=ttyS0 rootdelay=300",
+        "disable_submenu": True,
+        "disable_recovery": True,
+        "distributor": "$(sed 's, release .*$,,g' /etc/system-release)",
+        "serial": "serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1",
+        "terminal": ["serial", "console"],
+        "terminal_output": ["console"],
+        "timeout": 10,
+        "timeout_style": "countdown",
+    }, """GRUB_TIMEOUT=10
+GRUB_CMDLINE_LINUX="loglevel=3 crashkernel=auto console=tty1 console=ttyS0 earlyprintk=ttyS0 rootdelay=300"
+GRUB_DISABLE_SUBMENU=true
+GRUB_DISABLE_RECOVERY=true
+GRUB_TIMEOUT_STYLE=countdown
+GRUB_DEFAULT=saved
+GRUB_DISTRIBUTOR="$(sed 's, release .*$,,g' /etc/system-release)"
+GRUB_SERIAL_COMMAND="serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1"
+GRUB_TERMINAL="serial console"
+GRUB_TERMINAL_OUTPUT="console"
+"""),
+])
+def test_grub2_default_conf(tmp_path, stage_module, test_data, expected_conf):
+    treedir = tmp_path / "tree"
+    confpath = treedir / "etc/default/grub"
+    confpath.parent.mkdir(parents=True, exist_ok=True)
+
+    options = {
+        "rootfs": {
+            "label": "root"
+        },
+        "entries": [
+            {
+                "id": "fff",
+                "kernel": "4.18",
+                "product": {
+                    "name": "Fedora",
+                    "version": "40"
+                }
+            }
+        ]
+    }
+
+    options["config"] = test_data
+    stage_module.main(treedir, options)
+
+    assert os.path.exists(confpath)
+    assert confpath.read_text() == expected_conf
diff --git a/tools/gen-stage-test-diff b/tools/gen-stage-test-diff
index fb50eaf1..c302b5e7 100755
--- a/tools/gen-stage-test-diff
+++ b/tools/gen-stage-test-diff
@@ -40,7 +40,7 @@ def run_osbuild(output_directory, store, cache_max_size, libdir, manifest):
     except subprocess.CalledProcessError as err:
         raise RuntimeError(
             f"osbuild crashed when building {manifest}:\n\nstdout:\n{err.stdout}\n\nstderr:\n{err.stderr}"
-        )
+        ) from err
 
 
 epilog = """