diff --git a/traffic_core/src/swap.rs b/traffic_core/src/swap.rs index b8a3ccb..c040c56 100644 --- a/traffic_core/src/swap.rs +++ b/traffic_core/src/swap.rs @@ -56,8 +56,14 @@ const DEFAULT_SWAP_PATH: &str = "/swapfile"; const SWAP_OWNERSHIP_PATH: &str = "/var/lib/awg2-harness/swap-ownership.conf"; const SWAP_OWNERSHIP_MARKER: &str = "# Managed by awg2-harness swap-ownership"; const SWAP_OWNERSHIP_VERSION: u32 = 1; -/// Sidecar written by earlier versions next to the swapfile. Only ever removed. +/// Sidecar written by earlier versions next to the swapfile. Only ever removed, and only +/// when it carries the marker that version wrote. const LEGACY_OWNERSHIP_SUFFIX: &str = ".awg2-harness-owned"; +const LEGACY_OWNERSHIP_MARKER: &str = "# Managed by awg2-harness swap"; +/// Replacement being built while the current swapfile is still active. +const STAGING_SUFFIX: &str = ".awg2-harness.new"; +/// The outgoing swapfile, kept until the replacement is confirmed active. +const PREVIOUS_SUFFIX: &str = ".awg2-harness.previous"; /// Identity of a file on disk, independent of its name. /// @@ -173,9 +179,9 @@ pub fn ensure_swap(cfg: &Config) -> anyhow::Result<()> { assert_swap_path_is_safe(&desired.path)?; let active_before = read_active_swaps()?; - // 2-3. Prepare and activate the managed swap. + // 2-3. Prepare and activate the managed swap. Activation belongs to this step so that + // every transition can roll back while it still owns both the old and the new file. ensure_desired_swapfile(&desired, &active_before)?; - enable_desired_swap(&desired)?; // 4. Confirm it is really live before touching anything else. let active_after = read_active_swaps()?; @@ -475,15 +481,17 @@ fn set_dir_mode(_dir: &Path) -> anyhow::Result<()> { /// Only removed, never trusted: ownership now lives in the state directory. A sidecar at /// a path we do not own is left alone. fn drop_legacy_ownership_sidecar(swap_path: &Path) { - let Some(name) = swap_path.file_name() else { + let Ok(sidecar) = suffixed_path(swap_path, LEGACY_OWNERSHIP_SUFFIX) else { return; }; - let mut sidecar = name.to_os_string(); - sidecar.push(LEGACY_OWNERSHIP_SUFFIX); - let sidecar = swap_path.with_file_name(sidecar); - match fs::symlink_metadata(&sidecar) { - Ok(md) if md.file_type().is_file() => { - if let Err(e) = fs::remove_file(&sidecar) { + // Goes through the same marker check as every other managed file. Removing any regular + // file that happened to match the name would delete an operator's file purely because + // of what it was called. + let file = ManagedFile::new(sidecar.clone(), LEGACY_OWNERSHIP_MARKER); + match file.ownership() { + Ok(Ownership::Absent) => {} + Ok(Ownership::Managed) => { + if let Err(e) = file.remove() { tracing::warn!( target: "swap", "could not remove legacy ownership sidecar {}: {e}", @@ -491,7 +499,16 @@ fn drop_legacy_ownership_sidecar(swap_path: &Path) { ); } } - _ => {} + Ok(Ownership::Foreign) => tracing::warn!( + target: "swap", + "{} exists but is not the legacy ownership sidecar; leaving it alone", + sidecar.display() + ), + Err(e) => tracing::warn!( + target: "swap", + "could not inspect {}: {e}", + sidecar.display() + ), } } @@ -710,20 +727,19 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh drop_legacy_ownership_sidecar(p); Ok(()) } - // Right file, right size, but the active priority differs. Re-activating is - // enough; the previous code fell through to a full recreate, so changing + // Right file, right size, but the active priority differs. A reactivation, not a + // rebuild - the previous code fell through to a full recreate, so changing // swap.priority destroyed and rebuilt a perfectly good multi-gigabyte file. - (true, Some(_)) => { - run("swapoff", &[p.to_string_lossy().as_ref()])?; + (true, Some(entry)) => { set_swapfile_mode(p)?; drop_legacy_ownership_sidecar(p); - Ok(()) + change_swap_priority(desired, entry.priority) } - // Right file, not active: the caller activates it next. + // Right file, not active: activate it. (true, None) => { set_swapfile_mode(p)?; drop_legacy_ownership_sidecar(p); - Ok(()) + swapon_at(p, desired.priority) } // Wrong size or no file at all, and nothing of ours is active: build in place. (false, None) => { @@ -731,74 +747,210 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh fs::remove_file(p) .map_err(|e| anyhow::anyhow!("failed to remove {}: {e}", p.display()))?; } - build_swapfile(p, desired.size_bytes)?; - record_ownership(p)?; + create_fresh_swapfile(desired)?; drop_legacy_ownership_sidecar(p); Ok(()) } // Wrong size while our swap is live: replace it through a staging file. - (false, Some(_)) => replace_active_swapfile(desired), + (false, Some(_)) => { + replace_active_swapfile(desired)?; + drop_legacy_ownership_sidecar(p); + Ok(()) + } } } -/// Resizes the managed swapfile while it is active, without a swapless gap during the -/// slow part. +/// Re-activates the managed swap at a new priority, restoring the old one on failure. /// -/// A live swapfile cannot be resized in place, so it has to be replaced. Doing that as -/// swapoff-then-create left the host with no swap for the whole duration of -/// `fallocate`/`dd`/`mkswap` - on a machine that was given swap precisely because memory -/// is tight, and with no way back if any of those steps failed. The allocation now happens -/// while the old swap is still serving, and the only gap left is the swapoff/rename/swapon -/// triple, which allocates nothing. +/// `swapoff` followed by `swapon` is two steps, and the second can fail - a bad priority, +/// a transient kernel error. Without the restore, changing one integer in the config could +/// leave the host with no swap at all. +fn change_swap_priority(desired: &DesiredSwap, previous_priority: i32) -> anyhow::Result<()> { + let p = &desired.path; + run("swapoff", &[p.to_string_lossy().as_ref()])?; + + let Err(e) = swapon_at(p, desired.priority) else { + return Ok(()); + }; + match swapon_at(p, previous_priority) { + Ok(()) => Err(anyhow::anyhow!( + "failed to activate {} with priority {}: {e}; restored the previous priority {}", + p.display(), + desired.priority, + previous_priority + )), + Err(re) => Err(anyhow::anyhow!( + "failed to activate {} with priority {} ({e}) and could not restore priority {} \ + ({re}); the host may currently have no swap", + p.display(), + desired.priority, + previous_priority + )), + } +} + +/// Builds and activates a swapfile where none of ours is running. +/// +/// The file is removed again if anything after its creation fails. Leaving it behind would +/// produce an unowned regular file at the managed path, which the next run correctly +/// classifies as `Foreign` and then refuses to touch - so a single transient `mkswap` +/// failure would wedge the module until an operator intervened. +fn create_fresh_swapfile(desired: &DesiredSwap) -> anyhow::Result<()> { + let p = &desired.path; + build_swapfile(p, desired.size_bytes)?; + + let prepared = record_ownership(p).and_then(|()| swapon_at(p, desired.priority)); + if let Err(e) = prepared { + // Created by this run inside an allowed root-owned location, so removing it is + // safe and leaves the host exactly as it was found. + if let Err(re) = fs::remove_file(p) { + return Err(anyhow::anyhow!( + "{e}; additionally failed to clean up the partially prepared {}: {re}", + p.display() + )); + } + let _ = ownership_file().remove(); + return Err(e); + } + Ok(()) +} + +/// Replaces a live managed swapfile with one of a different size. +/// +/// A live swapfile cannot be resized in place, so it has to be swapped out. Allocation and +/// `mkswap` happen while the old swap is still serving. The old file is then kept aside as +/// `.previous` rather than deleted, so every step after that point can be undone: until +/// the new file is confirmed active, the original is still on disk and can be put back. +/// The previous version deleted the old file before the new one was activated, so a +/// failure at `rename` or `swapon` left the host with no swap and nothing to recover from. fn replace_active_swapfile(desired: &DesiredSwap) -> anyhow::Result<()> { let p = &desired.path; - let staging = staging_path(p)?; + let staging = suffixed_path(p, STAGING_SUFFIX)?; + let previous = suffixed_path(p, PREVIOUS_SUFFIX)?; - match fs::remove_file(&staging) { - Ok(()) => {} - Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} - Err(e) => anyhow::bail!("failed to clear stale {}: {e}", staging.display()), - } + remove_if_present(&staging)?; + remove_if_present(&previous)?; - // Built and activated while the old swap is still up. + // Built and activated while the old swap is still up: no gap during the slow part. build_swapfile(&staging, desired.size_bytes)?; - if let Err(e) = run( - "swapon", - &[ - "-p", - &desired.priority.to_string(), - staging.to_string_lossy().as_ref(), - ], - ) { + if let Err(e) = swapon_at(&staging, desired.priority) { let _ = fs::remove_file(&staging); return Err(e); } - // The old file is only released once the replacement is carrying load. + // From here the old swap can be released, because the replacement is carrying load. run("swapoff", &[p.to_string_lossy().as_ref()])?; - fs::remove_file(p).map_err(|e| anyhow::anyhow!("failed to remove {}: {e}", p.display()))?; - - // Short gap: no allocation happens between here and the swapon below. - run("swapoff", &[staging.to_string_lossy().as_ref()])?; - fs::rename(&staging, p).map_err(|e| { + fs::rename(p, &previous).map_err(|e| { anyhow::anyhow!( - "failed to move {} into place at {}: {e}; activate it manually with swapon", - staging.display(), - p.display() + "failed to set aside {} as {}: {e}", + p.display(), + previous.display() ) })?; - record_ownership(p)?; - drop_legacy_ownership_sidecar(p); + + // Short window with no allocation in it. Everything below rolls back to `previous`. + let switched = switch_staging_into_place(desired, &staging, &previous); + if let Err(e) = switched { + return Err(restore_previous_swapfile(desired, &previous, e)); + } + + // The replacement is confirmed active; the old file is no longer needed. + if let Err(e) = fs::remove_file(&previous) { + tracing::warn!( + target: "swap", + "swap replaced successfully but the old file {} could not be removed: {e}", + previous.display() + ); + } Ok(()) } -fn staging_path(path: &Path) -> anyhow::Result { +fn switch_staging_into_place( + desired: &DesiredSwap, + staging: &Path, + previous: &Path, +) -> anyhow::Result<()> { + let p = &desired.path; + run("swapoff", &[staging.to_string_lossy().as_ref()])?; + fs::rename(staging, p).map_err(|e| { + anyhow::anyhow!( + "failed to move {} into place at {}: {e} (previous swapfile kept at {})", + staging.display(), + p.display(), + previous.display() + ) + })?; + record_ownership(p)?; + swapon_at(p, desired.priority)?; + assert_some_swap_is_active(p) +} + +/// Puts the original swapfile back after a failed replacement. +/// +/// Returns an error that carries both the original failure and the outcome of the +/// restore, so the operator sees what went wrong *and* whether the host still has swap. +fn restore_previous_swapfile( + desired: &DesiredSwap, + previous: &Path, + cause: anyhow::Error, +) -> anyhow::Error { + let p = &desired.path; + let _ = remove_if_present(p); + if let Err(e) = fs::rename(previous, p) { + return anyhow::anyhow!( + "{cause}; rollback failed: could not restore {} from {}: {e}. The host may \ + currently have no swap", + p.display(), + previous.display() + ); + } + if let Err(e) = record_ownership(p) { + tracing::warn!(target: "swap", "restored swapfile but could not rewrite ownership: {e}"); + } + match swapon_at(p, desired.priority) { + Ok(()) => anyhow::anyhow!("{cause}; rolled back to the previous swapfile"), + Err(e) => anyhow::anyhow!( + "{cause}; rollback restored {} but could not activate it: {e}. The host may \ + currently have no swap", + p.display() + ), + } +} + +/// Refuses to report success while the host has no swap at all. +fn assert_some_swap_is_active(expected: &Path) -> anyhow::Result<()> { + let active = read_active_swaps()?; + if active.iter().any(|s| s.name == expected) { + return Ok(()); + } + anyhow::bail!( + "{} is not present in the active swap list after activation", + expected.display() + ) +} + +fn swapon_at(path: &Path, priority: i32) -> anyhow::Result<()> { + run( + "swapon", + &["-p", &priority.to_string(), path.to_string_lossy().as_ref()], + ) +} + +fn remove_if_present(path: &Path) -> anyhow::Result<()> { + match fs::remove_file(path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(anyhow::anyhow!("failed to remove {}: {e}", path.display())), + } +} + +fn suffixed_path(path: &Path, suffix: &str) -> anyhow::Result { let name = path .file_name() .ok_or_else(|| anyhow::anyhow!("invalid swap.path: {}", path.display()))?; - let mut staging = name.to_os_string(); - staging.push(".awg2-harness.new"); - Ok(path.with_file_name(staging)) + let mut out = name.to_os_string(); + out.push(suffix); + Ok(path.with_file_name(out)) } /// Allocates a swapfile and formats it, with the restrictive mode applied before `mkswap` @@ -894,26 +1046,6 @@ fn render_fstab_line(path: &Path, priority: i32) -> String { ) } -fn enable_desired_swap(desired: &DesiredSwap) -> anyhow::Result<()> { - let active = read_active_swaps()?; - if let Some(found) = active.iter().find(|v| v.name == desired.path) - && active_swap_size_matches(found.size_bytes, desired.size_bytes) - && found.priority == desired.priority - { - return Ok(()); - } - - run( - "swapon", - &[ - "-p", - &desired.priority.to_string(), - desired.path.to_string_lossy().as_ref(), - ], - )?; - Ok(()) -} - fn apply_swappiness(v: u8) -> anyhow::Result<()> { run("sysctl", &["-w", &format!("vm.swappiness={v}")])?; let current = read_current_swappiness()?; @@ -1330,11 +1462,36 @@ mod tests { #[test] fn staging_path_is_a_sibling_of_the_swapfile() { - let staging = staging_path(Path::new("/var/lib/awg2-harness/swapfile")).expect("staging"); + let base = Path::new("/var/lib/awg2-harness/swapfile"); assert_eq!( - staging, + suffixed_path(base, STAGING_SUFFIX).expect("staging"), PathBuf::from("/var/lib/awg2-harness/swapfile.awg2-harness.new") ); + assert_eq!( + suffixed_path(base, PREVIOUS_SUFFIX).expect("previous"), + PathBuf::from("/var/lib/awg2-harness/swapfile.awg2-harness.previous") + ); + assert_eq!( + suffixed_path(base, LEGACY_OWNERSHIP_SUFFIX).expect("sidecar"), + PathBuf::from("/var/lib/awg2-harness/swapfile.awg2-harness-owned") + ); + } + + /// The legacy sidecar is removed only when it carries the marker the old version + /// wrote; any other file with that name belongs to the operator. + #[test] + fn legacy_sidecar_is_only_removed_when_it_carries_the_old_marker() { + let dir = scratch("sidecar"); + let swapfile = dir.join("swapfile"); + let sidecar = dir.join("swapfile.awg2-harness-owned"); + + fs::write(&sidecar, "# something an operator wrote\n").expect("seed foreign"); + drop_legacy_ownership_sidecar(&swapfile); + assert!(sidecar.exists(), "a foreign file must not be removed"); + + fs::write(&sidecar, format!("{LEGACY_OWNERSHIP_MARKER}\n")).expect("seed ours"); + drop_legacy_ownership_sidecar(&swapfile); + assert!(!sidecar.exists(), "our own sidecar must be removed"); } /// Foreign swaps are identified by not being the managed path; the managed one is