From 19fd2c58a945fc614477d42acb0206d2f29c7763 Mon Sep 17 00:00:00 2001 From: Tommy McCormick Date: Wed, 29 Jul 2026 21:30:39 -0400 Subject: [PATCH] refactor(cells): async cell allocate/free Replace the `proxy_if_needed!` macro and make `CellsCache` async. `broadcast_kill` stays the only sync function, as it is called by `Drop`. Likewise, `do_free!` is replaced by `teardown_process_and_cgroup`. This now frees cells concurrently, so a host with many cells tears down in roughly the time of the slowest cell rather than the sum of all of them. No behavior change is expected. --- auraed/src/cells/cell_service/cell_service.rs | 8 +- auraed/src/cells/cell_service/cells/cell.rs | 120 ++++--- auraed/src/cells/cell_service/cells/cells.rs | 292 ++++++++++-------- .../cells/cell_service/cells/cells_cache.rs | 54 +--- .../cell_service/cells/cgroups/cgroup.rs | 49 +-- auraed/src/cells/cell_service/cells/mod.rs | 2 +- 6 files changed, 264 insertions(+), 261 deletions(-) diff --git a/auraed/src/cells/cell_service/cell_service.rs b/auraed/src/cells/cell_service/cell_service.rs index 96a16de6a..3479e063f 100644 --- a/auraed/src/cells/cell_service/cell_service.rs +++ b/auraed/src/cells/cell_service/cell_service.rs @@ -144,10 +144,10 @@ impl CellService { let mut cells = self.cells.lock().await; - let cell = cells.allocate(cell_name, cell_spec)?; + let cell = cells.allocate(cell_name, cell_spec).await?; Ok(CellServiceAllocateResponse { - cell_name: cell.name().clone().to_string(), + cell_name: cell.name().to_string(), cgroup_v2: cell.v2().expect("allocated cell returns `Some`"), }) } @@ -170,7 +170,7 @@ impl CellService { let mut cells = self.cells.lock().await; - cells.free(&cell_name)?; + cells.free(&cell_name).await?; Ok(CellServiceFreeResponse::default()) } @@ -180,7 +180,7 @@ impl CellService { let mut cells = self.cells.lock().await; // Attempt to gracefully free all cells - cells.broadcast_free(); + cells.broadcast_free().await; // The cells that remain failed to shut down for some reason. // Forcefully kill any remaining cells that failed to shut down diff --git a/auraed/src/cells/cell_service/cells/cell.rs b/auraed/src/cells/cell_service/cells/cell.rs index 8abe437b6..a29108fd3 100644 --- a/auraed/src/cells/cell_service/cells/cell.rs +++ b/auraed/src/cells/cell_service/cells/cell.rs @@ -18,45 +18,13 @@ use super::{ nested_auraed::NestedAuraed, }; use client::AuraeSocket; +use std::io; use tracing::info; // TODO https://github.com/aurae-runtime/aurae/issues/199 && // aurae.io/signals, which is more accurate // TODO nested auraed should proxy (bus) POSIX signals to child executables -macro_rules! do_free { - ( - $self:ident, - $nested_auraed_call:ident($($nested_auraed_call_arg:ident),*), - $($children_call:ident($($children_call_arg:ident),*)),* - ) => {{ - if let CellState::Allocated { cgroup, nested_auraed, children } = - &mut $self.state - { - $(children.$children_call($($children_call_arg),*));*; - - let _exit_status = nested_auraed - .$nested_auraed_call($($nested_auraed_call_arg),*) - .map_err(|e| { - CellsError::FailedToKillCellChildren { - cell_name: $self.cell_name.clone(), - source: e, - } - })?; - - cgroup.delete().map_err(|e| CellsError::FailedToFreeCell { - cell_name: $self.cell_name.clone(), - source: e, - })?; - } - - // set cell state to freed, independent of the current state - $self.state = CellState::Freed; - - Ok(()) - }}; -} - // We should not be able to change a cell after it has been created. // You must free the cell and create a new one if you want to change anything about the cell. // In order to facilitate that immutability: @@ -81,10 +49,30 @@ impl Cell { Self { cell_name, spec: cell_spec, state: CellState::Unallocated } } + /// Signal the nested auraed and delete the cgroup. Shared by + /// `free` (graceful `shutdown`) and `kill` (forceful `kill`). + fn teardown_process_and_cgroup( + cell_name: &CellName, + cgroup: &Cgroup, + signal: impl FnOnce() -> io::Result, + ) -> Result<()> { + let _exit_status = + signal().map_err(|e| CellsError::FailedToKillCellChildren { + cell_name: cell_name.clone(), + source: e, + })?; + + cgroup.delete().map_err(|e| CellsError::FailedToFreeCell { + cell_name: cell_name.clone(), + source: e, + })?; + Ok(()) + } + /// Creates the underlying cgroup. /// Does nothing if [Cell] has been previously allocated. // Here is where we define the "default" cgroup parameters for Aurae cells - pub fn allocate(&mut self) -> Result<()> { + pub(crate) async fn allocate(&mut self) -> Result<()> { let CellState::Unallocated = &self.state else { return Ok(()); }; @@ -141,15 +129,39 @@ impl Cell { /// The [Cell::state] will be set to [CellState::Freed] regardless of it's state prior to this call. /// /// A [Cell] should never be reused once in the [CellState::Freed] state. - pub fn free(&mut self) -> Result<()> { - do_free!(self, shutdown(), broadcast_free()) + pub(crate) async fn free(&mut self) -> Result<()> { + if let CellState::Allocated { cgroup, nested_auraed, children } = + &mut self.state + { + children.broadcast_free().await; + Self::teardown_process_and_cgroup(&self.cell_name, cgroup, || { + nested_auraed.shutdown() + })?; + } + + // set cell state to freed, independent of the current state + self.state = CellState::Freed; + Ok(()) } /// Sends a [SIGKILL] to the [NestedAuraed], and deletes the underlying cgroup. /// The [Cell::state] will be set to [CellState::Freed] regardless of it's state prior to this call. /// A [Cell] should never be reused once in the [CellState::Freed] state. + /// + /// Stays synchronous so [`Drop`] can call it. pub fn kill(&mut self) -> Result<()> { - do_free!(self, kill(), broadcast_kill()) + if let CellState::Allocated { cgroup, nested_auraed, children } = + &mut self.state + { + children.broadcast_kill(); + Self::teardown_process_and_cgroup(&self.cell_name, cgroup, || { + nested_auraed.kill() + })?; + } + + // set cell state to freed, independent of the current state + self.state = CellState::Freed; + Ok(()) } pub fn client_socket(&self) -> Result { @@ -182,7 +194,7 @@ impl Cell { } impl CellsCache for Cell { - fn allocate( + async fn allocate( &mut self, cell_name: CellName, cell_spec: CellSpec, @@ -193,17 +205,17 @@ impl CellsCache for Cell { }); }; - children.allocate(cell_name, cell_spec) + children.allocate(cell_name, cell_spec).await } - fn free(&mut self, cell_name: &CellName) -> Result<()> { + async fn free(&mut self, cell_name: &CellName) -> Result<()> { let CellState::Allocated { children, .. } = &mut self.state else { return Err(CellsError::CellNotAllocated { cell_name: self.cell_name.clone(), }); }; - children.free(cell_name) + children.free(cell_name).await } fn get(&mut self, cell_name: &CellName, f: F) -> Result @@ -231,22 +243,6 @@ impl CellsCache for Cell { children.get_all(f) } - - fn broadcast_free(&mut self) { - let CellState::Allocated { children, .. } = &mut self.state else { - return; - }; - - children.broadcast_free() - } - - fn broadcast_kill(&mut self) { - let CellState::Allocated { children, .. } = &mut self.state else { - return; - }; - - children.broadcast_kill() - } } impl Drop for Cell { @@ -265,8 +261,8 @@ mod tests { use crate::{AURAED_RUNTIME, AuraedRuntime}; use test_helpers::*; - #[test] - fn test_cant_unfree() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_cant_unfree() { skip_if_not_root!("test_cant_unfree"); // Docker's seccomp security profile (https://docs.docker.com/engine/security/seccomp/) blocks clone skip_if_seccomp!("test_cant_unfree"); @@ -277,14 +273,14 @@ mod tests { let mut cell = Cell::new(cell_name, CellSpec::new_for_tests()); assert!(matches!(cell.state, CellState::Unallocated)); - cell.allocate().expect("failed to allocate"); + cell.allocate().await.expect("failed to allocate"); assert!(matches!(cell.state, CellState::Allocated { .. })); - cell.free().expect("failed to free"); + cell.free().await.expect("failed to free"); assert!(matches!(cell.state, CellState::Freed)); // Calling allocate again should do nothing - cell.allocate().expect("failed to allocate 2"); + cell.allocate().await.expect("failed to allocate 2"); assert!(matches!(cell.state, CellState::Freed)); } } diff --git a/auraed/src/cells/cell_service/cells/cells.rs b/auraed/src/cells/cell_service/cells/cells.rs index ca52a22ef..e608e64b0 100644 --- a/auraed/src/cells/cell_service/cells/cells.rs +++ b/auraed/src/cells/cell_service/cells/cells.rs @@ -18,29 +18,6 @@ use crate::cells::cell_service::cells::cells_cache::CellsCache; use std::collections::HashMap; use tracing::warn; -macro_rules! proxy_if_needed { - ($self:ident, $cell_name:ident, $call:ident($($arg:ident),*), $expr:expr) => { - if !$cell_name.is_child($self.parent.as_ref()) { - // we are not in the direct parent - let child_cell_name = match &$self.parent { - None => $cell_name.to_root(), - Some(parent) => parent.to_child(&$cell_name).expect("child CellName"), - }; - - // we require that all ancestor cells exist - let Some(child) = $self.cache.get_mut(&child_cell_name) else { - return Err(CellsError::CellNotFound { - cell_name: child_cell_name, - }) - }; - - CellsCache::$call(child, $($arg),*) - } else { - $expr - } - }; -} - type Cache = HashMap; /// The in-memory cache of cells ([Cell]) created with Aurae. @@ -61,77 +38,134 @@ impl Cells { Self { parent: Some(parent), ..Self::default() } } - fn allocate( + /// If `cell_name` does not sit directly under our `parent`, return the + /// name of the immediate child cell the operation should be forwarded + /// to. Returns `None` when the name belongs directly to this + /// collection (the caller handles it locally). + fn child_to_forward(&self, cell_name: &CellName) -> Option { + if cell_name.is_child(self.parent.as_ref()) { + return None; + } + + Some(match &self.parent { + None => cell_name.to_root(), + Some(parent) => parent.to_child(cell_name).expect("child CellName"), + }) + } + + pub(crate) async fn allocate( &mut self, cell_name: CellName, cell_spec: CellSpec, ) -> Result<&Cell> { - proxy_if_needed!(self, cell_name, allocate(cell_name, cell_spec), { - if Cgroup::exists(&cell_name) { - return if self.cache.contains_key(&cell_name) { - Err(CellsError::CellExists { cell_name }) - } else { - Err(CellsError::CgroupIsNotACell { - cell_name: cell_name.clone(), - }) - }; - } + // If the requested name doesn't sit directly under our `parent`, + // walk down to the right child cell and forward the call. + if let Some(child_cell_name) = self.child_to_forward(&cell_name) { + let Some(child) = self.cache.get_mut(&child_cell_name) else { + return Err(CellsError::CellNotFound { + cell_name: child_cell_name, + }); + }; + return Box::pin(CellsCache::allocate(child, cell_name, cell_spec)) + .await; + } - // From here, we know the cgroup doesn't exist, so remove from cache if it does - if let Some(_removed) = self.cache.remove(&cell_name) { - // TODO: Should we not remove the cell (that has no cgroup) from the cache and - // force the user to call Free? Free will also return an error, but we may be - // calling other logic in free that we want to run. - warn!( - "Found cached cell ('{cell_name}') without cgroup. Did you forget to call free on the cell?" - ); - } + if Cgroup::exists(&cell_name) { + return if self.cache.contains_key(&cell_name) { + Err(CellsError::CellExists { cell_name }) + } else { + Err(CellsError::CgroupIsNotACell { + cell_name: cell_name.clone(), + }) + }; + } - let cell = self - .cache - .entry(cell_name.clone()) - .or_insert_with(|| Cell::new(cell_name, cell_spec)); + // From here, we know the cgroup doesn't exist, so remove from cache + // if it does + if let Some(_removed) = self.cache.remove(&cell_name) { + // TODO: Should we not remove the cell (that has no cgroup) from + // the cache and force the user to call Free? Free will also + // return an error, but we may be calling other logic in + // free that we want to run. + warn!( + "Found cached cell ('{cell_name}') without cgroup. Did you forget to call free on the cell?" + ); + } - // TODO: Should we remove the cell from the cache here if the call to allocate fails? - cell.allocate()?; + let cell = self + .cache + .entry(cell_name.clone()) + .or_insert_with(|| Cell::new(cell_name, cell_spec)); - Ok(cell) - }) + // TODO: Should we remove the cell from the cache here if the call to + // allocate fails? + cell.allocate().await?; + + Ok(cell) } - fn free(&mut self, cell_name: &CellName) -> Result<()> { - proxy_if_needed!(self, cell_name, free(cell_name), { - self.handle_cgroup_does_not_exist(cell_name)?; - self.get_mut(cell_name, |cell| cell.free())?; + pub(crate) async fn free(&mut self, cell_name: &CellName) -> Result<()> { + if let Some(child_cell_name) = self.child_to_forward(cell_name) { + let Some(child) = self.cache.get_mut(&child_cell_name) else { + return Err(CellsError::CellNotFound { + cell_name: child_cell_name, + }); + }; + return Box::pin(CellsCache::free(child, cell_name)).await; + } + + self.handle_cgroup_does_not_exist(cell_name)?; + + let res = match self.cache.get_mut(cell_name) { + Some(cell) => cell.free().await, + None => { + return Err(CellsError::CgroupIsNotACell { + cell_name: cell_name.clone(), + }); + } + }; + + if matches!(res, Err(CellsError::CellNotAllocated { .. })) { let _ = self.cache.remove(cell_name); - Ok(()) - }) + return res; + } + + res?; + let _ = self.cache.remove(cell_name); + Ok(()) } - fn get(&mut self, cell_name: &CellName, f: F) -> Result + pub(crate) fn get(&mut self, cell_name: &CellName, f: F) -> Result where F: Fn(&Cell) -> Result, { - proxy_if_needed!(self, cell_name, get(cell_name, f), { - self.handle_cgroup_does_not_exist(cell_name)?; - - let Some(cell) = self.cache.get(cell_name) else { - return Err(CellsError::CgroupIsNotACell { - cell_name: cell_name.clone(), + if let Some(child_cell_name) = self.child_to_forward(cell_name) { + let Some(child) = self.cache.get_mut(&child_cell_name) else { + return Err(CellsError::CellNotFound { + cell_name: child_cell_name, }); }; + return CellsCache::get(child, cell_name, f); + } - let res = f(cell); + self.handle_cgroup_does_not_exist(cell_name)?; - if matches!(res, Err(CellsError::CellNotAllocated { .. })) { - let _ = self.cache.remove(cell_name); - } + let Some(cell) = self.cache.get(cell_name) else { + return Err(CellsError::CgroupIsNotACell { + cell_name: cell_name.clone(), + }); + }; - res - }) + let res = f(cell); + + if matches!(res, Err(CellsError::CellNotAllocated { .. })) { + let _ = self.cache.remove(cell_name); + } + + res } - fn get_all(&self, f: F) -> Result>> + pub(crate) fn get_all(&self, f: F) -> Result>> where F: Fn(&Cell) -> Result, { @@ -155,27 +189,6 @@ impl Cells { .collect()) } - fn get_mut(&mut self, cell_name: &CellName, f: F) -> Result - where - F: FnOnce(&mut Cell) -> Result, - { - self.handle_cgroup_does_not_exist(cell_name)?; - - let Some(cell) = self.cache.get_mut(cell_name) else { - return Err(CellsError::CgroupIsNotACell { - cell_name: cell_name.clone(), - }); - }; - - let res = f(cell); - - if matches!(res, Err(CellsError::CellNotAllocated { .. })) { - let _ = self.cache.remove(cell_name); - } - - res - } - fn handle_cgroup_does_not_exist( &mut self, cell_name: &CellName, @@ -195,23 +208,31 @@ impl Cells { Err(CellsError::CgroupNotFound { cell_name: cell_name.clone() }) } - fn broadcast_free(&mut self) { - let freed_cells = self.do_broadcast(|cell| cell.free()); - - for cell_name in freed_cells { - let _ = self.cache.remove(&cell_name); + /// Free all cells concurrently, allowing each to perform its own netlink + process-reap work + pub(crate) async fn broadcast_free(&mut self) { + let results = + futures::future::join_all(self.cache.values_mut().map(|cell| { + let name = cell.name().clone(); + async move { (name, cell.free().await.is_ok()) } + })) + .await; + + for (cell_name, freed) in results { + if freed { + let _ = self.cache.remove(&cell_name); + } } } - fn broadcast_kill(&mut self) { - let killed_cells = self.do_broadcast(|cell| cell.kill()); + pub(crate) fn broadcast_kill(&mut self) { + let killed_cells = self.do_broadcast_sync(|cell| cell.kill()); for cell_name in killed_cells { let _ = self.cache.remove(&cell_name); } } - fn do_broadcast(&mut self, f: F) -> Vec + fn do_broadcast_sync(&mut self, f: F) -> Vec where F: Fn(&mut Cell) -> Result<()>, { @@ -220,11 +241,13 @@ impl Cells { .flat_map(|cell| { f(cell)?; - // We clone here because we need a way to reference the cell for the loop - // to remove it from the cache. Instead of cloning, we could make [Cell::state] - // `pub(crate)` and check the state of the cell, removing the ones in the - // [CellState::Freed] state, but that would expose internal functionality of the cell. - // We could also create and `is_freed` fn on the cell. + // We clone here because we need a way to reference the cell + // for the loop to remove it from the cache. Instead of + // cloning, we could make [Cell::state] `pub(crate)` and + // check the state of the cell, removing the ones in the + // [CellState::Freed] state, but that would expose internal + // functionality of the cell. We could also create an + // `is_freed` fn on the cell. Ok::<_, CellsError>(cell.name().clone()) }) .collect() @@ -232,38 +255,30 @@ impl Cells { } impl CellsCache for Cells { - fn allocate( + async fn allocate( &mut self, cell_name: CellName, cell_spec: CellSpec, ) -> Result<&Cell> { - self.allocate(cell_name, cell_spec) + Cells::allocate(self, cell_name, cell_spec).await } - fn free(&mut self, cell_name: &CellName) -> Result<()> { - self.free(cell_name) + async fn free(&mut self, cell_name: &CellName) -> Result<()> { + Cells::free(self, cell_name).await } fn get(&mut self, cell_name: &CellName, f: F) -> Result where F: Fn(&Cell) -> Result, { - self.get(cell_name, f) + Cells::get(self, cell_name, f) } fn get_all(&self, f: F) -> Result>> where F: Fn(&Cell) -> Result, { - self.get_all(f) - } - - fn broadcast_free(&mut self) { - self.broadcast_free() - } - - fn broadcast_kill(&mut self) { - self.broadcast_kill() + Cells::get_all(self, f) } } @@ -273,8 +288,8 @@ mod tests { use crate::{AURAED_RUNTIME, AuraedRuntime}; use test_helpers::*; - #[test] - fn test_allocate() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_allocate() { skip_if_not_root!("test_allocate"); // Docker's seccomp security profile (https://docs.docker.com/engine/security/seccomp/) blocks clone skip_if_seccomp!("test_cant_unfree"); @@ -287,12 +302,13 @@ mod tests { let cell_name = CellName::random_for_tests(); let cell = CellSpec::new_for_tests(); - let _ = cells.allocate(cell_name.clone(), cell).expect("allocate"); + let _ = + cells.allocate(cell_name.clone(), cell).await.expect("allocate"); assert!(cells.cache.contains_key(&cell_name)); } - #[test] - fn test_duplicate_allocate_is_error() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_duplicate_allocate_is_error() { skip_if_not_root!("test_duplicate_allocate_is_error"); // Docker's seccomp security profile (https://docs.docker.com/engine/security/seccomp/) blocks clone skip_if_seccomp!("test_cant_unfree"); @@ -307,17 +323,18 @@ mod tests { let cell_a = CellSpec::new_for_tests(); let _ = cells .allocate(cell_name_in.clone(), cell_a) + .await .expect("failed on first allocate"); let cell_b = CellSpec::new_for_tests(); assert!(matches!( - cells.allocate(cell_name_in.clone(), cell_b), + cells.allocate(cell_name_in.clone(), cell_b).await, Err(CellsError::CellExists { cell_name }) if cell_name == cell_name_in )); } - #[test] - fn test_get() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_get() { skip_if_not_root!("test_get"); // Docker's seccomp security profile (https://docs.docker.com/engine/security/seccomp/) blocks clone skip_if_seccomp!("test_get"); @@ -331,6 +348,7 @@ mod tests { let cell = CellSpec::new_for_tests(); let _ = cells .allocate(cell_name.clone(), cell) + .await .expect("failed to allocate"); cells.get(&cell_name, |_cell| Ok(())).expect("failed to get"); @@ -349,8 +367,8 @@ mod tests { )); } - #[test] - fn test_free() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_free() { skip_if_not_root!("test_free"); // Docker's seccomp security profile (https://docs.docker.com/engine/security/seccomp/) blocks clone skip_if_seccomp!("test_free"); @@ -364,21 +382,22 @@ mod tests { let cell = CellSpec::new_for_tests(); let _ = cells .allocate(cell_name.clone(), cell) + .await .expect("failed to allocate"); - cells.free(&cell_name).expect("failed to free"); + cells.free(&cell_name).await.expect("failed to free"); assert!(cells.cache.is_empty()); } - #[test] - fn test_free_missing_is_error() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_free_missing_is_error() { let mut cells = Cells::default(); assert!(cells.cache.is_empty()); let cell_name_in = CellName::random_for_tests(); assert!(matches!( - cells.free(&cell_name_in), + cells.free(&cell_name_in).await, Err(CellsError::CellNotFound { cell_name }) if cell_name == cell_name_in )); } @@ -388,8 +407,8 @@ mod tests { children: Vec, } - #[test] - fn test_cell_graph_triple_nested() { + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn test_cell_graph_triple_nested() { skip_if_not_root!("test_cell_graph_triple_nested"); skip_if_seccomp!("test_cell_graph_triple_nested"); @@ -403,6 +422,7 @@ mod tests { let grandparent_cell = CellSpec::new_for_tests(); let _ = cells .allocate(grandparent_cell_name.clone(), grandparent_cell) + .await .expect("failed to allocate"); // Create parent cell @@ -411,6 +431,7 @@ mod tests { let parent_cell = CellSpec::new_for_tests(); let _ = cells .allocate(parent_cell_name.clone(), parent_cell) + .await .expect("failed to allocate"); // Create child cell @@ -419,6 +440,7 @@ mod tests { let child_cell = CellSpec::new_for_tests(); let _ = cells .allocate(child_cell_name.clone(), child_cell) + .await .expect("failed to allocate"); fn cell_fn(cell: &Cell) -> Result { diff --git a/auraed/src/cells/cell_service/cells/cells_cache.rs b/auraed/src/cells/cell_service/cells/cells_cache.rs index 177da1674..760cdccbd 100644 --- a/auraed/src/cells/cell_service/cells/cells_cache.rs +++ b/auraed/src/cells/cell_service/cells/cells_cache.rs @@ -12,46 +12,29 @@ * Copyright 2022 - 2024, the aurae contributors * * SPDX-License-Identifier: Apache-2.0 * \* -------------------------------------------------------------------------- */ -/* -------------------------------------------------------------------------- *\ - * Apache 2.0 License Copyright © 2022-2023 The Aurae Authors * - * * - * +--------------------------------------------+ * - * | █████╗ ██╗ ██╗██████╗ █████╗ ███████╗ | * - * | ██╔══██╗██║ ██║██╔══██╗██╔══██╗██╔════╝ | * - * | ███████║██║ ██║██████╔╝███████║█████╗ | * - * | ██╔══██║██║ ██║██╔══██╗██╔══██║██╔══╝ | * - * | ██║ ██║╚██████╔╝██║ ██║██║ ██║███████╗ | * - * | ╚═╝ ╚═╝ ╚═════╝ ╚═╝ ╚═╝╚═╝ ╚═╝╚══════╝ | * - * +--------------------------------------------+ * - * * - * Distributed Systems Runtime * - * * - * -------------------------------------------------------------------------- * - * * - * Licensed under the Apache License, Version 2.0 (the "License"); * - * you may not use this file except in compliance with the License. * - * You may obtain a copy of the License at * - * * - * http://www.apache.org/licenses/LICENSE-2.0 * - * * - * Unless required by applicable law or agreed to in writing, software * - * distributed under the License is distributed on an "AS IS" BASIS, * - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * - * See the License for the specific language governing permissions and * - * limitations under the License. * - * * -\* -------------------------------------------------------------------------- */ use super::{Cell, CellName, CellSpec, Result}; -pub trait CellsCache { +/// Common interface for both `Cells` (the daemon's cell collection) and +/// `Cell` (forwarding to its child collection). The recursive structure +/// of nested cells means we walk the same operations through whichever +/// type happens to hold the next level. +/// +/// `allocate` and `free` are async because cell creation and teardown +/// perform rtnetlink and IPAM operations under a tokio executor. +/// `get`/`get_all` stay sync — they don't talk to the kernel. +/// +/// `broadcast_free` and `broadcast_kill` aren't part of this trait — +/// they're inherent methods on `Cells` only, called once at daemon +/// shutdown rather than recursing through the cell tree. +pub(crate) trait CellsCache { /// Calls [Cell::allocate] on a new [Cell] and adds it to it's cache with key [CellName]. /// /// # Errors /// * If cell exists -> [CellsError::CellExists] /// * If a cell is not in cache but cgroup exists on fs -> [CellsError::CgroupIsNotACell] /// * If cell fails to allocate (see [Cell::allocate]) - fn allocate( + async fn allocate( &mut self, cell_name: CellName, cell_spec: CellSpec, @@ -65,7 +48,7 @@ pub trait CellsCache { /// - note: cell will be removed from cache /// * If cell is not cached and cgroup exists on fs -> [CellsError::CgroupIsNotACell] /// * If cell fails to free (see [Cell::free]) - fn free(&mut self, cell_name: &CellName) -> Result<()>; + async fn free(&mut self, cell_name: &CellName) -> Result<()>; fn get(&mut self, cell_name: &CellName, f: F) -> Result where @@ -74,11 +57,4 @@ pub trait CellsCache { fn get_all(&self, f: F) -> Result>> where F: Fn(&Cell) -> Result; - - /// Calls [Cell::Free] on all cells in the cache, ignoring any errors. - /// Successfully freed cells will be removed from the cache. - fn broadcast_free(&mut self); - - /// Sends a [SIGKILL] to all Cells, ignoring any errors. - fn broadcast_kill(&mut self); } diff --git a/auraed/src/cells/cell_service/cells/cgroups/cgroup.rs b/auraed/src/cells/cell_service/cells/cgroups/cgroup.rs index 05e6176df..3a082782e 100644 --- a/auraed/src/cells/cell_service/cells/cgroups/cgroup.rs +++ b/auraed/src/cells/cell_service/cells/cgroups/cgroup.rs @@ -182,27 +182,36 @@ impl Cgroup { } pub fn delete(&self) -> Result<()> { - let leaf = v2::manager::Manager::new( - DEFAULT_CGROUP_ROOT.into(), - get_leaf_path(&self.cell_name), - ) - .expect("valid cgroup"); - - leaf.remove().map_err(|e| CgroupsError::DeleteCgroup { - cell_name: self.cell_name.clone(), - source: e.into(), - })?; - - let non_leaf = v2::manager::Manager::new( - DEFAULT_CGROUP_ROOT.into(), - self.cell_name.clone().into_inner(), - ) - .expect("valid cgroup"); + let leaf_path = get_leaf_path(&self.cell_name); + let mut absolute_leaf_path = PathBuf::from(DEFAULT_CGROUP_ROOT); + absolute_leaf_path.push(&leaf_path); + if absolute_leaf_path.exists() { + let leaf = v2::manager::Manager::new( + DEFAULT_CGROUP_ROOT.into(), + leaf_path, + ) + .expect("valid cgroup"); + leaf.remove().map_err(|e| CgroupsError::DeleteCgroup { + cell_name: self.cell_name.clone(), + source: e.into(), + })?; + } - non_leaf.remove().map_err(|e| CgroupsError::DeleteCgroup { - cell_name: self.cell_name.clone(), - source: e.into(), - }) + let non_leaf_path = self.cell_name.clone().into_inner(); + let mut absolute_non_leaf_path = PathBuf::from(DEFAULT_CGROUP_ROOT); + absolute_non_leaf_path.push(&non_leaf_path); + if absolute_non_leaf_path.exists() { + let non_leaf = v2::manager::Manager::new( + DEFAULT_CGROUP_ROOT.into(), + non_leaf_path, + ) + .expect("valid cgroup"); + non_leaf.remove().map_err(|e| CgroupsError::DeleteCgroup { + cell_name: self.cell_name.clone(), + source: e.into(), + })?; + } + Ok(()) } pub fn v2(&self) -> bool { diff --git a/auraed/src/cells/cell_service/cells/mod.rs b/auraed/src/cells/cell_service/cells/mod.rs index 213a67b59..0c9c3aacb 100644 --- a/auraed/src/cells/cell_service/cells/mod.rs +++ b/auraed/src/cells/cell_service/cells/mod.rs @@ -16,7 +16,7 @@ pub use cell::Cell; pub use cell_name::CellName; pub use cells::Cells; -pub use cells_cache::CellsCache; +pub(crate) use cells_cache::CellsCache; use cgroups::CgroupSpec; pub use error::{CellsError, Result}; pub use nested_auraed::IsolationControls;