Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression: --isolation=nspawn doesn't in 2.7 #678

Closed
cgwalters opened this issue Dec 14, 2020 · 4 comments
Closed

regression: --isolation=nspawn doesn't in 2.7 #678

cgwalters opened this issue Dec 14, 2020 · 4 comments
Labels

Comments

@cgwalters
Copy link
Contributor

See https://pagure.io/releng/failed-composes/issue/2126

With mock-2.7-1.fc33.noarch, if I do mock --new-chroot --shell I am clearly not running in nspawn. Downgrading to 2.6 fixes it - I again see e.g. container=systemd-nspawn in the environment, and it shows up in machinectl etc.

@cgwalters
Copy link
Contributor Author

I tried to debug this but AFAICS at least as far as the parsing config.py USE_NSPAWN is True...but it somehow gets lost later?

@AdamWill
Copy link
Contributor

copy/pasted from the releng issue:

So my first suspect here is 877b332 , which moves around some of the relevant bits here. In particular there's some weirdness related to a var called USE_NSPAWN. Before that commit, it was only ever declared in mock/py/mockbuild/util.py; a few other things imported it from there. That commit moves some of the logic related to it into mock/py/mockbuild/config.py, but the result is kinda odd. It's still declared in util.py , from where config.py imports it and sets it as a global and does stuff to it. Other things still import it from util, not from config.

I haven't entirely traced out the consequences, but I'd bet a medium sized amount of money that's the problem.

@AdamWill
Copy link
Contributor

yeah, so I'm pretty sure that when config imports USE_NSPAWN from util and runs through various logic to set it, it's only setting config.USE_NSPAWN. Nothing it does changes util.USE_NSPAWN. So far as util itself and anything else that imports USE_NSPAWN from util is concerned, USE_NSPAWN is now just always False (as it's set False at the start of util.py and never changed).

AdamWill added a commit to AdamWill/mock that referenced this issue Dec 14, 2020
877b332 broke this by splitting the code that actually decides
what `USE_NSPAWN` should be into the `config` module...where it
now only set `config.USE_NSPAWN`, but everything that actually
changes behaviour based on the value reads `util.USE_NSPAWN`,
which was now always False.

This feels a bit ugly, but should at least solve the problem in
a safe way, by having the parsing code in `config` update the
value of the actual variable in `util`.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@AdamWill
Copy link
Contributor

#679 should fix this. I did check my work a bit: I hacked up everything in util that depends on the value of USE_NSPAWN to log it. Before that PR, even when running with --new-chroot, they all logged it as false. With the changes from my PR, they all log it as true. If I run mock -r fedora-rawhide-x86_64 --new-chroot --shell I can see a systemd-nspawn command running.

@praiskup praiskup added the bug label Dec 15, 2020
praiskup pushed a commit that referenced this issue Jan 31, 2021
877b332 broke this by splitting the code that actually decides
what `USE_NSPAWN` should be into the `config` module...where it
now only set `config.USE_NSPAWN`, but everything that actually
changes behaviour based on the value reads `util.USE_NSPAWN`,
which was now always False.

This feels a bit ugly, but should at least solve the problem in
a safe way, by having the parsing code in `config` update the
value of the actual variable in `util`.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants