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.

Sign up for GitHub

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

Mimic pinns namespace path layout #1067

Merged
merged 1 commit into from
Feb 3, 2023
Merged

Mimic pinns namespace path layout #1067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,16 @@ linters:
# - wsl
linters-settings:
funlen:
lines: 150
lines: 155
statements: 50
varnamelen:
min-name-length: 1
cyclop:
max-complexity: 30
max-complexity: 35
gocognit:
min-complexity: 45
min-complexity: 50
gocyclo:
min-complexity: 50
nestif:
min-complexity: 15
errcheck:
Expand Down
2 changes: 2 additions & 0 deletions conmon-rs/common/proto/conmon.capnp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ interface Conmon {
namespaces @1 :List(Namespace); # The list of namespaces to unshare.
uidMappings @2 :List(Text); # User ID mappings when unsharing the user namespace.
gidMappings @3 :List(Text); # Group ID mappings when unsharing the user namespace.
basePath @4 :Text; # The root path for storing the namespaces.
podId @5 :Text; # The pod identifier.
}

enum Namespace {
Expand Down
27 changes: 23 additions & 4 deletions conmon-rs/server/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,35 @@ pub struct Config {
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Subcommand)]
/// Possible subcommands.
pub enum Commands {
/// Run pause instead of the server.
/// Run pause, which bind mounts selected namespaces to the local file system.
///
/// If a namespace is not selected by one of the flags, then it will fallback to the host
/// namespace and still create the bind mount to it. All namespaces are mounted to
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this behavior was added in #1064 but something I've just thought of: we will needessly mount a namespace for private namespaces (container level), as we unconditionally do the host mounting, but then it wouldn't be used. I think that's okay and not that much overhead, but it would be more mounts than needed (for instance, kube can't even specify mount namespaces, so it's always private, but we always create a host level mount namespace)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I'm gonna open an issue to track

Copy link
Collaborator

Choose a reason for hiding this comment

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

#1081

/// /var/run/[ipc,pid,net,user,uts]ns/$POD_ID, whereas the POD_ID is being passed from the
/// client.
///
/// Tracking of the pause PID will be done by using a file in /var/run/conmonrs/$POD_ID.pid,
/// which gets removed together with the mounted namespaces if `conmonrs pause` terminates.
///
/// UID and GID mappings are required if unsharing of the user namespace (via `--user`) is
/// selected.
Pause {
#[clap(
env(concat!(prefix!(), "PAUSE_PATH")),
long("path"),
default_value("/var/run"),
env(concat!(prefix!(), "PAUSE_BASE_PATH")),
long("base-path"),
short('p'),
value_name("PATH")
)]
/// The base path for pinning the namespaces.
path: PathBuf,
base_path: PathBuf,

#[clap(
env(concat!(prefix!(), "PAUSE_POD_ID")),
long("pod-id"),
)]
/// The unique pod identifier for referring to the namespaces.
pod_id: String,

#[clap(long("ipc"))]
/// Unshare the IPC namespace.
Expand Down
120 changes: 77 additions & 43 deletions conmon-rs/server/src/pause.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ use std::{
process::{exit, Command},
};
use strum::{AsRefStr, Display, EnumIter, EnumString, IntoEnumIterator, IntoStaticStr};
use tracing::{debug, error, info};
use uuid::Uuid;
use tracing::{debug, info, trace, warn};

/// The main structure for this module.
#[derive(Debug, CopyGetters, Getters)]
pub struct Pause {
#[get = "pub"]
path: PathBuf,
base_path: PathBuf,

#[get = "pub"]
pod_id: String,

#[get = "pub"]
namespaces: Vec<Namespace>,
Expand All @@ -39,21 +41,18 @@ pub struct Pause {
/// The global shared multiple pause instance.
static PAUSE: OnceCell<Pause> = OnceCell::new();

/// The global path for storing bin mounted namespaces.
const PAUSE_PATH: &str = "/var/run/conmonrs";

/// The file path for storing the pause PID.
const PAUSE_PID_FILE: &str = ".pause_pid";

impl Pause {
/// Retrieve the global instance of pause
pub fn init_shared(
base_path: &str,
pod_id: &str,
namespaces: Reader<conmon::Namespace>,
uid_mappings: Vec<String>,
gid_mappings: Vec<String>,
) -> Result<&'static Pause> {
PAUSE.get_or_try_init(|| {
Self::init(namespaces, uid_mappings, gid_mappings).context("init pause")
Self::init(base_path, pod_id, namespaces, uid_mappings, gid_mappings)
.context("init pause")
})
}

Expand All @@ -66,34 +65,39 @@ impl Pause {
pub fn stop(&self) {
info!("Stopping pause");
for namespace in self.namespaces() {
if let Err(e) = namespace.umount(self.path()) {
if let Err(e) = namespace.umount(self.base_path(), self.pod_id()) {
debug!("Unable to umount namespace {namespace}: {:#}", e);
}
}
if let Err(e) = fs::remove_dir_all(self.path()) {
error!(
"Unable to remove pause path {}: {:#}",
self.path().display(),
e
);
}

info!("Killing pause PID: {}", self.pid());
if let Err(e) = kill(self.pid(), Signal::SIGTERM) {
error!("Unable to kill pause PID {}: {:#}", self.pid(), e);
warn!("Unable to kill pause PID {}: {:#}", self.pid(), e);
}

let pause_pid_path = Self::pause_pid_path(self.base_path(), self.pod_id());
if let Err(e) = fs::remove_file(&pause_pid_path) {
debug!(
"Unable to remove pause PID path {}: {:#}",
pause_pid_path.display(),
e
);
}
}

/// Initialize a new pause instance.
fn init(
base_path: &str,
pod_id: &str,
init_namespaces: Reader<conmon::Namespace>,
uid_mappings: Vec<String>,
gid_mappings: Vec<String>,
) -> Result<Self> {
debug!("Initializing pause");

let mut args: Vec<String> = vec![];
let mut args: Vec<String> = vec![format!("--pod-id={pod_id}")];
let mut namespaces = vec![];

for namespace in init_namespaces.iter() {
match namespace? {
conmon::Namespace::Ipc => {
Expand Down Expand Up @@ -138,15 +142,15 @@ impl Pause {
debug!("Pause namespaces: {:?}", namespaces);
debug!("Pause args: {:?}", args);

let path = PathBuf::from(PAUSE_PATH).join(Uuid::new_v4().to_string());
fs::create_dir_all(&path).context("create base path")?;
debug!("Pause base path: {}", path.display());
let base_path = PathBuf::from(base_path);
fs::create_dir_all(&base_path).context("create base path")?;
debug!("Pause base path: {}", base_path.display());

let program = env::args().next().context("no args set")?;
let mut child = Command::new(program)
.arg("pause")
.arg("--path")
.arg(&path)
.arg("--base-path")
.arg(&base_path)
.args(args)
.spawn()
.context("run pause")?;
Expand All @@ -156,24 +160,34 @@ impl Pause {
bail!("exit status not ok: {status}")
}

let pid = fs::read_to_string(path.join(PAUSE_PID_FILE))
let pid = fs::read_to_string(Self::pause_pid_path(&base_path, pod_id))
.context("read pause PID path")?
.trim()
.parse::<u32>()
.context("parse pause PID")?;
info!("Pause PID is: {pid}");

Ok(Self {
path,
base_path,
pod_id: pod_id.to_owned(),
namespaces: Namespace::iter().collect(),
pid: Pid::from_raw(pid as pid_t),
})
}

/// Retrieve the pause PID path for a base and pod ID.
fn pause_pid_path<T: AsRef<Path>>(base_path: T, pod_id: &str) -> PathBuf {
base_path
.as_ref()
.join("conmonrs")
.join(format!("{pod_id}.pid"))
}

#[allow(clippy::too_many_arguments)]
/// Run a new pause instance.
pub fn run<T: AsRef<Path> + Copy>(
path: T,
base_path: T,
pod_id: &str,
ipc: bool,
pid: bool,
net: bool,
Expand Down Expand Up @@ -209,16 +223,22 @@ impl Pause {
let (mut sock_parent, mut sock_child) =
UnixStream::pair().context("create unix socket pair")?;
const MSG: &[u8] = &[1];
let mut res = [0];

match unsafe { fork().context("forking process")? } {
ForkResult::Parent { child } => {
let mut file = File::create(path.as_ref().join(PAUSE_PID_FILE))
.context("create pause PID file")?;
let pause_pid_path = Self::pause_pid_path(base_path, pod_id);
fs::create_dir_all(
pause_pid_path
.parent()
.context("no parent for pause PID path")?,
)
.context("create pause PID parent path")?;
let mut file = File::create(pause_pid_path).context("create pause PID file")?;
write!(file, "{child}").context("write child to pause file")?;

if user {
// Wait for user namespace creation
let mut res = [0];
sock_parent.read_exact(&mut res)?;

// Write mappings
Expand All @@ -229,6 +249,9 @@ impl Pause {
sock_parent.write_all(MSG)?;
}

// Wait for mounts to be created
sock_parent.read_exact(&mut res)?;

exit(0);
}

Expand All @@ -239,7 +262,6 @@ impl Pause {
sock_child.write_all(MSG)?;

// Wait for the mappings to be written
let mut res = [0];
sock_child.read_exact(&mut res)?;

// Set the UID and GID
Expand All @@ -258,12 +280,15 @@ impl Pause {

// We bind all namespaces, if not unshared then we use the host namespace.
for namespace in Namespace::iter() {
namespace.bind(path.as_ref()).context(format!(
namespace.bind(base_path, pod_id).context(format!(
"bind namespace to path: {}",
namespace.path(path).display(),
namespace.path(base_path, pod_id).display(),
))?;
}

// Notify that all mounts are created
sock_child.write_all(MSG)?;

let mut signals = Signals::new(TERM_SIGNALS).context("register signals")?;
signals.forever().next().context("no signal number")?;
Ok(())
Expand Down Expand Up @@ -323,9 +348,15 @@ pub enum Namespace {
}

impl Namespace {
/// Bind the namespace to the provided base path.
pub fn bind<T: AsRef<Path>>(&self, path: T) -> Result<()> {
let bind_path = self.path(path);
/// Bind the namespace to the provided base path and pod ID.
pub fn bind<T: AsRef<Path>>(&self, path: T, pod_id: &str) -> Result<()> {
let bind_path = self.path(path, pod_id);
fs::create_dir_all(
bind_path
.parent()
.context("no parent namespace bind path")?,
)
.context("create namespace parent path")?;
File::create(&bind_path).context("create namespace bind path")?;
let source_path = PathBuf::from("/proc/self/ns").join(self.as_ref());

Expand All @@ -342,14 +373,17 @@ impl Namespace {
}

/// Umount the namespace.
pub fn umount<T: AsRef<Path>>(&self, path: T) -> Result<()> {
let bind_path = self.path(path);
umount(&bind_path).context("umount namespace")
pub fn umount<T: AsRef<Path>>(&self, path: T, pod_id: &str) -> Result<()> {
let bind_path = self.path(path, pod_id);
if let Err(e) = umount(&bind_path) {
trace!("Unable to umount namespace {self}: {:#}", e);
}
fs::remove_file(&bind_path).context("remove namespace bind path")
}

/// Retrieve the bind path of the namespace for the provided base path.
pub fn path<T: AsRef<Path>>(&self, path: T) -> PathBuf {
path.as_ref().join(self.as_ref())
/// Retrieve the bind path of the namespace for the provided base path and pod ID.
pub fn path<T: AsRef<Path>>(&self, path: T, pod_id: &str) -> PathBuf {
path.as_ref().join(format!("{self}ns")).join(pod_id)
}

pub fn to_capnp_namespace(self) -> conmon::Namespace {
Expand Down
19 changes: 14 additions & 5 deletions conmon-rs/server/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,19 @@ impl conmon::Server for Server {
) -> Promise<(), capnp::Error> {
debug!("Got a create namespaces request");
let req = pry!(pry!(params.get()).get_request());
let pod_id = pry_err!(req.get_pod_id());

let span = debug_span!(
"create_namespaces",
uuid = Uuid::new_v4().to_string().as_str()
);
if pod_id.is_empty() {
return Promise::err(Error::failed("no pod ID provided".into()));
}

let span = new_root_span!("create_namespaces", pod_id);
let _enter = span.enter();
pry_err!(Telemetry::set_parent_context(pry!(req.get_metadata())));

let pause = pry_err!(Pause::init_shared(
pry!(req.get_base_path()),
pod_id,
pry!(req.get_namespaces()),
capnp_vec_str!(req.get_uid_mappings()),
capnp_vec_str!(req.get_gid_mappings()),
Expand All @@ -380,7 +384,12 @@ impl conmon::Server for Server {

for (idx, namespace) in pause.namespaces().iter().enumerate() {
let mut ns = namespaces.reborrow().get(pry_err!(idx.try_into()));
ns.set_path(&namespace.path(pause.path()).display().to_string());
ns.set_path(
&namespace
.path(pause.base_path(), pod_id)
.display()
.to_string(),
);
ns.set_type(namespace.to_capnp_namespace());
}

Expand Down
6 changes: 4 additions & 2 deletions conmon-rs/server/src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ impl Server {
}

if let Some(Commands::Pause {
path,
base_path,
pod_id,
ipc,
pid,
net,
Expand All @@ -75,7 +76,8 @@ impl Server {
}) = server.config().command()
{
Pause::run(
path,
base_path,
pod_id,
*ipc,
*pid,
*net,
Expand Down
Loading