Skip to content

Commit

Permalink
No per-command bootstrap mount leftovers
Browse files Browse the repository at this point in the history
The d4c1269 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: d4c1269
Relates: rpm-software-management#1414
Relates: rpm-software-management#1286
  • Loading branch information
praiskup committed Sep 25, 2024
1 parent c5ba879 commit efc7580
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 3 deletions.
2 changes: 2 additions & 0 deletions mock/py/mockbuild/buildroot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
14 changes: 13 additions & 1 deletion mock/py/mockbuild/mounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions mock/py/mockbuild/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit efc7580

Please sign in to comment.