-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[fix] execute prepareRootfs for a new mount namespace only #1907
base: main
Are you sure you want to change the base?
Conversation
bug was introduced in 91ca331 (cc: @crosbymichael) |
I get that |
Makes sense |
@crosbymichael do you have an opinion what is the right way to proceed? |
@cyphar I'm not really sure about our next step here, do I need to add some validation and raise an error if we have mounts specified with no NEWNS or we're ok to land the diff as is or do we want to wait for @crosbymichael ? |
also, to fix travis somebody needs to update the job:
The root cause is:
|
looking |
What if others depend on this functionality? What is the point of preventing it? |
Each container restart produces additional "leaked" mounts, and eventually you can see in mount(8) output thousands of lines. It's not critical, it doesn't affect containers in any bad way but it seems like a bug for me. |
But givin the configuration, that is expected and should be handled. Atleast that is my POV, what do you think @cyphar ? |
Thanks @mikebrow will rebase! |
I agree with @crosbymichael -- you are asking us to mount things in a chroot setup. This is far less than ideal, and I would tell people to simply not do this (after all, this is not a secure way to set up a container) but I don't see the benefit of disabling it entirely. If there was an underlying security issue (like we were mounting over host paths) then obviously we'd need to do something about it, but I'm not so sure this is as significant of an issue. Though, there is an argument that there are some |
@verm666 my rebase a pr flow: first rebase master $ git checkout master Then checkout your pr's branch and rebase that pr against master: once rebase is complete: |
You need to add a |
(Note that I'm still not in favour of this change -- I'm just telling you what is causing the CI to fail.) |
Without that if condition runc creates a bunch of mounts for non NEWNS containers and don't clean after all.