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

move unshare into chdir #277

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zzzzzzzzzy9
Copy link
Contributor

We don't need clone if we don't need to change current dir.

@github-actions github-actions bot added the C-shim Containerd shim label May 30, 2024
@zzzzzzzzzy9
Copy link
Contributor Author

@Burning1020 @fuweid Thanks for suggestion.
#266

@zzzzzzzzzy9 zzzzzzzzzy9 force-pushed the pull-branch branch 11 times, most recently from e239630 to 87e134e Compare June 25, 2024 09:17
let fs_type = Some("overlay");
let source = Some("overlay");
let workdir =
"/var/lib/containerd-test/io.containerd.snapshotter.v1.overlayfs/snapshots/0/work";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I don't think it's good idea to create test folder like this. Please use /tmp/$RANDOM-LONG-STRING. Thanks.

let target = "/run/containerd/io.containerd.runtime.v2.task/k8s.io/mount-test/rootfs";
std::fs::create_dir_all(target).unwrap();
let current_dir = env::current_dir().unwrap();
std::thread::spawn(move || mount_rootfs(fs_type, source, &options, target).unwrap());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why must it spawn thread to mount it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because this mount_rootfs is called in a new thread by tokio::spawn_blocking, so I think it's better to test mount_rootfs in a spawn thread.

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

Successfully merging this pull request may close these issues.

2 participants