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

reduce the number of clones by introducing lifetime to namespaces. #197

Merged
merged 2 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions oci_spec/src/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,11 +532,10 @@ pub struct LinuxNamespace {
#[serde(rename = "type")]
/// Type is the type of namespace.
pub typ: LinuxNamespaceType,

#[serde(default, skip_serializing_if = "Option::is_none")]
/// Path is a path to an existing namespace persisted on disk that can be joined and is of the
/// same type
pub path: Option<String>,
pub path: Option<PathBuf>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

}

// Utility function to get default namespaces
Expand Down
14 changes: 7 additions & 7 deletions src/container/builder_impl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anyhow::{Context, Result};
use nix::sched::CloneFlags;
use oci_spec::Spec;
use std::{fs, os::unix::prelude::RawFd, path::PathBuf};

Expand Down Expand Up @@ -54,12 +55,6 @@ impl<'a> ContainerBuilderImpl<'a> {
let linux = self.spec.linux.as_ref().context("no linux in spec")?;
let cgroups_path = utils::get_cgroup_path(&linux.cgroups_path, &self.container_id);
let cmanager = cgroups::common::create_cgroup_manager(&cgroups_path, self.use_systemd)?;
let namespaces: Namespaces = linux
.namespaces
.as_ref()
.context("no namespaces in linux spec")?
.clone()
.into();

// create the parent and child process structure so the parent and child process can sync with each other
let (mut parent, parent_channel) = parent::ParentProcess::new(&self.rootless)?;
Expand Down Expand Up @@ -91,7 +86,12 @@ impl<'a> ContainerBuilderImpl<'a> {
0
});

let init_pid = fork::clone(cb, namespaces.clone_flags)?;
let clone_flags = linux
.namespaces
.as_ref()
.map(|ns| Namespaces::from(ns).clone_flags)
.unwrap_or_else(CloneFlags::empty);
let init_pid = fork::clone(cb, clone_flags)?;
log::debug!("init pid is {:?}", init_pid);

parent.wait_for_child_ready(init_pid)?;
Expand Down
2 changes: 1 addition & 1 deletion src/container/tenant_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ impl TenantContainerBuilder {
let tenant_ns = LinuxNamespaceType::try_from(ns_type)?;
tenant_namespaces.push(LinuxNamespace {
typ: tenant_ns,
path: Some(init_ns.path.to_string_lossy().to_string()),
path: Some(init_ns.path.clone()),
})
}
}
Expand Down
21 changes: 10 additions & 11 deletions src/namespaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ use nix::{
use oci_spec::LinuxNamespace;

/// Holds information about namespaces
pub struct Namespaces {
spaces: Vec<LinuxNamespace>,
pub struct Namespaces<'a> {
spaces: &'a Vec<LinuxNamespace>,
command: Box<dyn Syscall>,
pub clone_flags: CloneFlags,
}

impl From<Vec<LinuxNamespace>> for Namespaces {
fn from(namespaces: Vec<LinuxNamespace>) -> Self {
impl<'a> From<&'a Vec<LinuxNamespace>> for Namespaces<'a> {
fn from(namespaces: &'a Vec<LinuxNamespace>) -> Self {
let clone_flags = namespaces.iter().filter(|ns| ns.path.is_none()).fold(
CloneFlags::empty(),
|mut cf, ns| {
Expand All @@ -43,8 +43,7 @@ impl From<Vec<LinuxNamespace>> for Namespaces {
}
}

impl Namespaces {
/// sets namespaces as defined in structure to calling process
impl<'a> Namespaces<'a> {
pub fn apply_setns(&self) -> Result<()> {
let to_enter: Vec<(CloneFlags, i32)> = self
.spaces
Expand All @@ -53,7 +52,7 @@ impl Namespaces {
.map(|ns| {
let space = CloneFlags::from_bits_truncate(ns.typ as i32);
let fd = fcntl::open(
&*ns.path.as_ref().unwrap().clone(),
&*ns.path.as_ref().unwrap(),
fcntl::OFlag::empty(),
stat::Mode::empty(),
)
Expand Down Expand Up @@ -94,11 +93,11 @@ mod tests {
vec![
LinuxNamespace {
typ: LinuxNamespaceType::Mount,
path: Some("/dev/null".to_string()),
path: Some("/dev/null".into()),
},
LinuxNamespace {
typ: LinuxNamespaceType::Network,
path: Some("/dev/null".to_string()),
path: Some("/dev/null".into()),
},
LinuxNamespace {
typ: LinuxNamespaceType::Pid,
Expand All @@ -118,7 +117,7 @@ mod tests {
#[test]
fn test_namespaces_set_ns() {
let sample_linux_namespaces = gen_sample_linux_namespaces();
let namespaces: Namespaces = sample_linux_namespaces.into();
let namespaces = Namespaces::from(&sample_linux_namespaces);
let test_command: &TestHelperSyscall = namespaces.command.as_any().downcast_ref().unwrap();
assert!(namespaces.apply_setns().is_ok());

Expand All @@ -136,7 +135,7 @@ mod tests {
#[test]
fn test_namespaces_unshare() {
let sample_linux_namespaces = gen_sample_linux_namespaces();
let namespaces: Namespaces = sample_linux_namespaces.into();
let namespaces = Namespaces::from(&sample_linux_namespaces);
assert!(namespaces.apply_unshare(CloneFlags::CLONE_NEWIPC).is_ok());

let test_command: &TestHelperSyscall = namespaces.command.as_any().downcast_ref().unwrap();
Expand Down
31 changes: 14 additions & 17 deletions src/process/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,12 @@ pub struct ContainerInitArgs {
pub fn container_init(args: ContainerInitArgs) -> Result<()> {
let command = &args.syscall;
let spec = &args.spec;
let linux = &spec.linux.as_ref().context("no linux in spec")?;
let namespaces: Namespaces = linux
.namespaces
.as_ref()
.context("no namepsaces in linux spec")?
.clone()
.into();
let linux = spec.linux.as_ref().context("no linux in spec")?;

// need to create the notify socket before we pivot root, since the unix
// domain socket used here is outside of the rootfs of container
let mut notify_socket: NotifyListener = NotifyListener::new(&args.notify_path)?;
let proc = &spec.process.as_ref().context("no process in spec")?;
let proc = spec.process.as_ref().context("no process in spec")?;
let mut envs: Vec<String> = proc.env.as_ref().unwrap_or(&vec![]).clone();
let rootfs = &args.rootfs;
let mut child = args.child;
Expand Down Expand Up @@ -165,7 +160,15 @@ pub fn container_init(args: ContainerInitArgs) -> Result<()> {
}

// join existing namespaces
namespaces.apply_setns()?;
let bind_service = if let Some(ns) = linux.namespaces.as_ref() {
let namespaces = Namespaces::from(ns);
namespaces.apply_setns()?;
namespaces
.clone_flags
.contains(sched::CloneFlags::CLONE_NEWUSER)
} else {
false
};

if let Some(hostname) = spec.hostname.as_ref() {
command.set_hostname(hostname)?;
Expand All @@ -176,14 +179,8 @@ pub fn container_init(args: ContainerInitArgs) -> Result<()> {
}

if args.init {
rootfs::prepare_rootfs(
spec,
rootfs,
namespaces
.clone_flags
.contains(sched::CloneFlags::CLONE_NEWUSER),
)
.with_context(|| "Failed to prepare rootfs")?;
rootfs::prepare_rootfs(spec, rootfs, bind_service)
.with_context(|| "Failed to prepare rootfs")?;

// change the root of filesystem of the process to the rootfs
command
Expand Down
13 changes: 7 additions & 6 deletions src/rootless.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@ pub fn validate(spec: &Spec) -> Result<()> {
bail!("rootless containers require at least one gid mapping")
}

let namespaces: Namespaces = linux
.namespaces
.as_ref()
.context("rootless containers require namespaces in spec")?
.clone()
.into();
let namespaces = Namespaces::from(
linux
.namespaces
.as_ref()
.context("rootless containers require the namespaces.")?,
);

if !namespaces.clone_flags.contains(CloneFlags::CLONE_NEWUSER) {
bail!("rootless containers require the specification of a user namespace");
}
Expand Down