stage/systemd.unit.create: move systemd-analyze verify to tests

Verifying the systemd unit also checks if any referred systemd units
(Wants, Requires, After) exist and if all commands in Exec exist and are
executable.  Without '--root', the systemd-analyze verify command is
testing this against files in the build root, which isn't valid.

Units and binaries might not exist in the build root when referenced in
the image root tree, making the unit fail when when it's valid.
Conversely, the verification can succeed by finding executables in the
build root that don't exist in the image root tree when it should be
failing.

When verifying user units, systemd expects runtime directories.

All of this makes it quite difficult to verify systemd units properly
when building an image.  The call is useful for making sure the unit is
structured properly, but the user unit verification setup is difficult
to accomplish in a general way while building.

Remove the systemd-analyze verify step from the stage.  Move it to the
unit test so that we have some assurance that our unit file structure is
correct and things work as expected.  Create referenced unit files and
commands to make the unit valid.
This commit is contained in:
Achilleas Koutsou 2024-04-17 02:07:41 +02:00 committed by Sanne Raymaekers
parent 86baf802d5
commit f255fba09f
2 changed files with 30 additions and 10 deletions

View file

@ -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()

View file

@ -1,7 +1,7 @@
#!/usr/bin/python3
import os
import os.path
import subprocess
import textwrap
import pytest
@ -85,8 +85,8 @@ def test_systemd_unit_create(tmp_path, stage_module, unit_type, unit_path, expec
"Type": "oneshot",
"RemainAfterExit": True,
"ExecStart": [
"mkdir -p /etc/mydir",
"touch /etc/myfile"
"/usr/bin/mkdir -p /etc/mydir",
"/usr/bin/touch /etc/myfile"
],
"Environment": [
{
@ -113,12 +113,35 @@ 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
@ -135,8 +158,8 @@ def test_systemd_unit_create(tmp_path, stage_module, unit_type, unit_path, expec
[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