Skip to content

Commit

Permalink
osbuild: update the org.osbuild.bootupd stage with latest upstream
Browse files Browse the repository at this point in the history
There were some updates from code review in [1]. Let's pull them
here and also update the mpp.yaml manifest to include the ostree
deployment specification.

[1] osbuild/osbuild#1519
  • Loading branch information
dustymabe committed Jan 9, 2024
1 parent 866d635 commit 970f4c5
Show file tree
Hide file tree
Showing 6 changed files with 414 additions and 37 deletions.
7 changes: 5 additions & 2 deletions build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ patch_osbuild() {
mv /usr/bin/osbuild-mpp /usr/lib/osbuild/tools/

# Now all the software is under the /usr/lib/osbuild dir and we can patch
cat /usr/lib/coreos-assembler/0001-create-org.osbuild.bootupd-stage.patch \
/usr/lib/coreos-assembler/0001-tools-osbuild-mpp-run-_process_format-for-mpp-embed-.patch \
cat /usr/lib/coreos-assembler/0001-create-org.osbuild.bootupd-stage.patch \
/usr/lib/coreos-assembler/0002-stages-bootupd-add-schema-test.patch \
/usr/lib/coreos-assembler/0003-stages-bootupd-add-test-for-existing-behavior-and-re.patch \
/usr/lib/coreos-assembler/0004-stages-bootupd-refactor-test-to-test-bind_mounts-and.patch \
/usr/lib/coreos-assembler/0001-tools-osbuild-mpp-run-_process_format-for-mpp-embed-.patch \
| patch -d /usr/lib/osbuild -p1

# And then move the files back; supermin appliance creation will need it back
Expand Down
70 changes: 35 additions & 35 deletions src/0001-create-org.osbuild.bootupd-stage.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 1daad28dda6ad47109499fbd67a5094605c4221b Mon Sep 17 00:00:00 2001
From e06d65754ae9c5d0ae1e7525ef009c04fa11d14a Mon Sep 17 00:00:00 2001
From: Renata Ravanelli <[email protected]>
Date: Thu, 16 Nov 2023 09:41:18 -0500
Subject: [PATCH] create org.osbuild.bootupd stage
Subject: [PATCH 1/4] create org.osbuild.bootupd stage

Add the bootupd stage to install GRUB on both BIOS and UEFI systems,
ensuring that your bootloader stays up-to-date.
Expand All @@ -14,7 +14,7 @@ Signed-off-by: Renata Ravanelli <[email protected]>

diff --git a/stages/org.osbuild.bootupd b/stages/org.osbuild.bootupd
new file mode 100755
index 00000000..96613019
index 00000000..a7ffe610
--- /dev/null
+++ b/stages/org.osbuild.bootupd
@@ -0,0 +1,125 @@
Expand Down Expand Up @@ -45,43 +45,43 @@ index 00000000..96613019
+ "type": "array"
+},
+"options": {
+"additionalProperties": true,
+"properties": {
+ "deployment": {
+ "additionalProperties": false,
+ "required": ["osname", "ref"],
+ "properties": {
+ "osname": {
+ "description": "Name of the stateroot to be used in the deployment",
+ "type": "string"
+ },
+ "ref": {
+ "description": "OStree ref to create and use for deployment",
+ "type": "string"
+ },
+ "serial": {
+ "description": "The deployment serial (usually '0')",
+ "type": "number",
+ "default": 0
+ }
+ }
+ },
+ "static-configs": {
+ "description": "Install the grub configs defined for Fedora CoreOS",
+ "type": "boolean"
+ },
+ "bios": {
+ "additionalProperties": false,
+ "required": ["disk"],
+ "properties": {
+ "disk": {
+ "description": "Disk to install GRUB for BIOS-based systems",
+ "additionalProperties": true,
+ "properties": {
+ "deployment": {
+ "additionalProperties": false,
+ "required": ["osname", "ref"],
+ "properties": {
+ "osname": {
+ "description": "Name of the stateroot to be used in the deployment",
+ "type": "string"
+ },
+ "ref": {
+ "description": "OStree ref to create and use for deployment",
+ "type": "string"
+ },
+ "serial": {
+ "description": "The deployment serial (usually '0')",
+ "type": "number",
+ "default": 0
+ }
+ }
+ },
+ "static-configs": {
+ "description": "Install the grub configs defined for Fedora CoreOS",
+ "type": "boolean"
+ },
+ "bios": {
+ "additionalProperties": false,
+ "required": ["disk"],
+ "properties": {
+ "disk": {
+ "description": "Disk to install GRUB for BIOS-based systems",
+ "type": "string"
+ }
+ }
+ }
+ }
+}
+}
+"""
+
+
Expand Down
100 changes: 100 additions & 0 deletions src/0002-stages-bootupd-add-schema-test.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
From 63f8e8e5d340513ca4a5dc0abe6c2d269ee57be3 Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
Date: Tue, 9 Jan 2024 11:12:06 +0100
Subject: [PATCH 2/4] stages(bootupd): add schema test

---
stages/org.osbuild.bootupd | 4 ++-
stages/test/test_bootupd.py | 56 +++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 stages/test/test_bootupd.py

diff --git a/stages/org.osbuild.bootupd b/stages/org.osbuild.bootupd
index a7ffe610..2371900e 100755
--- a/stages/org.osbuild.bootupd
+++ b/stages/org.osbuild.bootupd
@@ -25,9 +25,10 @@ SCHEMA_2 = r"""
"type": "array"
},
"options": {
- "additionalProperties": true,
+ "additionalProperties": false,
"properties": {
"deployment": {
+ "type": "object",
"additionalProperties": false,
"required": ["osname", "ref"],
"properties": {
@@ -52,6 +53,7 @@ SCHEMA_2 = r"""
},
"bios": {
"additionalProperties": false,
+ "type": "object",
"required": ["disk"],
"properties": {
"disk": {
diff --git a/stages/test/test_bootupd.py b/stages/test/test_bootupd.py
new file mode 100644
index 00000000..bddd69dd
--- /dev/null
+++ b/stages/test/test_bootupd.py
@@ -0,0 +1,56 @@
+#!/usr/bin/python3
+
+import os.path
+import subprocess
+from unittest import mock
+
+import pytest
+
+import osbuild.meta
+from osbuild.testutil import has_executable, make_fake_input_tree
+from osbuild.testutil.imports import import_module_from_path
+
+
+@pytest.mark.parametrize("test_data,expected_err", [
+ # bad
+ ({"deployment": "totally"}, "'totally' is not of type 'object'"),
+ ({"deployment": {"osname": "some-os"}}, "'ref' is a required property"),
+ ({"deployment": {"ref": "some-ref"}}, "'osname' is a required property"),
+ ({"deployment": {"osname": "some-os", "ref": "some-ref", "serial": "yo"}}, "'yo' is not of type 'number'"),
+ ({"random": "property"}, "Additional properties are not allowed"),
+ ({"bios": {}}, "'disk' is a required property"),
+ ({"bios": "yes"}, "'yes' is not of type 'object'"),
+ # good
+ ({}, ""),
+ ({"deployment": {
+ "osname": "some-os",
+ "ref": "some-ref",
+ "serial": 1,
+ },
+ "static-configs": True,
+ "bios": {
+ "disk": "/dev/sda",
+ }}, "")
+
+])
+def test_schema_validation_bootupd(test_data, expected_err):
+ name = "org.osbuild.bootupd"
+ root = os.path.join(os.path.dirname(__file__), "../..")
+ mod_info = osbuild.meta.ModuleInfo.load(root, "Stage", name)
+ schema = osbuild.meta.Schema(mod_info.get_schema(version="2"), name)
+
+ test_input = {
+ "type": "org.osbuild.bootupd",
+ "options": {
+ }
+ }
+ test_input["options"].update(test_data)
+ res = schema.validate(test_input)
+
+ if expected_err == "":
+ assert res.valid is True, f"err: {[e.as_dict() for e in res.errors]}"
+ else:
+ assert res.valid is False
+ assert len(res.errors) == 1, [e.as_dict() for e in res.errors]
+ err_msgs = [e.as_dict()["message"] for e in res.errors]
+ assert expected_err in err_msgs[0]
--
2.43.0

167 changes: 167 additions & 0 deletions src/0003-stages-bootupd-add-test-for-existing-behavior-and-re.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
From de97cbe05ea187d32a0ad4cacdfd70103cc7eb1f Mon Sep 17 00:00:00 2001
From: Michael Vogt <[email protected]>
Date: Tue, 9 Jan 2024 11:54:37 +0100
Subject: [PATCH 3/4] stages(bootupd): add test for existing behavior and
refactor bind mounts into a helper

---
stages/org.osbuild.bootupd | 27 ++++++++-------
stages/test/test_bootupd.py | 65 ++++++++++++++++++++++++++++---------
2 files changed, 64 insertions(+), 28 deletions(-)

diff --git a/stages/org.osbuild.bootupd b/stages/org.osbuild.bootupd
index 2371900e..84483861 100755
--- a/stages/org.osbuild.bootupd
+++ b/stages/org.osbuild.bootupd
@@ -9,6 +9,7 @@ and GRUB for BIOS firmware on x86_64.
The project is deployed in Fedora CoreOS and derivatives
"""

+import contextlib
import os
import subprocess
import sys
@@ -67,6 +68,19 @@ SCHEMA_2 = r"""
"""


+@contextlib.contextmanager
+def bind_mounts(sources, dest_root):
+ for src in sources:
+ dst = os.path.join(dest_root, src.lstrip("/"))
+ subprocess.run(["mount", "--rbind", src, dst], check=True)
+ try:
+ yield
+ finally:
+ for src in sources:
+ dst = os.path.join(dest_root, src.lstrip("/"))
+ subprocess.run(["umount", "--recursive", dst], check=False)
+
+
def main(args, options):
deployment = options.get("deployment", None)
static_configs = options.get("static-configs", False)
@@ -101,22 +115,11 @@ def main(args, options):
# want to make sure the version used matches the target and not
# risk any inconsistencies with the build root). Let's set up
# and chroot to run the bootupctl command from the target.
- submounts = ['dev', 'proc', 'sys', 'run', 'var', 'tmp']
- for mnt in submounts:
- subprocess.run(['mount', '--rbind',
- os.path.join("/", mnt),
- os.path.join(root, mnt)],
- check=True)
- try:
+ with bind_mounts(['/dev', '/proc', '/sys', '/run', '/var', '/tmp'], root):
cmd = ['chroot', root, '/usr/bin/bootupctl', 'backend', 'install']
cmd.extend(bootupd_args)
cmd.append(mounts)
subprocess.run(cmd, check=True)
- finally:
- for mnt in submounts:
- subprocess.run(['umount', '--recursive',
- os.path.join(root, mnt)],
- check=False)

return 0

diff --git a/stages/test/test_bootupd.py b/stages/test/test_bootupd.py
index bddd69dd..026c8d8d 100644
--- a/stages/test/test_bootupd.py
+++ b/stages/test/test_bootupd.py
@@ -1,39 +1,42 @@
#!/usr/bin/python3

import os.path
-import subprocess
-from unittest import mock
+from unittest.mock import call, patch

import pytest

import osbuild.meta
-from osbuild.testutil import has_executable, make_fake_input_tree
from osbuild.testutil.imports import import_module_from_path


@pytest.mark.parametrize("test_data,expected_err", [
# bad
- ({"deployment": "totally"}, "'totally' is not of type 'object'"),
+ ({"deployment": "must-be-object"}, "'must-be-object' is not of type 'object'"),
({"deployment": {"osname": "some-os"}}, "'ref' is a required property"),
({"deployment": {"ref": "some-ref"}}, "'osname' is a required property"),
- ({"deployment": {"osname": "some-os", "ref": "some-ref", "serial": "yo"}}, "'yo' is not of type 'number'"),
+ ({"deployment": {"osname": "some-os", "ref": "some-ref", "serial": "must-be-number"}},
+ "'must-be-number' is not of type 'number'"),
({"random": "property"}, "Additional properties are not allowed"),
({"bios": {}}, "'disk' is a required property"),
- ({"bios": "yes"}, "'yes' is not of type 'object'"),
+ ({"bios": "must-be-object"}, "'must-be-object' is not of type 'object'"),
# good
({}, ""),
- ({"deployment": {
- "osname": "some-os",
- "ref": "some-ref",
- "serial": 1,
- },
- "static-configs": True,
- "bios": {
- "disk": "/dev/sda",
- }}, "")
+ ({
+ "deployment":
+ {
+ "osname": "some-os",
+ "ref": "some-ref",
+ "serial": 1,
+ },
+ "static-configs": True,
+ "bios":
+ {
+ "disk": "/dev/sda",
+ },
+ }, "")

])
-def test_schema_validation_bootupd(test_data, expected_err):
+def test_bootupd_schema_validation(test_data, expected_err):
name = "org.osbuild.bootupd"
root = os.path.join(os.path.dirname(__file__), "../..")
mod_info = osbuild.meta.ModuleInfo.load(root, "Stage", name)
@@ -54,3 +57,33 @@ def test_schema_validation_bootupd(test_data, expected_err):
assert len(res.errors) == 1, [e.as_dict() for e in res.errors]
err_msgs = [e.as_dict()["message"] for e in res.errors]
assert expected_err in err_msgs[0]
+
+
+@patch("subprocess.run")
+def test_bootupd_defaults(mocked_run):
+ stage_path = os.path.join(os.path.dirname(__file__), "../org.osbuild.bootupd")
+ stage = import_module_from_path("bootupd_stage", stage_path)
+
+ args = {
+ "paths": {
+ "mounts": "/run/osbuild/mounts",
+ },
+ }
+ options = {}
+ stage.main(args, options)
+
+ assert mocked_run.call_args_list == [
+ call(["mount", "--rbind", "/dev", "/run/osbuild/mounts/dev"], check=True),
+ call(["mount", "--rbind", "/proc", "/run/osbuild/mounts/proc"], check=True),
+ call(["mount", "--rbind", "/sys", "/run/osbuild/mounts/sys"], check=True),
+ call(["mount", "--rbind", "/run", "/run/osbuild/mounts/run"], check=True),
+ call(["mount", "--rbind", "/var", "/run/osbuild/mounts/var"], check=True),
+ call(["mount", "--rbind", "/tmp", "/run/osbuild/mounts/tmp"], check=True),
+ call(["chroot", "/run/osbuild/mounts", "/usr/bin/bootupctl", "backend", "install", "/run/osbuild/mounts"], check=True),
+ call(["umount", "--recursive", "/run/osbuild/mounts/dev"], check=False),
+ call(["umount", "--recursive", "/run/osbuild/mounts/proc"], check=False),
+ call(["umount", "--recursive", "/run/osbuild/mounts/sys"], check=False),
+ call(["umount", "--recursive", "/run/osbuild/mounts/run"], check=False),
+ call(["umount", "--recursive", "/run/osbuild/mounts/var"], check=False),
+ call(["umount", "--recursive", "/run/osbuild/mounts/tmp"], check=False),
+ ]
--
2.43.0

Loading

0 comments on commit 970f4c5

Please sign in to comment.