Repository: osbuild/osbuild
Pull request #1742 (2024-04-18 14:57:25)

stages: allow `bootloader --append` via kickstart
This commit allows use to append kernel commandline options via the kickstart file. This is useful for e.g. the bootc installer where we support customizing this via blueprints.


Patch:
diff --git a/stages/org.osbuild.kickstart b/stages/org.osbuild.kickstart
index f3439cfc..21d03ace 100755
--- a/stages/org.osbuild.kickstart
+++ b/stages/org.osbuild.kickstart
@@ -243,6 +243,9 @@ def main(tree, options):  # pylint: disable=too-many-branches
     if reboot:
         config += [reboot]
     config += make_network(options)
+    kargs_append = options.get("bootloader", {}).get("append")
+    if kargs_append:
+        config += [f"bootloader --append='{kargs_append}'"]
 
     target = os.path.join(tree, path)
     base = os.path.dirname(target)
diff --git a/stages/org.osbuild.kickstart.meta.json b/stages/org.osbuild.kickstart.meta.json
index 773c9f09..21ad84b3 100644
--- a/stages/org.osbuild.kickstart.meta.json
+++ b/stages/org.osbuild.kickstart.meta.json
@@ -542,6 +542,17 @@
         "items": {
           "$ref": "#/definitions/network-options-ref"
         }
+      },
+      "bootloader": {
+        "description": "Specifies how the boot loader should be installed",
+        "additionalProperties": false,
+        "type": "object",
+        "properties": {
+          "append": {
+            "description": "Specifies additional kernel parameters",
+            "type": "string"
+          }
+        }
       }
     }
   }
diff --git a/stages/org.osbuild.systemd.unit.create b/stages/org.osbuild.systemd.unit.create
index f4c78181..f0b61016 100755
--- a/stages/org.osbuild.systemd.unit.create
+++ b/stages/org.osbuild.systemd.unit.create
@@ -1,6 +1,5 @@
 #!/usr/bin/python3
 import configparser
-import subprocess
 import sys
 
 import osbuild.api
@@ -48,8 +47,6 @@ def main(tree, options):
     with open(f"{systemd_dir}/{filename}", "w", encoding="utf8") as f:
         config.write(f, space_around_delimiters=False)
 
-    subprocess.run(["systemd-analyze", "verify", f"{systemd_dir}/{filename}"], check=True)
-
 
 if __name__ == '__main__':
     args = osbuild.api.arguments()
diff --git a/stages/org.osbuild.systemd.unit.create.meta.json b/stages/org.osbuild.systemd.unit.create.meta.json
index d64d01ec..8919faab 100644
--- a/stages/org.osbuild.systemd.unit.create.meta.json
+++ b/stages/org.osbuild.systemd.unit.create.meta.json
@@ -18,6 +18,7 @@
     "    - 'DefaultDependencies' - bool",
     "    - 'Requires' - [strings]",
     "    - 'Wants' - [strings]",
+    "    - 'After' - [strings]",
     "  - 'Service' section",
     "    - 'Type' - string",
     "    - 'RemainAfterExit' - bool",
@@ -83,6 +84,12 @@
                   "type": "string"
                 }
               },
+              "After": {
+                "type": "array",
+                "items": {
+                  "type": "string"
+                }
+              },
               "Requires": {
                 "type": "array",
                 "items": {


Test patch:
diff --git a/stages/test/test_kickstart.py b/stages/test/test_kickstart.py
index e62f4433..1aeeb305 100644
--- a/stages/test/test_kickstart.py
+++ b/stages/test/test_kickstart.py
@@ -234,6 +234,7 @@ TEST_INPUT = [
         },
         "ostreecontainer --url=http://some-ostree-url.com/foo",
     ),
+    ({"bootloader": {"append": "karg1 karg2=0"}}, "bootloader --append='karg1 karg2=0'"),
 ]
 
 
diff --git a/stages/test/test_systemd_unit_create.py b/stages/test/test_systemd_unit_create.py
index b686261d..a6c22c95 100644
--- a/stages/test/test_systemd_unit_create.py
+++ b/stages/test/test_systemd_unit_create.py
@@ -1,7 +1,7 @@
 #!/usr/bin/python3
 
 import os
-import os.path
+import subprocess
 import textwrap
 
 import pytest
@@ -66,14 +66,27 @@ def test_systemd_unit_create(tmp_path, stage_module, unit_type, unit_path, expec
                 ],
                 "ConditionPathIsDirectory": [
                     "|!/etc/mydir"
-                ]
+                ],
+                # need to use real units otherwise the validation will fail
+                "Wants": [
+                    "basic.target",
+                    "sysinit.target",
+                ],
+                "Requires": [
+                    "paths.target",
+                    "sockets.target",
+                ],
+                "After": [
+                    "sysinit.target",
+                    "default.target",
+                ],
             },
             "Service": {
                 "Type": "oneshot",
                 "RemainAfterExit": True,
                 "ExecStart": [
-                    "mkdir -p /etc/mydir",
-                    "touch /etc/myfile"
+                    "/usr/bin/mkdir -p /etc/mydir",
+                    "/usr/bin/touch /etc/myfile"
                 ],
                 "Environment": [
                     {
@@ -100,24 +113,53 @@ def test_systemd_unit_create(tmp_path, stage_module, unit_type, unit_path, expec
             }
         }
     }
+
+    # create units in the tree for systemd-analyze verify
+    systemd_dir = "usr/lib/systemd/system/"
+    # if a unit file is empty, it gets detected as "masked" and fails verification
+    testutil.make_fake_tree(tmp_path, {
+        systemd_dir + "basic.target": "[Unit]\n",
+        systemd_dir + "sysinit.target": "[Unit]\n",
+        systemd_dir + "paths.target": "[Unit]\n",
+        systemd_dir + "sockets.target": "[Unit]\n",
+        "usr/bin/mkdir": "fake-mkdir",
+        "usr/bin/touch": "fake-touch",
+    })
+    for bin_name in ["mkdir", "touch"]:
+        os.chmod(tmp_path / "usr/bin" / bin_name, mode=0o755)
+
     expected_unit_path = tmp_path / expected_prefix / "create-directory.service"
-    # should the stage create the dir?
-    expected_unit_path.parent.mkdir(parents=True)
+    expected_unit_path.parent.mkdir(parents=True, exist_ok=True)
 
     stage_module.main(tmp_path, options)
     assert os.path.exists(expected_unit_path)
+    if unit_type == "system":
+        # When verifying user units, systemd expects runtime directories and more generally a booted system. Setting
+        # something up like that to verify it is more trouble than it's worth, especially since the system units are
+        # identical.
+        subprocess.run(["systemd-analyze", "verify",
+                        "--man=no",  # disable manpage checks
+                        f"--root={tmp_path}",  # verify commands and units in the root tree
+                        expected_unit_path],
+                       check=True)
     assert expected_unit_path.read_text(encoding="utf-8") == textwrap.dedent("""\
     [Unit]
     Description=Create directory
     DefaultDependencies=False
     ConditionPathExists=|!/etc/myfile
     ConditionPathIsDirectory=|!/etc/mydir
+    Wants=basic.target
+    Wants=sysinit.target
+    Requires=paths.target
+    Requires=sockets.target
+    After=sysinit.target
+    After=default.target
 
     [Service]
     Type=oneshot
     RemainAfterExit=True
-    ExecStart=mkdir -p /etc/mydir
-    ExecStart=touch /etc/myfile
+    ExecStart=/usr/bin/mkdir -p /etc/mydir
+    ExecStart=/usr/bin/touch /etc/myfile
     Environment="DEBUG=1"
     Environment="TRACE=1"
     EnvironmentFile=/etc/example.env