diff --git a/Cargo.lock b/Cargo.lock index cd304cc..55784ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -84,7 +84,7 @@ checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" [[package]] name = "awg2-harness" -version = "0.1.6" +version = "0.3.0" dependencies = [ "anyhow", "chrono", diff --git a/README.md b/README.md index e2d1933..f578d42 100644 --- a/README.md +++ b/README.md @@ -188,10 +188,10 @@ Security note: ```bash cd awg2_harness ./tools/build.sh --prepare-build-env --bootstrap-rust --yes -./tools/build.sh --package --version 0.1.6 +./tools/build.sh --package --version 0.3.0 (cd dist && sha256sum -c SHA256SUMS) -dpkg-deb -I dist/awg2-harness_0.1.6_amd64.deb -dpkg-deb -c dist/awg2-harness_0.1.6_amd64.deb | sort +dpkg-deb -I dist/awg2-harness_0.3.0_amd64.deb +dpkg-deb -c dist/awg2-harness_0.3.0_amd64.deb | sort ``` `--version` можно не указывать, если версия в `traffic_core/Cargo.toml` уже соответствует release. Если version override указан, он меняет версию Debian-пакета, а не версию crate в `Cargo.toml`. @@ -224,7 +224,7 @@ scp -P 2323 dist/SHA256SUMS root@:/root/ ```bash cd /root sha256sum -c SHA256SUMS -chmod 644 /root/awg2-harness_0.1.6_amd64.deb +chmod 644 /root/awg2-harness_0.3.0_amd64.deb apt install ./awg2-harness_*_amd64.deb install -d -m 0755 /etc/awg2-harness install -m 0644 /root/awg2-harness.config.toml /etc/awg2-harness/config.toml @@ -237,7 +237,7 @@ install -m 0644 /root/awg2-harness.config.toml /etc/awg2-harness/config.toml ```bash cd /root sha256sum -c SHA256SUMS -chmod 644 /root/awg2-harness_0.1.6_amd64.deb +chmod 644 /root/awg2-harness_0.3.0_amd64.deb apt install ./awg2-harness_*_amd64.deb install -d -m 0755 /etc/awg2-harness cp /usr/share/doc/awg2-harness/repo/config.example.toml /etc/awg2-harness/config.toml diff --git a/docs/ACCEPTANCE_CHECKLIST.md b/docs/ACCEPTANCE_CHECKLIST.md index b9589a8..68755d7 100644 --- a/docs/ACCEPTANCE_CHECKLIST.md +++ b/docs/ACCEPTANCE_CHECKLIST.md @@ -74,7 +74,7 @@ - [ ] На старте daemon сбой `sysctl_tuning` или `swap` завершает процесс, а сбой traffic/firewall только логируется — сервис остаётся живым. - [ ] После обрыва потока `docker events` он перезапускается автоматически (проверяется, например, `systemctl restart docker`). - [ ] Параллельный запуск `apply-limits` во время работы daemon не приводит к гонке: второй процесс ждёт блокировку до 10 секунд и затем сообщает `lock busy timeout`. -- [ ] Устаревший lock-файл после `kill -9` не блокирует работу: следующий запуск перезахватывает `/run/awg2-harness/traffic.lock` по отсутствию `/proc/`. +- [ ] После `kill -9` блокировка освобождается ядром автоматически; оставшийся lock-файл не мешает следующему захвату `/run/awg2-harness/traffic.lock`. ## F. Статус и наблюдаемость diff --git a/docs/INSTALLATION.md b/docs/INSTALLATION.md index c829ce6..f41dded 100644 --- a/docs/INSTALLATION.md +++ b/docs/INSTALLATION.md @@ -40,10 +40,10 @@ cargo test --locked --workspace --release --manifest-path Cargo.toml ### A3. Собрать Debian-пакет ```bash -./tools/build.sh --package --version 0.1.6 +./tools/build.sh --package --version 0.3.0 (cd dist && sha256sum -c SHA256SUMS) -dpkg-deb -I dist/awg2-harness_0.1.6_amd64.deb -dpkg-deb -c dist/awg2-harness_0.1.6_amd64.deb | sort +dpkg-deb -I dist/awg2-harness_0.3.0_amd64.deb +dpkg-deb -c dist/awg2-harness_0.3.0_amd64.deb | sort ``` `--version` можно опустить, если версия в `traffic_core/Cargo.toml` уже равна release version. Version override влияет только на metadata `.deb`. diff --git a/tools/build.sh b/tools/build.sh index 53016ea..55c86f5 100644 --- a/tools/build.sh +++ b/tools/build.sh @@ -205,6 +205,11 @@ prepare_build_gate() { require_cmd pkg-config require_cmd file require_cmd ldd + require_cmd shellcheck + + # This script installs systemd units and writes to /usr/sbin, so it is part of the + # release surface and is linted like the Rust code is. + shellcheck "$ROOT_DIR/tools/build.sh" if [[ ! -f "$RUST_TOOLCHAIN_FILE" ]]; then echo "Missing rust toolchain pin file: $RUST_TOOLCHAIN_FILE" @@ -240,7 +245,7 @@ prepare_build_gate() { systemd-analyze verify "$ROOT_DIR/awg2-harness.service" systemd-analyze verify "$ROOT_DIR/awg2-reboot-scheduler.service" else - echo "Skipping source-tree systemd-analyze verify: package runtime files or docker.service are not present on this build host" + echo "Skipping source-tree systemd-analyze verify: package runtime files or docker.service are not present on this build host."; echo "This check is a post-install gate, not a build gate: on a clean builder it is expected to be skipped, and the units are verified after --install or on the target host." fi fi diff --git a/traffic_core/Cargo.toml b/traffic_core/Cargo.toml index c0223e8..46a0686 100644 --- a/traffic_core/Cargo.toml +++ b/traffic_core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "awg2-harness" -version = "0.1.6" +version = "0.3.0" edition = "2024" [dependencies] diff --git a/traffic_core/src/swap.rs b/traffic_core/src/swap.rs index 3b995b5..d3b456b 100644 --- a/traffic_core/src/swap.rs +++ b/traffic_core/src/swap.rs @@ -772,12 +772,7 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh refusing to touch it. Remove it manually or point swap.path elsewhere", p.display() ), - SwapfileOwnership::StaleOwnership => anyhow::bail!( - "{} is not the file awg2-harness created - it was replaced since. Refusing to \ - touch it. Remove it manually, or delete {SWAP_OWNERSHIP_PATH} to let \ - awg2-harness adopt a freshly created swapfile", - p.display() - ), + SwapfileOwnership::StaleOwnership => anyhow::bail!("{}", stale_ownership_help(p)?), _ => {} } @@ -829,6 +824,43 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh } } +/// Explains a stale ownership record, distinguishing the two ways it happens. +/// +/// A leftover `.previous` next to the swapfile is the signature of a replacement that was +/// killed between installing the new file and recording its identity: the path holds the +/// new inode while the record still describes the old one. That is recoverable by hand and +/// deserves the exact commands, not the generic message. +/// +/// The generic advice must also not say "delete the ownership record" on its own - without +/// a record the file becomes `Foreign`, which this refuses just as firmly. +fn stale_ownership_help(p: &Path) -> anyhow::Result { + let previous = suffixed_path(p, PREVIOUS_SUFFIX)?; + if previous.exists() { + return Ok(format!( + "{} does not match the recorded swapfile, and {} is present: a swap replacement \ + was interrupted after the new file was installed but before its identity was \ + recorded. Nothing is lost - the previous swapfile is still at {}. To go back: \ + `mv {} {}` then remove {}. To keep the new file instead: remove {} and {}, \ + then re-run ensure-swap so it is created and recorded from scratch", + p.display(), + previous.display(), + previous.display(), + previous.display(), + p.display(), + SWAP_OWNERSHIP_PATH, + previous.display(), + SWAP_OWNERSHIP_PATH, + )); + } + Ok(format!( + "{} is not the file awg2-harness created - it was replaced since. Refusing to touch \ + it. Move it aside or delete it, and remove {}, then re-run ensure-swap: a swapfile \ + with no ownership record is treated as foreign and is never reused", + p.display(), + SWAP_OWNERSHIP_PATH + )) +} + /// Re-activates the managed swap at a new priority, restoring the old one on failure. /// /// `swapoff` followed by `swapon` is two steps, and the second can fail - a bad priority, @@ -999,6 +1031,39 @@ fn rollback_replacement( previous: &Path, state: &ReplacementState, cause: anyhow::Error, +) -> anyhow::Error { + rollback_replacement_with(&SystemSwap, desired, staging, previous, state, cause) +} + +/// The two kernel operations the rollback depends on. +/// +/// Behind a trait so the recovery order can be tested. The invariant that matters - +/// never release the staging swap while the original is not back - is only observable as a +/// sequence of calls, so asserting it needs a seam rather than another pure helper. +trait SwapCommands { + fn swapon(&self, path: &Path, priority: i32) -> anyhow::Result<()>; + fn swapoff(&self, path: &Path) -> anyhow::Result<()>; +} + +struct SystemSwap; + +impl SwapCommands for SystemSwap { + fn swapon(&self, path: &Path, priority: i32) -> anyhow::Result<()> { + swapon_at(path, priority) + } + + fn swapoff(&self, path: &Path) -> anyhow::Result<()> { + run("swapoff", &[path.to_string_lossy().as_ref()]) + } +} + +fn rollback_replacement_with( + ops: &dyn SwapCommands, + desired: &DesiredSwap, + staging: &Path, + previous: &Path, + state: &ReplacementState, + cause: anyhow::Error, ) -> anyhow::Error { let p = &desired.path; let mut notes: Vec = Vec::new(); @@ -1026,7 +1091,7 @@ fn rollback_replacement( // Reactivate it if we took it down, at the priority it had before. let mut original_back = state.original_active; if !state.original_active { - match swapon_at(p, state.previous_priority) { + match ops.swapon(p, state.previous_priority) { Ok(()) => original_back = true, Err(e) => notes.push(format!( "could not reactivate {} at priority {}: {e}", @@ -1036,32 +1101,53 @@ fn rollback_replacement( } } - // Only now release the staging swap, so the two are never both down. + // The staging swap is the only thing still carrying load at this point. Releasing it + // before the original is confirmed back would turn a failed resize into a host with no + // swap at all - the rollback would be doing more damage than the failure it recovers + // from. An extra temporary swap is always the better end state. + if !original_back { + let detail = format_notes(¬es); + return anyhow::anyhow!( + "{cause}; the previous swap could not be restored{detail}; leaving the staging \ + swap at {} active so the host is not left without swap. Once memory pressure \ + allows, run `swapon -p {} {}` and then `swapoff {}`", + staging.display(), + state.previous_priority, + p.display(), + staging.display() + ); + } + + // Original confirmed back: now the staging swap can go. The file is only unlinked once + // swapoff has actually succeeded - removing an active swapfile is not possible, and + // trying leaves an error that says nothing useful. if state.staging_active { - match run("swapoff", &[staging.to_string_lossy().as_ref()]) { - Ok(()) => {} + match ops.swapoff(staging) { + Ok(()) => { + if let Err(e) = remove_if_present(staging) { + notes.push(format!("inactive staging file left behind: {e}")); + } + } Err(e) => notes.push(format!( - "staging swap {} is still active: {e}", + "previous swap is restored, but the staging swap {} is still active: {e}", staging.display() )), } - } - if let Err(e) = remove_if_present(staging) { - notes.push(format!("staging file left behind: {e}")); + } else if let Err(e) = remove_if_present(staging) { + notes.push(format!("inactive staging file left behind: {e}")); } - let detail = if notes.is_empty() { + anyhow::anyhow!( + "{cause}; rolled back to the previous swapfile{}", + format_notes(¬es) + ) +} + +fn format_notes(notes: &[String]) -> String { + if notes.is_empty() { String::new() } else { format!(" ({})", notes.join("; ")) - }; - if original_back { - anyhow::anyhow!("{cause}; rolled back to the previous swapfile{detail}") - } else { - anyhow::anyhow!( - "{cause}; rollback could not restore the previous swap{detail}. The host may \ - currently have no swap" - ) } } @@ -1648,6 +1734,150 @@ mod tests { assert!(!state.new_installed); } + /// Records the swap calls a rollback makes, so their order and absence can be asserted. + struct FakeSwap { + swapon_fails: bool, + calls: std::sync::Mutex>, + } + + impl FakeSwap { + fn new(swapon_fails: bool) -> Self { + Self { + swapon_fails, + calls: std::sync::Mutex::new(Vec::new()), + } + } + fn calls(&self) -> Vec { + self.calls.lock().expect("lock").clone() + } + } + + impl SwapCommands for FakeSwap { + fn swapon(&self, path: &Path, priority: i32) -> anyhow::Result<()> { + self.calls + .lock() + .expect("lock") + .push(format!("swapon {} -p {priority}", path.display())); + if self.swapon_fails { + anyhow::bail!("cannot allocate swap"); + } + Ok(()) + } + fn swapoff(&self, path: &Path) -> anyhow::Result<()> { + self.calls + .lock() + .expect("lock") + .push(format!("swapoff {}", path.display())); + Ok(()) + } + } + + fn rollback_fixture() -> (DesiredSwap, PathBuf, PathBuf) { + let desired = DesiredSwap { + path: PathBuf::from("/swapfile"), + size_bytes: 4096, + priority: 5, + swappiness: 1, + }; + ( + desired, + PathBuf::from("/swapfile.awg2-harness.new"), + PathBuf::from("/swapfile.awg2-harness.previous"), + ) + } + + /// The invariant that matters most: if the original cannot be brought back, the staging + /// swap is the only thing carrying load, so releasing it would turn a failed resize + /// into a host with no swap at all. An extra temporary swap is always the better end + /// state. + #[test] + fn rollback_keeps_staging_active_when_the_original_cannot_be_restored() { + let (desired, staging, previous) = rollback_fixture(); + let ops = FakeSwap::new(true); + let state = ReplacementState { + staging_active: true, + original_active: false, + original_moved: false, + new_installed: false, + previous_priority: 1, + }; + + let err = rollback_replacement_with( + &ops, + &desired, + &staging, + &previous, + &state, + anyhow::anyhow!("resize failed"), + ); + + let calls = ops.calls(); + assert_eq!( + calls, + vec!["swapon /swapfile -p 1"], + "the staging swap must not be released" + ); + assert!(!calls.iter().any(|c| c.starts_with("swapoff"))); + let msg = err.to_string(); + assert!(msg.contains("leaving the staging swap")); + assert!(msg.contains("so the host is not left without swap")); + } + + /// With the original confirmed back, the staging swap is released - and at the previous + /// priority, not the one being applied, which may be what failed. + #[test] + fn rollback_releases_staging_only_after_the_original_is_back() { + let (desired, staging, previous) = rollback_fixture(); + let ops = FakeSwap::new(false); + let state = ReplacementState { + staging_active: true, + original_active: false, + original_moved: false, + new_installed: false, + previous_priority: 1, + }; + + let err = rollback_replacement_with( + &ops, + &desired, + &staging, + &previous, + &state, + anyhow::anyhow!("resize failed"), + ); + + assert_eq!( + ops.calls(), + vec![ + "swapon /swapfile -p 1", + "swapoff /swapfile.awg2-harness.new", + ], + "restore first, release second" + ); + assert!( + err.to_string() + .contains("rolled back to the previous swapfile") + ); + } + + /// Nothing was taken down yet, so there is nothing to restore and nothing to release. + #[test] + fn rollback_is_quiet_when_the_original_was_never_deactivated() { + let (desired, staging, previous) = rollback_fixture(); + let ops = FakeSwap::new(false); + let state = ReplacementState::default(); + + let _ = rollback_replacement_with( + &ops, + &desired, + &staging, + &previous, + &state, + anyhow::anyhow!("resize failed"), + ); + assert!(ops.calls().is_empty()); + } + /// A leftover from a killed run that is still active cannot be unlinked. Removing it /// blindly fails every reconcile, so it is reported with the command that fixes it. #[test]