Support 'shared' and 'unbindable' rootfs propagations #309
Conversation
Codecov Report
@@ Coverage Diff @@
## main #309 +/- ##
==========================================
- Coverage 77.11% 73.15% -3.96%
==========================================
Files 43 44 +1
Lines 6139 6541 +402
==========================================
+ Hits 4734 4785 +51
- Misses 1405 1756 +351 |
|
@tiqwab Thanks for your PR! Could you add unit tests whenever possible? |
|
Of course. I feel for now it is difficult to add enough unit tests, but I'll try it. |
I know it is difficult to add complete unit tests in this area. So it's okay as long as you can. |
There was a problem hiding this comment.
Overall looks good. Thanks for doing this.
Can you please add error handling and context when you can. I know some of the existing code are not doing this, but we are trying to have better error handling whenever we can :)
You may have to create the unit test using fork and entering into user and mount namespace. Let me know if you run into any troubles.
src/process/init.rs
Outdated
| @@ -248,6 +248,8 @@ pub fn container_init( | |||
| .with_context(|| format!("Failed to chroot to {:?}", rootfs))?; | |||
| } | |||
|
|
|||
| rootfs::adjust_root_mount_propagation(spec)?; | |||
There was a problem hiding this comment.
Can we add context to the error handling? with_context or context.
src/rootfs.rs
Outdated
| @@ -35,6 +38,8 @@ pub fn prepare_rootfs(spec: &Spec, rootfs: &Path, bind_devices: bool) -> Result< | |||
| nix_mount(None::<&str>, "/", None::<&str>, flags, None::<&str>) | |||
| .context("Failed to mount rootfs")?; | |||
|
|
|||
| make_parent_mount_private(rootfs)?; | |||
src/rootfs.rs
Outdated
| } | ||
|
|
||
| /// Change propagation type of rootfs as specified in spec. | ||
| pub fn adjust_root_mount_propagation(spec: &Spec) -> Result<()> { |
There was a problem hiding this comment.
Can we pass in linux rather than the whole spec. Looks like only the linux is used.
|
I added some error contexts at the suggest places and a simple unit test for |
Resolve #256.
This pull request enables youki to handle
sharedandunbindablerootfs propagation.Most of the change in the pull request follows runc implementation, but one difference is that runc actually doesn't pass
linux_rootfs_propagation/linux_rootfs_propagation.tintegration test (it fails in settingsharedorunbindablepropagation type).This pull request added
rootfs::adjust_root_mount_propagationafter pivot_root to set appropriate propagation type at root mount point of container ifsharedorunbindablerootfs propagation is specified. This approach comes from an open pull request of runc.