debian-forge/osbuild
Michael Vogt 2a17756f45 Revert "runners: clean up temp files before exiting the runner"
This reverts commit bc04bfc366.

The `remove_tmpfiles()` helper is nice but it is also problematic
because it creates extra output after the command was run and
created output. E.g. a test failure on centos stream9 [0]
```
            r = root.run(["stat", "--format=%a", "/var/tmp"], monitor)
            assert r.returncode == 0
>           assert r.stdout.strip().split("\n")[-1] == "1777"
E           AssertionError: assert '/usr/lib/tmp... such process' == '1777'
E
E             - 1777
E             + /usr/lib/tmpfiles.d/rpcbind.conf:2: Failed to resolve user 'rpc': No such process
```
Here the output from "stat" is not the last output because the
rempve_tmpfiles runs `systemd-tmpfiles --clean --remove` which
produces some noisy output after stat was run.

This was found by @thozza (thanks!) and discussed in osbuild PR#1785.

There are various ways to fix this, the one is to use the
`--graceful` option of systemd-tmpfiles. However that only got added in
systemd v256 and centos-stream9 has v252 so that is sadly not an option.

Plus even when avaialble it will produce some informational output like
```
All rules containing unresolvable specifiers will be skipped.
```

Another way would be to sent the output from systemd-tmpfiles cleanup
to /dev/null. Not really great as we will not know about real problems
or warnings that we should care about.

None of the option above is good. So I started looking at the tmpfiles.d
rules and the cleanup and why we are doing it. It was added relatively
recently in https://github.com/osbuild/osbuild/pull/1458 and after
some medidiation not having it seems to do no harm (details below). The
tl;dr is that the buildroot is created inside bubblewrap and the
dirs that `--clean` and `--remove` touch are already tmpdirs created
just for the buildroot so the cleanup in the runner is redundant
(and because the cleanup is now run for each buidlroot.run() command
there *might* be unintended conequences but the current rules seem
to not have any).

In detail, the tmpfiles_cleanup() does two things:
1. `--clean`
It will remove files that are older then the given age
in tmpfiles.d. The tmpfiles in centos9 give me the following ages:
```
$ systemd-tmpfiles --cat-config|grep -E '[0-9]+d$'
d /var/lib/systemd/pstore 0755 root root 14d
d /var/lib/systemd/coredump 0755 root root 3d
q /tmp 1777 root root 10d
q /var/tmp 1777 root root 30d
D! /tmp/.X11-unix 1777 root root 10d
D! /tmp/.ICE-unix 1777 root root 10d
D! /tmp/.XIM-unix 1777 root root 10d
D! /tmp/.font-unix 1777 root root 10d
```
Given that we run our commands inside a bubblewrap environment and
give it a fresh /run, /tmp, /var [1] there really should be no long
lived things and even if there are they are cleaned up from the
buildroot itself

2. `--remove`
It will remove files marked for removal in tmpdfiles.d. Running
it on a centos9 env it yields for me:
```
$ systemd-tmpfiles --cat-config|grep -E '^[rRD]'
R /var/tmp/dnf*/locks/*
r /var/cache/dnf/download_lock.pid
r /var/cache/dnf/metadata_lock.pid
r /var/lib/dnf/rpmdb_lock.pid
r /var/log/log_lock.pid
r! /forcefsck
r! /fastboot
r! /forcequotacheck
D! /var/lib/containers/storage/tmp 0700 root root
D! /run/podman 0700 root root
D! /var/lib/cni/networks
R! /var/tmp/container_images*
D     /run/rpcbind 0700  rpc  rpc  -  -
D /run/sudo/ts 0700 root root
R! /tmp/systemd-private-*
R! /var/tmp/systemd-private-*
r! /var/lib/systemd/coredump/.#*
D! /tmp/.X11-unix 1777 root root 10d
D! /tmp/.ICE-unix 1777 root root 10d
D! /tmp/.XIM-unix 1777 root root 10d
D! /tmp/.font-unix 1777 root root 10d
r! /tmp/.X[0-9]*-lock
```
which is also covered by the bwrap cleanup.

[0] https://artifacts.dev.testing-farm.io/2d07b8f3-5f52-4e61-b1fa-5328a0ff1058/#artifacts-/plans/unit-tests
[1] https://github.com/osbuild/osbuild/blob/main/osbuild/buildroot.py#L218
2024-05-20 11:55:24 -07:00
..
formats format/v2: add new "partition" field to "describe_mount" 2023-12-22 10:18:29 -05:00
testutil stages: fix test values for "images" inputs 2024-04-29 14:11:44 +02:00
util Revert "runners: clean up temp files before exiting the runner" 2024-05-20 11:55:24 -07:00
__init__.py Post release version bump 2024-05-08 08:13:38 +00:00
__main__.py osbuild: run isort on all files 2022-09-12 13:32:51 +02:00
api.py Fix errors reported by new version of mypy 2023-04-12 11:57:18 +02:00
buildroot.py osbuild: make the entire /etc/selinux avaialble for the buildroot 2024-03-22 11:24:35 +01:00
devices.py osbuild: error when {Device,Mount} is modified after creation 2024-01-19 02:54:26 +01:00
host.py tox: add tox 2023-08-01 15:01:13 +02:00
inputs.py osbuild: error when {Device,Mount} is modified after creation 2024-01-19 02:54:26 +01:00
loop.py Revert "osbuild: ensure loop.Loop() has the required device node" 2024-03-13 22:16:51 +02:00
main_cli.py osbuild: fix newline issue with final output 2024-04-12 13:41:04 +02:00
meta.py meta: automatically allow devices as input in the stages schemas 2024-04-16 08:04:43 +02:00
mixins.py osbuild: error when {Device,Mount} is modified after creation 2024-01-19 02:54:26 +01:00
monitor.py monitor: make origin a setter 2024-03-12 16:44:12 +01:00
mounts.py mounts/ostree.deployment: support deployments on mount 2024-01-31 16:46:01 -05:00
objectstore.py objectstore: add new skip_preserve_owner to Object.export() 2023-12-20 09:28:39 +01:00
pipeline.py osbuild: add workaround to integrate sources into progress reporting 2024-03-12 16:44:12 +01:00
remoteloop.py osbuild: reuse loop_for_fd() in LoopServer 2024-03-11 10:56:49 +01:00
sources.py sources: transform() is only used in the curl sources, remove from ABC 2024-03-19 14:21:57 +01:00