Some doc and refactoring

Author: Virv12

.github/workflows/rust.yml

@@ -39,6 +39,7 @@ jobs:
 
       - name: Install apt dependencies
         run: |
+          sudo apt-get update
           sudo apt-get install -y dnsmasq qemu-system-x86 ovmf
           sudo mkdir -p /etc/qemu
           echo allow br-pixie | sudo tee /etc/qemu/bridge.conf

.gitignore

@@ -4,3 +4,4 @@ build/*
 buildroot/pixie.tar.gz
 pixie-web/dist
 .*.swp
+pixie-shared/Cargo.lock

pixie-server/src/dnsmasq.rs

@@ -1,3 +1,5 @@
+//! Starts and configures dnsmasq.
+
 use crate::{find_network, state::State};
 use anyhow::{Context, Result};
 use macaddr::MacAddr6;
@@ -35,14 +37,15 @@ impl Drop for DnsmasqHandle {
 async fn write_config(state: &State) -> Result<()> {
     let (name, _) = find_network(state.config.hosts.listen_on)?;
 
-    let mut dnsmasq_conf = File::create(state.storage_dir.join("dnsmasq.conf"))?;
+    let mut dnsmasq_conf = File::create(state.run_dir.join("dnsmasq.conf"))?;
 
     let dhcp_dynamic_conf = match state.config.hosts.dhcp {
         DhcpMode::Static(low, high) => format!("dhcp-range=tag:netboot,{low},{high}"),
         DhcpMode::Proxy(ip) => format!("dhcp-range=tag:netboot,{},proxy", ip),
     };
 
     let storage_str = state.storage_dir.to_str().unwrap();
+    let run_str = state.run_dir.to_str().unwrap();
 
     write!(
         dnsmasq_conf,
@@ -52,7 +55,7 @@ async fn write_config(state: &State) -> Result<()> {
 ## net0
 {dhcp_dynamic_conf}
 dhcp-range=tag:!netboot,10.0.0.0,static,255.0.0.0
-dhcp-hostsfile={storage_str}/hosts
+dhcp-hostsfile={run_str}/hosts
 dhcp-boot=pixie-uefi.efi
 interface={name}
 except-interface=lo
@@ -81,7 +84,7 @@ dhcp-vendorclass=set:netboot,pixie
 }
 
 async fn write_hosts(state: &State, hosts: &[(MacAddr6, Ipv4Addr, Option<String>)]) -> Result<()> {
-    let file = File::create(state.storage_dir.join("hosts"))?;
+    let file = File::create(state.run_dir.join("hosts"))?;
     let mut file = BufWriter::new(file);
 
     for (mac, ip, hostname) in hosts {
@@ -123,8 +126,8 @@ pub async fn main(state: Arc<State>) -> Result<()> {
     let dnsmasq = DnsmasqHandle {
         child: Command::new("dnsmasq")
             .arg(format!(
-                "--conf-file={storage_str}/dnsmasq.conf",
-                storage_str = state.storage_dir.to_str().unwrap()
+                "--conf-file={run_str}/dnsmasq.conf",
+                run_str = state.run_dir.to_str().unwrap()
             ))
             .arg("--log-dhcp")
             .arg("--no-daemon")

pixie-server/src/http.rs

@@ -1,3 +1,5 @@
+//! HTTP server for the admin web interface.
+
 use crate::state::{State, UnitSelector};
 use anyhow::Result;
 use axum::{
@@ -17,11 +19,16 @@ use tower_http::{
     services::ServeDir, trace::TraceLayer, validate_request::ValidateRequestHeaderLayer,
 };
 
+/// `GET /admin/action/{unit_selector}/{action}`
+///
+/// Sets the next [`Action`] for all [`Unit`]s accepted by the [`UnitSelector`].
+///
+/// [`Unit`]: pixie_shared::config::Unit
 async fn action(
-    Path((unit_filter, action)): Path<(String, Action)>,
+    Path((unit_selector, action)): Path<(String, Action)>,
     extract::State(state): extract::State<Arc<State>>,
 ) -> impl IntoResponse {
-    let Some(unit_selector) = UnitSelector::parse(&state, unit_filter) else {
+    let Some(unit_selector) = UnitSelector::parse(&state, unit_selector) else {
         return (
             StatusCode::BAD_REQUEST,
             "Invalid unit selector\n".to_owned(),
@@ -36,11 +43,16 @@ async fn action(
     }
 }
 
+/// `GET /admin/curr_action/{unit_selector}/{action}`
+///
+/// Sets the current [`Action`] for all [`Unit`]s accepted by the [`UnitSelector`].
+///
+/// [`Unit`]: pixie_shared::config::Unit
 async fn curr_action(
-    Path((unit_filter, action)): Path<(String, Action)>,
+    Path((unit_selector, action)): Path<(String, Action)>,
     extract::State(state): extract::State<Arc<State>>,
 ) -> impl IntoResponse {
-    let Some(unit_selector) = UnitSelector::parse(&state, unit_filter) else {
+    let Some(unit_selector) = UnitSelector::parse(&state, unit_selector) else {
         return (
             StatusCode::BAD_REQUEST,
             "Invalid unit selector\n".to_owned(),
@@ -55,8 +67,14 @@ async fn curr_action(
     }
 }
 
+/// `GET /admin/image/{unit_selector}/{image}`
+///
+/// Sets the [`Image`] for all [`Unit`]s accepted by the [`UnitSelector`].
+///
+/// [`Unit`]: pixie_shared::config::Unit
+/// [`Image`]: pixie_shared::Image
 async fn image(
-    Path((unit_filter, image)): Path<(String, String)>,
+    Path((unit_selector, image)): Path<(String, String)>,
     extract::State(state): extract::State<Arc<State>>,
 ) -> impl IntoResponse {
     if !state.config.images.contains(&image) {
@@ -66,7 +84,7 @@ async fn image(
         );
     }
 
-    let Some(unit_selector) = UnitSelector::parse(&state, unit_filter) else {
+    let Some(unit_selector) = UnitSelector::parse(&state, unit_selector) else {
         return (
             StatusCode::BAD_REQUEST,
             "Invalid unit selector\n".to_owned(),
@@ -80,11 +98,16 @@ async fn image(
     }
 }
 
+/// `GET /admin/forget/{unit_selector}`
+///
+/// Forgets all [`Unit`]s selected by the [`UnitSelector`].
+///
+/// [`Unit`]: pixie_shared::config::Unit
 async fn forget(
-    Path(unit_filter): Path<String>,
+    Path(unit_selector): Path<String>,
     extract::State(state): extract::State<Arc<State>>,
 ) -> impl IntoResponse {
-    let Some(unit_selector) = UnitSelector::parse(&state, unit_filter) else {
+    let Some(unit_selector) = UnitSelector::parse(&state, unit_selector) else {
         return (
             StatusCode::BAD_REQUEST,
             "Invalid unit selector\n".to_owned(),
@@ -119,13 +142,19 @@ async fn delete_image(
     }
 }
 
+/// `GET /admin/gc`
+///
+/// Removes all chunks not used by any image.
 async fn gc(extract::State(state): extract::State<Arc<State>>) -> impl IntoResponse {
     match state.gc_chunks() {
         Ok(()) => (StatusCode::NO_CONTENT, String::new()),
         Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, format!("{e}\n")),
     }
 }
 
+/// `GET /admin/status`
+///
+/// Stream of json-formatted events on changes to the database.
 async fn status(extract::State(state): extract::State<Arc<State>>) -> impl IntoResponse {
     let initial_messages = [StatusUpdate::Config(state.config.clone())];
 
@@ -161,10 +190,13 @@ pub async fn main(state: Arc<State>) -> Result<()> {
     let mut router = Router::new()
         .route("/admin/status", get(status))
         .route("/admin/gc", get(gc))
-        .route("/admin/action/:unit/:action", get(action))
-        .route("/admin/curr_action/:unit/:action", get(curr_action))
-        .route("/admin/image/:unit/:image", get(image))
-        .route("/admin/forget/:unit", get(forget))
+        .route("/admin/action/:unit_selector/:action", get(action))
+        .route(
+            "/admin/curr_action/:unit_selector/:action",
+            get(curr_action),
+        )
+        .route("/admin/image/:unit_selector/:image", get(image))
+        .route("/admin/forget/:unit_selector", get(forget))
         .route("/admin/rollback/:image", get(rollback))
         .route("/admin/delete/:image", get(delete_image))
         .nest_service(

pixie-server/src/main.rs

@@ -21,6 +21,10 @@ use std::{
 };
 use tokio::task::JoinHandle;
 
+/// Finds the mac address for the given ip.
+///
+/// This function searches the address in the arp cache, if it is not available it tries to
+/// populate it by pinging the peer.
 fn find_mac(ip: Ipv4Addr) -> Result<MacAddr6> {
     struct Zombie {
         inner: Child,
@@ -93,10 +97,11 @@ fn find_network(ip: Ipv4Addr) -> Result<(String, Ipv4Net)> {
     bail!("Could not find the network for {}", ip);
 }
 
+/// Command line arguments for pixie-server.
 #[derive(Parser, Debug)]
 struct PixieOptions {
     /// Directory in which files will be stored.
-    /// Must already contain files: tftpboot/pixie-uefi.efi, config.yaml
+    /// Must already contain files: `tftpboot/pixie-uefi.efi` and `config.yaml`.
     #[clap(short, long, default_value = "./storage")]
     storage_dir: PathBuf,
 }

pixie-server/src/ping.rs

@@ -1,3 +1,5 @@
+//! Handles pings from clients.
+
 use crate::{
     find_mac,
     state::{State, UnitSelector},

pixie-server/src/state/images.rs

@@ -4,13 +4,15 @@ use pixie_shared::{ChunkHash, ChunkStats, ChunksStats, Image, ImagesStats};
 use tokio::sync::watch;
 
 impl State {
+    /// Checks whether the database contains the given chunk.
     pub fn has_chunk(&self, hash: ChunkHash) -> bool {
         self.chunks_stats
             .lock()
             .expect("chunks_stats lock is poisoned")
             .contains_key(&hash)
     }
 
+    /// Get the chunk compressed data.
     pub fn get_chunk_cdata(&self, hash: ChunkHash) -> Result<Option<Vec<u8>>> {
         let path = self.storage_dir.join(CHUNKS_DIR).join(hex::encode(hash));
         let chunks_stats = self
@@ -24,6 +26,7 @@ impl State {
         Ok(cdata)
     }
 
+    /// Store the given chunk to the database.
     pub fn add_chunk(&self, data: &[u8]) -> Result<()> {
         ensure!(
             data.len() <= pixie_shared::MAX_CHUNK_SIZE,
@@ -57,6 +60,7 @@ impl State {
         res
     }
 
+    /// Finds and deletes all chunks which are not part of any image.
     pub fn gc_chunks(&self) -> Result<()> {
         let mut res = Ok(());
         self.images_stats.send_modify(|images_stats| {

pixie-server/src/state/mod.rs

@@ -1,10 +1,22 @@
+//! The database for pixie-server.
+//!
+//! Structure of the storage directory:
+//! - `config.yaml`: configuration file for pixie-server
+//! - `registered.json`: json file containing all information about registered units.
+//! - `admin/`: directory containing the static files for the admin web interface.
+//! - `chunks/`: directory containing the image's chunks.
+//! - `images/`: directory containing the image's info.
+//! - `tftpboot/`: directory containing the necessary files for network boot.
+
 #![warn(clippy::unwrap_used)]
 
 mod images;
 mod units;
 
 use anyhow::{anyhow, ensure, Context, Result};
-use pixie_shared::{ChunkHash, ChunkStats, ChunksStats, Config, Image, ImagesStats, Station, Unit};
+use pixie_shared::{
+    ChunkHash, ChunkStats, ChunksStats, Config, Image, ImagesStats, RegistrationInfo, Unit,
+};
 use std::{
     collections::HashMap,
     fs::File,
@@ -25,6 +37,10 @@ const REGISTERED_JSON: &str = "registered.json";
 const CHUNKS_DIR: &str = "chunks";
 const IMAGES_DIR: &str = "images";
 
+/// Atomically write `data` at the specified `path`.
+///
+/// On crash `path` is guaranteed to be in a consistent state, but a temporary file might be left
+/// behind.
 fn atomic_write(path: &Path, data: &[u8]) -> Result<()> {
     static CNT: AtomicU64 = AtomicU64::new(0);
 
@@ -44,14 +60,18 @@ fn atomic_write(path: &Path, data: &[u8]) -> Result<()> {
     Ok(())
 }
 
+/// Builds a map from ip address to hostname parsing the hostfile at `path`.
 fn build_hostmap(path: Option<&Path>) -> Result<HashMap<Ipv4Addr, String>> {
     let mut hostmap = HashMap::new();
     if let Some(path) = path {
         let hosts =
             hostfile::parse_file(path).map_err(|e| anyhow!("Error parsing host file: {e}"))?;
         for mut host in hosts {
             if let IpAddr::V4(ip) = host.ip {
-                hostmap.insert(ip, std::mem::take(&mut host.names[0]));
+                let old = hostmap.insert(ip, std::mem::take(&mut host.names[0]));
+                if old.is_some() {
+                    log::warn!("Duplicated hostname for {ip}");
+                }
             } else {
                 log::warn!(
                     "ignoring non-IPv4 address {} for host {}",
@@ -64,19 +84,25 @@ fn build_hostmap(path: Option<&Path>) -> Result<HashMap<Ipv4Addr, String>> {
     Ok(hostmap)
 }
 
+/// See [the module-level documentation][self].
 pub struct State {
+    /// The storage_dir received by command line arguments.
     pub storage_dir: PathBuf,
+    /// A directory stored in ram for dynamically generated files, will be deleted on Drop.
+    pub run_dir: PathBuf,
+    /// The config parsed from the config file.
     pub config: Config,
+    /// The hostmap built from the hostmap file.
     hostmap: watch::Sender<HashMap<Ipv4Addr, String>>,
 
     units: watch::Sender<Vec<Unit>>,
-    // TODO: use an Option
-    last: Mutex<Station>,
+    registration_hint: Mutex<Option<RegistrationInfo>>,
     images_stats: watch::Sender<ImagesStats>,
     chunks_stats: Mutex<ChunksStats>,
 }
 
 impl State {
+    /// Loads the [`State`] from the given path.
     pub fn load(storage_dir: PathBuf) -> Result<Self> {
         let config: Config = {
             let path = storage_dir.join(CONFIG_YAML);
@@ -121,22 +147,6 @@ impl State {
             }
         });
 
-        let last = Station {
-            group: config
-                .groups
-                .iter()
-                .next()
-                .context("there should be at least one group")?
-                .0
-                .clone(),
-            image: config
-                .images
-                .first()
-                .context("there should be at least one image")?
-                .clone(),
-            ..Default::default()
-        };
-
         let chunks_dir = storage_dir.join(CHUNKS_DIR);
         let mut chunks_stats: ChunksStats = std::fs::read_dir(&chunks_dir)
             .with_context(|| format!("open chunks dir: {}", chunks_dir.display()))?
@@ -193,12 +203,16 @@ impl State {
             images,
         };
 
+        let run_dir = PathBuf::from(format!("/run/pixie-{}", std::process::id()));
+        std::fs::create_dir(&run_dir)?;
+
         Ok(Self {
             storage_dir,
+            run_dir,
             config,
             hostmap: watch::Sender::new(hostmap),
             units,
-            last: Mutex::new(last),
+            registration_hint: Mutex::new(None),
             images_stats: watch::Sender::new(images_stats),
             chunks_stats: Mutex::new(chunks_stats),
         })
@@ -214,3 +228,11 @@ impl State {
         self.hostmap.subscribe()
     }
 }
+
+impl Drop for State {
+    fn drop(&mut self) {
+        // If we fail to remove the directory it's not an issue as it will be deleted on shutdown
+        // and doesn't take much space.
+        let _ = std::fs::remove_dir_all(&self.run_dir);
+    }
+}