Skip to content

Commit

Permalink
Split container builder into dedicated init and tenant builders
Browse files Browse the repository at this point in the history 
The current monolithic builder provides options that should only be called
during init and not when creating a tenant and vice versa. This puts the
burden on the user of the builder to know which methods are safe to call.
Now the ContainerBuilder can be used to specify options that are common to
both scenarios and afterwards as_init/as_tenant can be called to provide
scenario specific options. This also simplifies the whole "if init then else"
branching logic during container build.
  • Loading branch information
@Furisto
Furisto committed Jul 4, 2021
1 parent a387a43 commit 959c007
Show file tree
Hide file tree
Showing 7 changed files with 295 additions and 224 deletions.
205 changes: 26 additions & 179 deletions src/container/builder.rs
View file
Original file line number Diff line number Diff line change
@@ -1,78 +1,38 @@
#![allow(unused_imports, unused_variables)]
use crate::command::linux::LinuxCommand;
use std::path::PathBuf;

use std::{
collections::HashMap,
fs,
path::{Path, PathBuf},
};

use anyhow::{bail, Result};
use nix::unistd;
use oci_spec::Spec;

use crate::{command::{Syscall, linux::{self, LinuxCommand}, syscall::create_syscall}, notify_socket::NotifyListener, rootless::{self, lookup_map_binaries, should_use_rootless, Rootless}, tty, utils};
use super::{init_builder::InitContainerBuilder, tenant_builder::TenantContainerBuilder};
pub struct ContainerBuilder {
pub(super) container_id: String,

use super::{builder_impl::ContainerBuilderImpl, Container, ContainerStatus};
pub(super) root_path: PathBuf,

pub struct ContainerBuilder {
// defaults
///
init: bool,
///
use_systemd: bool,
///
syscall: LinuxCommand,
////
root_path: PathBuf,
pub(super) syscall: LinuxCommand,

// required
///
container_id: String,
///
bundle: Option<PathBuf>,
pub(super) pid_file: Option<PathBuf>,

// optional
///
pid_file: Option<PathBuf>,
///
console_socket: Option<PathBuf>,
pub(super) console_socket: Option<PathBuf>,
}

impl ContainerBuilder {
pub fn new_init<P: Into<PathBuf>>(container_id: String, bundle: P) -> Result<Self> {
let bundle = Some(fs::canonicalize(bundle.into())?);
let root_path = PathBuf::from("/run/youki");

Ok(Self {
init: true,
use_systemd: true,
syscall: LinuxCommand,
root_path,
container_id,
bundle,
pid_file: None,
console_socket: None,
})
}

pub fn new_tenant(container_id: String) -> Self {
pub fn new(container_id: String) -> Self {
let root_path = PathBuf::from("/run/youki");

Self {
init: false,
use_systemd: true,
syscall: LinuxCommand,
root_path,
container_id,
bundle: None,
root_path,
syscall: LinuxCommand,
pid_file: None,
console_socket: None,
}
}

pub fn with_systemd(mut self, should_use: bool) -> Self {
self.use_systemd = should_use;
self
pub fn as_tenant(self) -> TenantContainerBuilder {
TenantContainerBuilder::new(self)
}

pub fn as_init<P: Into<PathBuf>>(self, bundle: P) -> InitContainerBuilder {
InitContainerBuilder::new(self, bundle.into())
}

pub fn with_root_path<P: Into<PathBuf>>(mut self, path: P) -> Self {
Expand All @@ -89,129 +49,14 @@ impl ContainerBuilder {
self.console_socket = Some(path.into());
self
}

pub fn with_env(mut self, env: HashMap<String, String>) -> Self {
todo!();
}

pub fn with_cwd<P: Into<PathBuf>>(mut self, path: P) -> Self {
todo!();
}

pub fn with_container_command(mut self, command: Vec<String>) -> Self {
todo!();
}

pub fn build(mut self) -> Result<()> {
let container_dir = self.prepare_container_dir()?;
let spec = self.load_and_safeguard_spec(&container_dir)?;
unistd::chdir(&*container_dir)?;

let container = if self.init {
Some(self.create_container_state(&container_dir)?)
} else {
None
};

let notify_socket: NotifyListener = NotifyListener::new(&container_dir)?;
// convert path of root file system of the container to absolute path
let rootfs = fs::canonicalize(&spec.root.path)?;

// if socket file path is given in commandline options,
// get file descriptors of console socket
let csocketfd = if let Some(console_socket) = &self.console_socket {
Some(tty::setup_console_socket(&container_dir, console_socket)?)
} else {
None
};

let rootless = self.is_rootless_required(&spec)?;

let mut builder_impl = ContainerBuilderImpl {
init: self.init,
use_systemd: self.use_systemd,
root_path: self.root_path,
container_id: self.container_id,
pid_file: self.pid_file,
syscall: self.syscall,
console_socket: csocketfd,
rootless,
container_dir,
spec,
rootfs,
notify_socket,
container,
};

builder_impl.create()?;
Ok(())
}

fn prepare_container_dir(&mut self) -> Result<PathBuf> {
let container_dir = self.root_path.join(&self.container_id);
log::debug!("container directory will be {:?}", container_dir);

match (self.init, container_dir.exists()) {
(true, true) => bail!("container {} already exists", self.container_id),
(true, false) => utils::create_dir_all(&container_dir)?,
(false, true) => {}
(false, false) => bail!("container {} does not exist", self.container_id),
}

Ok(container_dir)
}

fn load_and_safeguard_spec(&self, container_dir: &Path) -> Result<Spec> {
let spec_path = if self.init {
let config_path = self.bundle.as_ref().unwrap().join("config.json");
fs::copy(&config_path, container_dir.join("config.json"))?;
config_path
} else {
container_dir.join("config.json")
};

let spec = oci_spec::Spec::load(spec_path)?;
Ok(spec)
}

fn is_rootless_required(&self, spec: &Spec) -> Result<Option<Rootless>> {
let linux = spec.linux.as_ref().unwrap();

let rootless = if should_use_rootless() {
log::debug!("rootless container should be created");
log::warn!(
"resource constraints and multi id mapping is unimplemented for rootless containers"
);
rootless::validate(spec)?;
let mut rootless = Rootless::from(linux);
if let Some((uid_binary, gid_binary)) = lookup_map_binaries(linux)? {
rootless.newuidmap = Some(uid_binary);
rootless.newgidmap = Some(gid_binary);
}
Some(rootless)
} else {
None
};

Ok(rootless)
}

fn create_container_state(&self, container_dir: &Path) -> Result<Container> {
let container = Container::new(
&self.container_id,
ContainerStatus::Creating,
None,
self.bundle.as_ref().unwrap().to_str().unwrap(),
&container_dir,
)?;
container.save()?;
Ok(container)
}
}

#[cfg(test)]
mod tests {
use std::collections::HashMap;

use super::*;
use anyhow::Result;

// required values (must be specified in new...)
// - create
Expand Down Expand Up @@ -250,11 +95,12 @@ mod tests {
let console_socket = PathBuf::from("");
let root_path = PathBuf::from("");

let container = ContainerBuilder::new_init(id, bundle)?
let container = ContainerBuilder::new(id)
.with_pid_file(pid_file) // optional
.with_console_socket(console_socket) //optional
.with_systemd(false) // overwrite default
.with_root_path(root_path) // overwrite default
.as_init(bundle)
.with_systemd(false)
.build()?;

Ok(())
Expand All @@ -268,9 +114,10 @@ mod tests {
let cwd = PathBuf::from("");
let env = HashMap::new();

let container = ContainerBuilder::new_tenant(id)
let container = ContainerBuilder::new(id)
.with_pid_file(pid_file)
.with_console_socket(console_socket)
.as_tenant()
.with_cwd(cwd)
.with_env(env)
.with_container_command(vec!["sleep".to_owned(), "9001".to_owned()])
Expand Down
30 changes: 22 additions & 8 deletions src/container/builder_impl.rs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
use std::{path::PathBuf};
use std::path::PathBuf;

use anyhow::{Result};
use anyhow::Result;
use nix::{
sched,
unistd::{Gid, Uid},
};
use oci_spec::Spec;

use crate::{cgroups, command::{Syscall, linux::LinuxCommand}, namespaces::Namespaces, notify_socket::NotifyListener, process::{Process, fork, setup_init_process}, rootless::{Rootless}, stdio::FileDescriptor, tty, utils};
use crate::{
cgroups,
command::{linux::LinuxCommand, Syscall},
namespaces::Namespaces,
notify_socket::NotifyListener,
process::{fork, setup_init_process, Process},
rootless::Rootless,
stdio::FileDescriptor,
tty, utils,
};

use super::{Container, ContainerStatus};

pub(super) struct ContainerBuilderImpl{
pub(super) struct ContainerBuilderImpl {
pub init: bool,
pub syscall: LinuxCommand,
pub use_systemd: bool,
Expand Down Expand Up @@ -80,7 +89,12 @@ impl ContainerBuilderImpl {
// This is actually the child process after fork
Process::Init(mut init) => {
// prepare process
setup_init_process(&self.spec, &self.syscall, self.rootfs.clone(), &namespaces)?;
setup_init_process(
&self.spec,
&self.syscall,
self.rootfs.clone(),
&namespaces,
)?;
init.ready()?;
self.notify_socket.wait_for_container_start()?;
// actually run the command / program to be run in container
Expand All @@ -91,9 +105,9 @@ impl ContainerBuilderImpl {
if let Some(container) = &self.container {
// the command / program is done executing
container
.refresh_state()?
.update_status(ContainerStatus::Stopped)
.save()?;
.refresh_state()?
.update_status(ContainerStatus::Stopped)
.save()?;
}

Ok(Process::Init(init))
Expand Down
Loading

0 comments on commit 959c007

Please sign in to comment.