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

fix(libcontainer) no_pivot args is not used #2923

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

xujihui1985
Copy link

fix the problem that no_pivot args is not used when create container when we prepare rootfs with chroot, we should move_mount the rootfs before chroot, otherwise we will not able to use the new rootfs when exec into the container

@xujihui1985
Copy link
Author

@udzura This PR addresses the issue where the no_pivot argument is not recognized. This argument is crucial in environments where the pivot_root syscall is not permitted, such as when running containers within containers.

@udzura
Copy link
Contributor

udzura commented Sep 22, 2024

I understand what this PR aim to do 👍 But I have no privilege to merge this. I guess you've mistaken me for someone!

@xujihui1985
Copy link
Author

I understand what this PR aim to do 👍 But I have no privilege to merge this. I guess you've mistaken me for someone!

@udzura sorry for bother you, could you help to involve someone that is able to have a look at this PR? Really appreciate.

@utam0k
Copy link
Member

utam0k commented Sep 23, 2024

I'll check this PR. Thanks for your first contribution!

@utam0k
Copy link
Member

utam0k commented Sep 23, 2024

@xujihui1985 May I ask you to add an integration test for this PR?
https://containers.github.io/youki/developer/e2e/rust_oci_test.html

@xujihui1985
Copy link
Author

@xujihui1985 May I ask you to add an integration test for this PR?

https://containers.github.io/youki/developer/e2e/rust_oci_test.html

sure, I'd like to learn how to add integrate test

@utam0k
Copy link
Member

utam0k commented Sep 23, 2024

sure, I'd like to learn how to add integrate test

I can help you if you need ;)

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Sep 23, 2024

Hey @xujihui1985 Thanks for your contribution! May I ask you to also take a look at #2597 , which also deals with no_pivot and the create and run commands. Can you check if you need to pick up any changes from there to here? Thanks.

@xujihui1985
Copy link
Author

Hey @xujihui1985 Thanks for your contribution! May I ask you to also take a look at #2597 , which also deals with no_pivot and the create and run commands. Can you check if you need to pick up any changes from there to here? Thanks.

Hi @YJDoc2 I just noticed a similar pull request that addresses this issue and is almost identical to my approach. However, the only problem is that the chroot implementation is incorrect. I can build upon his PR, fix the chroot issue, and include credit for his work.

Vanient and others added 2 commits September 27, 2024 09:04
Move the rootfs to the root of the host filesystem before chrooting,
this is equivalent to pivot_root, if don't move mount first, we will
not see the new rootfs when exec into the container

Signed-off-by: xujihui1985 <[email protected]>
@xujihui1985
Copy link
Author

@YJDoc2 I have pick the change from commit #2597, I will continue work on integration test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants