#3132 Use --scrub for buildroot removal
Closed: Dropped by tkopecek. Opened by tkopecek.

Mock has --scrub all option for some time now so it should be safe to use it instead of our internal buildroot removal process. Double-check which mock version introduced it before implementing.


Is it safe from https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192153 ?
While I'd like to think that rpm has fixed this and will never regress, the consequences of being wrong about that are disastrous.

Metadata Update from @mikem:
- Custom field Size adjusted to None

@praiskup Hi, how the --scrub is implemented with regard to the bugzilla Mike mentioned?

If rpm escapes from the --root directory (intentionally, or by accident), I'm afraid mock has no
way to track the files outside of the chroot. Or perhaps I did not get the question ..., can you elaborate?

Is mock using rpm to do the cleaning? Or is it independent on it? If it uses it it could be affected by the bug. If doesn't we are probably safe from that. Koji internally has this part of code https://pagure.io/koji/blob/master/f/koji/daemon.py#_837 due to the bug which makes it safe and my question is if we can remove it and rely on --scrub to do these things on mock's side without risk of destroying the builder.

Ah. in that case no ... mock is just removing chroot directories:
https://github.com/rpm-software-management/mock/blob/d081bc113e3c6af9b80167592f1dc95f7edd9c58/mock/py/mockbuild/backend.py#L115-L122
https://github.com/rpm-software-management/mock/blob/d081bc113e3c6af9b80167592f1dc95f7edd9c58/mock/py/mockbuild/buildroot.py#L860-L881

Ok, so it still can hit the problem (if rpm/rpmlib is from some reason still running in that moment). I've to test if rpm is still doing that in 2021 :-)

At least mock pays a lot of attention to kill all remaining processes, this is
tested behavior. So if mock is "gently" stopped to have the opportunity,
anything started by mock is stopped or killed (--orphanskill option).

Even processes that attempt to become daemons, and are in given --root:
https://github.com/rpm-software-management/mock/blob/d081bc113e3c6af9b80167592f1dc95f7edd9c58/mock/py/mockbuild/util.py#L175-L180

At least mock pays a lot of attention to kill all remaining processes, this is
tested behavior. So if mock is "gently" stopped to have the opportunity,
anything started by mock is stopped or killed (--orphanskill option).

Even processes that attempt to become daemons, and are in given --root:
https://github.com/rpm-software-management/mock/blob/d081bc113e3c6af9b80167592f1dc95f7edd9c58/mock/py/mockbuild/util.py#L175-L180

@mikem I've tried with current rpm versions and it seems that it properly fails in such case. Are you for moving to mock's cleanup or to stay with current code?

The issue I am concerned about is a tricky one. The way the problem looks in Koji goes like this:

  1. koji uses mock to create a buildroot, which in turn uses rpm
  2. something goes wrong with the build, so koji kills mock
  3. BUT, due some bug or glitch, the rpm process persists
  4. koji cleans the buildroot, since the task is failed
  5. BUT, the rpm process is still running and when the top level chroot dir no longer exists, we encounted RHBZ#192153. The rpm process begins altering files outside of the chroot. This is often catastrophic to the host system.

I do seem to recall that rpm has fixed this, but I'm not 100% sure when or how well tested the fix was. Given the severe consequences of the issue, I think it pays to be cautious here.

Are there practical negative effects to keeping the existing code?

Are there practical negative effects to keeping the existing code?

No, just wanted to clean up some code and leave responsibility on mock.

Metadata Update from @tkopecek:
- Issue close_status updated to: Dropped
- Issue status updated to: Closed (was: Open)

Metadata Update from @tkopecek:
- Issue set to the milestone: None (was: 1.28)

This issue has been migrated to Fedora Forge:
https://forge.fedoraproject.org/koji/koji/issues/3132

Please continue any further discussion there.

Metadata