From efc7580b043e155aa099b991cf0d43873181720c Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Wed, 25 Sep 2024 12:45:57 +0200 Subject: [PATCH] No per-command bootstrap mount leftovers The d4c12696a4ecd07695d16f86e595cfb6aa8997f9 commit broke this use-case: $ mock --chain .... $ mock --scrub=chroot $ mock --scrub=bootstrap The problem is that --chain always creates a host-local temporary repository (directory) for built results, which is also bind-mounted to the bootstrap chroot (so the inner packager may use the built results). The commit started modifying bootstrap's dnf.conf, thus it added a new bind-mount that never got unmounted. Complements: d4c12696a4ecd07695d16f86e595cfb6aa8997f9 Relates: #1414 Relates: #1286 --- mock/py/mockbuild/buildroot.py | 2 ++ mock/py/mockbuild/mounts.py | 14 +++++++++++++- mock/py/mockbuild/util.py | 4 ++-- .../umount-temp-dirs-from-bootstrap.bugfix | 7 +++++++ 4 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 releng/release-notes-next/umount-temp-dirs-from-bootstrap.bugfix diff --git a/mock/py/mockbuild/buildroot.py b/mock/py/mockbuild/buildroot.py index 8c34bb711..11efff52b 100644 --- a/mock/py/mockbuild/buildroot.py +++ b/mock/py/mockbuild/buildroot.py @@ -978,6 +978,8 @@ def finalize(self): buildroot (exclusive lock can be acquired) also kill orphan processes, unmount mounts and call postumount hooks. """ + self.mounts.umount_temporary() + if self.tmpdir: for d in self.tmpdir, self.make_chroot_path(self.tmpdir): if os.path.exists(d): diff --git a/mock/py/mockbuild/mounts.py b/mock/py/mockbuild/mounts.py index c5288515a..007317f88 100644 --- a/mock/py/mockbuild/mounts.py +++ b/mock/py/mockbuild/mounts.py @@ -192,6 +192,9 @@ def __init__(self, rootObj): self.user_mounts = [] # mounts injected by user self.bootstrap_mounts = [] + # managed mounts that disappear after each mock command + self.temporary_mounts = [] + # Instead of mounting a fresh procfs and sysfs, we bind mount /proc # and /sys. This avoids problems with kernel restrictions if running # within a user namespace, and is pretty much identical otherwise. @@ -323,6 +326,15 @@ def mountall_managed(self): self.mountall_essential() for m in self.managed_mounts: m.mount() + for m in self.temporary_mounts: + m.mount() + + def umount_temporary(self): + """ + Umount temporary mounts whenever mock command quits (finalize()). + """ + for m in reversed(self.temporary_mounts): + m.umount() @traceLog() def mountall_user(self): @@ -339,7 +351,7 @@ def umountall(self, force=False, nowarn=False): # as long as in every loop at least one umount succeed. failed_old = failed_new failed_new = 0 - for m in reversed(self.managed_mounts + self.user_mounts): + for m in reversed(self.temporary_mounts + self.managed_mounts + self.user_mounts): if m.umount() is False: failed_new += 1 if self._essential_mounted: diff --git a/mock/py/mockbuild/util.py b/mock/py/mockbuild/util.py index 30dd108c4..fa7d43292 100644 --- a/mock/py/mockbuild/util.py +++ b/mock/py/mockbuild/util.py @@ -1009,8 +1009,8 @@ def _fix_cfg(cfg): return mountpoint = bootstrap.make_chroot_path(local_dir) - bootstrap.mounts.add(BindMountPoint(srcpath=local_dir, - bindpath=mountpoint)) + bootstrap.mounts.temporary_mounts.append( + BindMountPoint(srcpath=local_dir, bindpath=mountpoint)) def subscription_redhat_init(opts, uidManager): diff --git a/releng/release-notes-next/umount-temp-dirs-from-bootstrap.bugfix b/releng/release-notes-next/umount-temp-dirs-from-bootstrap.bugfix new file mode 100644 index 000000000..e43d8dd2d --- /dev/null +++ b/releng/release-notes-next/umount-temp-dirs-from-bootstrap.bugfix @@ -0,0 +1,7 @@ +Mock needs to bind-mount (e.g., for `--chain` or for the `--addrepo +file:///local/repo` cases) additional repo directories from host to the +bootstrap chroot. These directories though were not umounted automatically +before - this prevented Mock from doing appropriate `--scrub=bootstrap` if the +set of directories changed between the Mock calls (i.e. `--scrub=bootstrap` +without `--addrepo`). This bug was present in Mock <= 5.6, but after +[issue#1414][] it become exposed to our testsuite.