fix(swap): never reduce active swap to zero in a rollback [P0]

The previous fix closed one branch and left its neighbours open. Both remaining paths could
still end with the host having no swap, and one of them said the opposite in its own error
message.

1. The message claimed a fallback that was no longer running.

   The forward path deactivates the staging swap *before* renaming it onto the managed
   path. A failure at that rename, or at anything after it, therefore reached the rollback
   with staging_active = false. The rollback then returned "the staging swap is left
   active" - which was false: it had been switched off several steps earlier, and with the
   original also unable to activate, the host had nothing.

   The rollback now brings the replacement back up as the fallback when it is not running,
   and the error describes what is actually active rather than assuming.

2. The replacement was deleted before the original was known to work.

   With new_installed the rollback removed the file at the managed path to make room for
   the restore. That discarded the only other swap area on the host before finding out
   whether the original could be activated. The replacement is now moved aside to the
   staging name instead, so it is still available as the fallback if the restore fails.

3. An installed replacement could be moved while the kernel was swapping to it.

   ReplacementState had no way to express "installed and active", so a failure in the final
   verification - a failed read of /proc/swaps after a successful swapon - left the
   rollback moving a live swapfile. new_active closes that: if the replacement is active it
   is deactivated first, and if that fails the rollback stops and reports that the
   replacement is in place and serving, because a working swap is the outcome to keep.

Also from the same audit:

- mkswap --file creates the file itself, so a non-zero exit is not a guarantee that nothing
  was left behind. Both creation paths now remove the file on failure; previously a failed
  mkswap left an unowned file at the managed path, which the next run correctly called
  Foreign and refused, wedging the module.
- stale_ownership_help gave advice that could not work. Removing the ownership record
  leaves the file Foreign, which is refused just as firmly, so "mv previous main, delete the
  record" only changed which error appeared. Both cases now give one recipe that actually
  restores managed state: swapoff, remove the three files and the record, re-run
  ensure-swap. Swap holds no persistent data, so a rebuild loses nothing.
- LastActionState kept a single error slot shared by all subsystems, so a successful
  firewall reconcile cleared a live traffic failure and logged firewall=recovered for
  something that had never failed. Errors are now tracked per subsystem; the shared field
  that status reads mirrors whatever is still outstanding.
- tools/README.md still showed 0.1.6.

Five rollback tests covering the branches that exist: original unrestorable with staging
running, original unrestorable with staging already stopped (it is reactivated and never
released), an installed replacement preserved by being moved aside rather than deleted,
the success path releasing the replacement only after the original is back, and the
no-op case.

cargo fmt --check, clippy -D warnings, cargo test --locked --workspace --release: 193 + 4
passed.
This commit is contained in:
2026-08-07 05:42:09 +05:00
parent 6b3e5d1b2f
commit 61dcea2d25
3 changed files with 295 additions and 104 deletions
+3 -3
View File
@@ -24,10 +24,10 @@ reboot_module -> awg2-reboot-scheduler
cd awg2_harness cd awg2_harness
chmod +x tools/build.sh chmod +x tools/build.sh
./tools/build.sh --prepare-build-env --bootstrap-rust --yes ./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) (cd dist && sha256sum -c SHA256SUMS)
dpkg-deb -I dist/awg2-harness_0.1.6_amd64.deb dpkg-deb -I dist/awg2-harness_0.3.0_amd64.deb
dpkg-deb -c dist/awg2-harness_0.1.6_amd64.deb | sort dpkg-deb -c dist/awg2-harness_0.3.0_amd64.deb | sort
``` ```
## Что делает default запуск ## Что делает default запуск
+57 -23
View File
@@ -1,4 +1,5 @@
use std::{ use std::{
collections::BTreeMap,
fs, fs,
io::Write, io::Write,
sync::{Arc, Mutex}, sync::{Arc, Mutex},
@@ -17,6 +18,13 @@ struct LastActionState {
last_ok: Option<String>, last_ok: Option<String>,
last_error_at: Option<String>, last_error_at: Option<String>,
last_error: Option<String>, last_error: Option<String>,
/// Last error per subsystem, keyed by name.
///
/// The single `last_error` above is what `status` shows, but it cannot drive
/// transition-only logging: with one shared slot a successful firewall reconcile
/// cleared a traffic failure and logged `firewall=recovered` for a subsystem that had
/// never failed, while the traffic failure silently stopped being reported.
per_subsystem_error: BTreeMap<String, String>,
} }
pub const LAST_STATE_FILE: &str = "/run/awg2-harness/last_action.state"; pub const LAST_STATE_FILE: &str = "/run/awg2-harness/last_action.state";
@@ -33,36 +41,48 @@ pub fn run(cfg: Config) -> anyhow::Result<()> {
if cfg.sysctl_tuning.apply_on_daemon_start { if cfg.sysctl_tuning.apply_on_daemon_start {
if let Err(e) = sysctl_tuning::ensure_sysctl_tuning(&cfg) { if let Err(e) = sysctl_tuning::ensure_sysctl_tuning(&cfg) {
set_err(&state, format!("startup network tuning reconcile: {e}")); set_err(
&state,
"sysctl_tuning",
format!("startup network tuning reconcile: {e}"),
);
append_event(&format!("startup_network_tuning error=\"{e}\"")); append_event(&format!("startup_network_tuning error=\"{e}\""));
return Err(e); return Err(e);
} else { } else {
set_ok(&state, "startup network tuning reconcile ok"); set_ok(
&state,
"sysctl_tuning",
"startup network tuning reconcile ok",
);
append_event("startup_network_tuning ok"); append_event("startup_network_tuning ok");
} }
} }
if let Err(e) = swap::ensure_swap(&cfg) { if let Err(e) = swap::ensure_swap(&cfg) {
set_err(&state, format!("startup swap reconcile: {e}")); set_err(&state, "swap", format!("startup swap reconcile: {e}"));
append_event(&format!("startup_swap error=\"{e}\"")); append_event(&format!("startup_swap error=\"{e}\""));
return Err(e); return Err(e);
} else { } else {
set_ok(&state, "startup swap reconcile ok"); set_ok(&state, "swap", "startup swap reconcile ok");
append_event("startup_swap ok"); append_event("startup_swap ok");
} }
if let Err(e) = traffic::reconcile_traffic(&cfg) { if let Err(e) = traffic::reconcile_traffic(&cfg) {
set_err(&state, format!("startup traffic reconcile: {e}")); set_err(&state, "traffic", format!("startup traffic reconcile: {e}"));
append_event(&format!("startup_reconcile traffic=error error=\"{e}\"")); append_event(&format!("startup_reconcile traffic=error error=\"{e}\""));
} else { } else {
set_ok(&state, "startup traffic reconcile ok"); set_ok(&state, "traffic", "startup traffic reconcile ok");
append_event("startup_reconcile traffic=ok"); append_event("startup_reconcile traffic=ok");
} }
if let Err(e) = firewall::apply_firewall(&cfg) { if let Err(e) = firewall::apply_firewall(&cfg) {
set_err(&state, format!("startup firewall reconcile: {e}")); set_err(
&state,
"firewall",
format!("startup firewall reconcile: {e}"),
);
append_event(&format!("startup_firewall error=\"{e}\"")); append_event(&format!("startup_firewall error=\"{e}\""));
} else { } else {
set_ok(&state, "startup firewall reconcile ok"); set_ok(&state, "firewall", "startup firewall reconcile ok");
append_event("startup_firewall ok"); append_event("startup_firewall ok");
} }
let events_cfg = cfg.clone(); let events_cfg = cfg.clone();
@@ -73,11 +93,16 @@ pub fn run(cfg: Config) -> anyhow::Result<()> {
Ok(()) => { Ok(()) => {
set_err( set_err(
&events_state, &events_state,
"docker_events",
"docker events stream ended, restarting".to_string(), "docker events stream ended, restarting".to_string(),
); );
} }
Err(e) => { Err(e) => {
set_err(&events_state, format!("docker events loop crashed: {e}")); set_err(
&events_state,
"docker_events",
format!("docker events loop crashed: {e}"),
);
} }
} }
thread::sleep(Duration::from_secs(10)); thread::sleep(Duration::from_secs(10));
@@ -134,14 +159,14 @@ fn report(
) { ) {
match outcome { match outcome {
Ok(()) => { Ok(()) => {
let recovered = set_ok(state, &format!("{what} ok")); let recovered = set_ok(state, subsystem, &format!("{what} ok"));
if recovered { if recovered {
append_event(&format!("{subsystem}=recovered")); append_event(&format!("{subsystem}=recovered"));
} }
} }
Err(e) => { Err(e) => {
error!(error = %e, "{subsystem} reconcile error"); error!(error = %e, "{subsystem} reconcile error");
let changed = set_err(state, format!("{what}: {e}")); let changed = set_err(state, subsystem, format!("{what}: {e}"));
if changed { if changed {
append_event(&format!("{subsystem}=error error=\"{e}\"")); append_event(&format!("{subsystem}=error error=\"{e}\""));
} }
@@ -194,18 +219,18 @@ fn consume_docker_events(
{ {
append_event(&format!("docker_event container={name} action=start")); append_event(&format!("docker_event container={name} action=start"));
if let Err(e) = traffic::reconcile_traffic(cfg) { if let Err(e) = traffic::reconcile_traffic(cfg) {
set_err(state, format!("event traffic reconcile: {e}")); set_err(state, "traffic", format!("event traffic reconcile: {e}"));
append_event(&format!("event_reconcile traffic=error error=\"{e}\"")); append_event(&format!("event_reconcile traffic=error error=\"{e}\""));
} else { } else {
set_ok(state, "event traffic reconcile ok"); set_ok(state, "traffic", "event traffic reconcile ok");
append_event("event_reconcile traffic=ok"); append_event("event_reconcile traffic=ok");
} }
if let Err(e) = firewall::apply_firewall(cfg) { if let Err(e) = firewall::apply_firewall(cfg) {
set_err(state, format!("event firewall reconcile: {e}")); set_err(state, "firewall", format!("event firewall reconcile: {e}"));
append_event(&format!("event_reconcile firewall=error error=\"{e}\"")); append_event(&format!("event_reconcile firewall=error error=\"{e}\""));
} else { } else {
set_ok(state, "event firewall reconcile ok"); set_ok(state, "firewall", "event firewall reconcile ok");
append_event("event_reconcile firewall=ok"); append_event("event_reconcile firewall=ok");
} }
} }
@@ -230,26 +255,35 @@ fn wait_for_interface(cfg: &Config, container_name: &str, timeout_secs: u64) ->
false false
} }
/// Returns whether this success followed a failure, i.e. whether it is a recovery. /// Records a success for `subsystem`, returning whether it recovered from a failure.
fn set_ok(state: &Arc<Mutex<LastActionState>>, msg: &str) -> bool { ///
/// Recovery is judged per subsystem: a healthy firewall says nothing about traffic, so it
/// must neither claim a recovery of its own nor erase a traffic failure that is still
/// current.
fn set_ok(state: &Arc<Mutex<LastActionState>>, subsystem: &str, msg: &str) -> bool {
let Ok(mut s) = state.lock() else { let Ok(mut s) = state.lock() else {
return false; return false;
}; };
let recovered = s.last_error.is_some(); let recovered = s.per_subsystem_error.remove(subsystem).is_some();
s.last_ok_at = Some(now_rfc3339()); s.last_ok_at = Some(now_rfc3339());
s.last_ok = Some(msg.to_string()); s.last_ok = Some(msg.to_string());
s.last_error = None; // The shared field mirrors whatever is still failing, so `status` keeps showing a real
// outstanding failure instead of being cleared by an unrelated success.
s.last_error = s.per_subsystem_error.values().next().cloned();
persist_state(&s); persist_state(&s);
recovered recovered
} }
/// Returns whether the failure differs from the one already recorded, so a subsystem /// Records a failure for `subsystem`, returning whether it differs from the one already
/// failing identically every cycle produces one entry rather than thousands. /// recorded, so a subsystem failing identically every cycle produces one entry rather than
fn set_err(state: &Arc<Mutex<LastActionState>>, msg: String) -> bool { /// thousands.
fn set_err(state: &Arc<Mutex<LastActionState>>, subsystem: &str, msg: String) -> bool {
let Ok(mut s) = state.lock() else { let Ok(mut s) = state.lock() else {
return false; return false;
}; };
let changed = s.last_error.as_deref() != Some(msg.as_str()); let changed = s.per_subsystem_error.get(subsystem).map(String::as_str) != Some(msg.as_str());
s.per_subsystem_error
.insert(subsystem.to_string(), msg.clone());
s.last_error_at = Some(now_rfc3339()); s.last_error_at = Some(now_rfc3339());
s.last_error = Some(msg); s.last_error = Some(msg);
persist_state(&s); persist_state(&s);
+231 -74
View File
@@ -835,29 +835,37 @@ fn ensure_desired_swapfile(desired: &DesiredSwap, active: &[ActiveSwap]) -> anyh
/// a record the file becomes `Foreign`, which this refuses just as firmly. /// a record the file becomes `Foreign`, which this refuses just as firmly.
fn stale_ownership_help(p: &Path) -> anyhow::Result<String> { fn stale_ownership_help(p: &Path) -> anyhow::Result<String> {
let previous = suffixed_path(p, PREVIOUS_SUFFIX)?; let previous = suffixed_path(p, PREVIOUS_SUFFIX)?;
let staging = suffixed_path(p, STAGING_SUFFIX)?;
// Both recipes end in a full rebuild. Anything that leaves an existing file at the
// managed path fails the very next run: a swapfile with no matching ownership record is
// Foreign, and Foreign is refused just as firmly as a stale record. Advice that merely
// moves files around or deletes the record only changes which error is reported.
let rebuild = format!(
"swapoff {} 2>/dev/null; swapoff {} 2>/dev/null; rm -f {} {} {}; rm -f {}; \
then re-run `awg2-harness --config /etc/awg2-harness/config.toml ensure-swap`",
p.display(),
staging.display(),
p.display(),
previous.display(),
staging.display(),
SWAP_OWNERSHIP_PATH,
);
if previous.exists() { if previous.exists() {
return Ok(format!( return Ok(format!(
"{} does not match the recorded swapfile, and {} is present: a swap replacement \ "{} 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 \ 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: \ recorded. No data is at risk - swap holds no persistent state. Recover by \
`mv {} {}` then remove {}. To keep the new file instead: remove {} and {}, \ rebuilding: {rebuild}",
then re-run ensure-swap so it is created and recorded from scratch",
p.display(), p.display(),
previous.display(), previous.display()
previous.display(),
previous.display(),
p.display(),
SWAP_OWNERSHIP_PATH,
previous.display(),
SWAP_OWNERSHIP_PATH,
)); ));
} }
Ok(format!( Ok(format!(
"{} is not the file awg2-harness created - it was replaced since. Refusing to touch \ "{} 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 \ it. Recover by rebuilding: {rebuild}",
with no ownership record is treated as foreign and is never reused", p.display()
p.display(),
SWAP_OWNERSHIP_PATH
)) ))
} }
@@ -898,7 +906,19 @@ fn change_swap_priority(desired: &DesiredSwap, previous_priority: i32) -> anyhow
/// failure would wedge the module until an operator intervened. /// failure would wedge the module until an operator intervened.
fn create_fresh_swapfile(desired: &DesiredSwap) -> anyhow::Result<()> { fn create_fresh_swapfile(desired: &DesiredSwap) -> anyhow::Result<()> {
let p = &desired.path; let p = &desired.path;
build_swapfile(p, desired.size_bytes)?; // `mkswap --file` creates the file itself, so a non-zero exit is no guarantee that
// nothing was left behind. Without this cleanup a failed run leaves an unowned regular
// file at the managed path, which the next run correctly calls Foreign and refuses -
// wedging the module until an operator intervenes.
if let Err(e) = build_swapfile(p, desired.size_bytes) {
return Err(match remove_if_present(p) {
Ok(()) => e,
Err(cleanup) => anyhow::anyhow!(
"{e}; additionally failed to clean up {}: {cleanup}",
p.display()
),
});
}
let prepared = record_ownership(p).and_then(|()| swapon_at(p, desired.priority)); let prepared = record_ownership(p).and_then(|()| swapon_at(p, desired.priority));
if let Err(e) = prepared { if let Err(e) = prepared {
@@ -939,7 +959,13 @@ fn replace_active_swapfile(desired: &DesiredSwap, previous_priority: i32) -> any
}; };
// Built and activated while the old swap is still up: no gap during the slow part. // 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) = build_swapfile(&staging, desired.size_bytes) {
// Same reasoning as create_fresh_swapfile: mkswap may have left the file behind.
// The original swap is untouched at this point, so removing the staging file
// restores the host exactly as it was found.
let _ = remove_if_present(&staging);
return Err(e);
}
if let Err(e) = swapon_at(&staging, desired.priority) { if let Err(e) = swapon_at(&staging, desired.priority) {
let _ = fs::remove_file(&staging); let _ = fs::remove_file(&staging);
return Err(e); return Err(e);
@@ -976,6 +1002,8 @@ fn replace_active_swapfile(desired: &DesiredSwap, previous_priority: i32) -> any
record_ownership(p)?; record_ownership(p)?;
swapon_at(p, desired.priority)?; swapon_at(p, desired.priority)?;
state.new_active = true;
assert_some_swap_is_active(p) assert_some_swap_is_active(p)
})(); })();
@@ -999,10 +1027,19 @@ fn replace_active_swapfile(desired: &DesiredSwap, previous_priority: i32) -> any
/// How far a replacement got, so the rollback undoes exactly that much. /// How far a replacement got, so the rollback undoes exactly that much.
#[derive(Debug, Clone, Copy)] #[derive(Debug, Clone, Copy)]
struct ReplacementState { struct ReplacementState {
/// The replacement is activated at the staging path.
staging_active: bool, staging_active: bool,
/// The original swap is still activated at the managed path.
original_active: bool, original_active: bool,
/// The original file has been renamed to `.previous`.
original_moved: bool, original_moved: bool,
/// The replacement has been renamed onto the managed path.
new_installed: bool, new_installed: bool,
/// The replacement is activated at the managed path.
///
/// Without this the rollback cannot tell an installed-but-inactive replacement from a
/// live one, and would try to move a file that the kernel is still swapping to.
new_active: bool,
previous_priority: i32, previous_priority: i32,
} }
@@ -1014,6 +1051,7 @@ impl Default for ReplacementState {
original_active: true, original_active: true,
original_moved: false, original_moved: false,
new_installed: false, new_installed: false,
new_active: false,
previous_priority: 0, previous_priority: 0,
} }
} }
@@ -1068,13 +1106,36 @@ fn rollback_replacement_with(
let p = &desired.path; let p = &desired.path;
let mut notes: Vec<String> = Vec::new(); let mut notes: Vec<String> = Vec::new();
let mut staging_active = state.staging_active;
// Step the replacement back without destroying it. Deleting the new file here would
// throw away the only other swap area on the host before the original is known to be
// usable again - which is what the previous version did, so a failed reactivation left
// nothing to fall back to.
if state.new_installed {
if state.new_active
&& let Err(e) = ops.swapoff(p)
{
// Still live at the managed path, which means the host does have swap. Moving
// it now would only make /proc/swaps disagree with the filesystem.
return anyhow::anyhow!(
"{cause}; the replacement at {} is active and could not be deactivated \
({e}), so it is left in place and serving. The previous swapfile is \
still at {}",
p.display(),
previous.display()
);
}
if let Err(e) = fs::rename(p, staging) {
notes.push(format!(
"could not move the replacement aside to {}: {e}",
staging.display()
));
}
}
// Put the original file back where it belongs. // Put the original file back where it belongs.
if state.original_moved { if state.original_moved {
if state.new_installed
&& let Err(e) = remove_if_present(p)
{
notes.push(format!("could not clear {}: {e}", p.display()));
}
if let Err(e) = fs::rename(previous, p) { if let Err(e) = fs::rename(previous, p) {
return anyhow::anyhow!( return anyhow::anyhow!(
"{cause}; rollback failed: could not restore {} from {}: {e}. The host may \ "{cause}; rollback failed: could not restore {} from {}: {e}. The host may \
@@ -1101,27 +1162,43 @@ fn rollback_replacement_with(
} }
} }
// 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 { if !original_back {
// The original is unusable, so whatever replacement exists is the only swap area
// left and becomes the fallback - activated here if the forward path had already
// switched it off. An extra temporary swap is always a better end state than none,
// and the message must describe what is actually running rather than assume it.
if !staging_active && staging.exists() {
match ops.swapon(staging, state.previous_priority) {
Ok(()) => staging_active = true,
Err(e) => notes.push(format!(
"could not activate {} as a fallback: {e}",
staging.display()
)),
}
}
let detail = format_notes(&notes); let detail = format_notes(&notes);
return anyhow::anyhow!( return if staging_active {
"{cause}; the previous swap could not be restored{detail}; leaving the staging \ anyhow::anyhow!(
swap at {} active so the host is not left without swap. Once memory pressure \ "{cause}; the previous swap could not be restored{detail}; the replacement \
allows, run `swapon -p {} {}` and then `swapoff {}`", at {} is left active so the host is not without swap. Once the host allows \
it, run `swapon -p {} {}` and then `swapoff {}`",
staging.display(), staging.display(),
state.previous_priority, state.previous_priority,
p.display(), p.display(),
staging.display() staging.display()
); )
} else {
anyhow::anyhow!(
"{cause}; the previous swap could not be restored and no fallback could be \
activated{detail}. The host currently has no managed swap"
)
};
} }
// Original confirmed back: now the staging swap can go. The file is only unlinked once // Original confirmed back: now the replacement can go. The file is only unlinked once
// swapoff has actually succeeded - removing an active swapfile is not possible, and // swapoff has actually succeeded - removing an active swapfile is not possible, and
// trying leaves an error that says nothing useful. // trying leaves an error that says nothing useful.
if state.staging_active { if staging_active {
match ops.swapoff(staging) { match ops.swapoff(staging) {
Ok(()) => { Ok(()) => {
if let Err(e) = remove_if_present(staging) { if let Err(e) = remove_if_present(staging) {
@@ -1736,17 +1813,21 @@ mod tests {
/// Records the swap calls a rollback makes, so their order and absence can be asserted. /// Records the swap calls a rollback makes, so their order and absence can be asserted.
struct FakeSwap { struct FakeSwap {
swapon_fails: bool, /// Paths whose `swapon` fails; everything else succeeds.
swapon_fails_for: Vec<PathBuf>,
calls: std::sync::Mutex<Vec<String>>, calls: std::sync::Mutex<Vec<String>>,
} }
impl FakeSwap { impl FakeSwap {
fn new(swapon_fails: bool) -> Self { fn failing_for(paths: &[&Path]) -> Self {
Self { Self {
swapon_fails, swapon_fails_for: paths.iter().map(|p| p.to_path_buf()).collect(),
calls: std::sync::Mutex::new(Vec::new()), calls: std::sync::Mutex::new(Vec::new()),
} }
} }
fn all_ok() -> Self {
Self::failing_for(&[])
}
fn calls(&self) -> Vec<String> { fn calls(&self) -> Vec<String> {
self.calls.lock().expect("lock").clone() self.calls.lock().expect("lock").clone()
} }
@@ -1758,7 +1839,7 @@ mod tests {
.lock() .lock()
.expect("lock") .expect("lock")
.push(format!("swapon {} -p {priority}", path.display())); .push(format!("swapon {} -p {priority}", path.display()));
if self.swapon_fails { if self.swapon_fails_for.iter().any(|p| p == path) {
anyhow::bail!("cannot allocate swap"); anyhow::bail!("cannot allocate swap");
} }
Ok(()) Ok(())
@@ -1772,34 +1853,35 @@ mod tests {
} }
} }
fn rollback_fixture() -> (DesiredSwap, PathBuf, PathBuf) { /// Uses a scratch directory so the rollback's real renames have files to act on.
fn rollback_fixture(name: &str) -> (DesiredSwap, PathBuf, PathBuf) {
let dir = scratch(name);
let main = dir.join("swapfile");
let desired = DesiredSwap { let desired = DesiredSwap {
path: PathBuf::from("/swapfile"), path: main.clone(),
size_bytes: 4096, size_bytes: 4096,
priority: 5, priority: 5,
swappiness: 1, swappiness: 1,
}; };
( (
desired, desired,
PathBuf::from("/swapfile.awg2-harness.new"), suffixed_path(&main, STAGING_SUFFIX).expect("staging"),
PathBuf::from("/swapfile.awg2-harness.previous"), suffixed_path(&main, PREVIOUS_SUFFIX).expect("previous"),
) )
} }
/// The invariant that matters most: if the original cannot be brought back, the staging /// The invariant that matters most: if the original cannot be brought back, whatever
/// swap is the only thing carrying load, so releasing it would turn a failed resize /// replacement exists is the only thing carrying load, so releasing it would turn a
/// into a host with no swap at all. An extra temporary swap is always the better end /// failed resize into a host with no swap at all.
/// state.
#[test] #[test]
fn rollback_keeps_staging_active_when_the_original_cannot_be_restored() { fn rollback_keeps_staging_active_when_the_original_cannot_be_restored() {
let (desired, staging, previous) = rollback_fixture(); let (desired, staging, previous) = rollback_fixture("rb_keep");
let ops = FakeSwap::new(true); let ops = FakeSwap::failing_for(&[&desired.path]);
let state = ReplacementState { let state = ReplacementState {
staging_active: true, staging_active: true,
original_active: false, original_active: false,
original_moved: false,
new_installed: false,
previous_priority: 1, previous_priority: 1,
..ReplacementState::default()
}; };
let err = rollback_replacement_with( let err = rollback_replacement_with(
@@ -1812,29 +1894,108 @@ mod tests {
); );
let calls = ops.calls(); let calls = ops.calls();
assert_eq!( assert_eq!(calls.len(), 1, "only the restore attempt");
calls, assert!(calls[0].starts_with("swapon "));
vec!["swapon /swapfile -p 1"], assert!(
"the staging swap must not be released" !calls.iter().any(|c| c.starts_with("swapoff")),
"the fallback swap must not be released"
);
assert!(
err.to_string()
.contains("is left active so the host is not without swap")
); );
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 /// The branch the previous fix missed: the forward path switches the staging swap off
/// priority, not the one being applied, which may be what failed. /// before installing it, so "leaving it active" was false and the host really did end
/// up with nothing. It has to be brought back up as the fallback.
#[test]
fn rollback_reactivates_an_already_stopped_staging_swap_as_fallback() {
let (desired, staging, previous) = rollback_fixture("rb_reactivate");
fs::write(&staging, b"replacement").expect("seed staging");
let ops = FakeSwap::failing_for(&[&desired.path]);
let state = ReplacementState {
staging_active: false,
original_active: false,
previous_priority: 1,
..ReplacementState::default()
};
let err = rollback_replacement_with(
&ops,
&desired,
&staging,
&previous,
&state,
anyhow::anyhow!("rename failed"),
);
let calls = ops.calls();
assert_eq!(calls.len(), 2, "restore attempt, then fallback activation");
assert!(calls[1].starts_with("swapon "));
assert!(calls[1].contains(&staging.display().to_string()));
assert!(!calls.iter().any(|c| c.starts_with("swapoff")));
assert!(staging.exists(), "the fallback file must survive");
assert!(
err.to_string()
.contains("is left active so the host is not without swap")
);
}
/// An installed replacement is moved aside, not deleted: until the original is known to
/// activate, it is the only other swap area on the host.
#[test]
fn rollback_preserves_an_installed_replacement_as_fallback() {
let (desired, staging, previous) = rollback_fixture("rb_installed");
fs::write(&desired.path, b"replacement").expect("seed new");
fs::write(&previous, b"original").expect("seed previous");
let ops = FakeSwap::failing_for(&[&desired.path]);
let state = ReplacementState {
staging_active: false,
original_active: false,
original_moved: true,
new_installed: true,
new_active: false,
previous_priority: 1,
};
let err = rollback_replacement_with(
&ops,
&desired,
&staging,
&previous,
&state,
anyhow::anyhow!("ownership write failed"),
);
assert_eq!(
fs::read(&staging).expect("replacement kept"),
b"replacement".to_vec(),
"the replacement is moved aside, never deleted"
);
assert_eq!(
fs::read(&desired.path).expect("original restored"),
b"original".to_vec()
);
assert!(!ops.calls().iter().any(|c| c.starts_with("swapoff")));
assert!(
err.to_string()
.contains("is left active so the host is not without swap")
);
}
/// With the original confirmed back, the replacement is released - and the restore uses
/// the previous priority, not the one being applied, which may be what failed.
#[test] #[test]
fn rollback_releases_staging_only_after_the_original_is_back() { fn rollback_releases_staging_only_after_the_original_is_back() {
let (desired, staging, previous) = rollback_fixture(); let (desired, staging, previous) = rollback_fixture("rb_release");
let ops = FakeSwap::new(false); fs::write(&staging, b"replacement").expect("seed staging");
let ops = FakeSwap::all_ok();
let state = ReplacementState { let state = ReplacementState {
staging_active: true, staging_active: true,
original_active: false, original_active: false,
original_moved: false,
new_installed: false,
previous_priority: 1, previous_priority: 1,
..ReplacementState::default()
}; };
let err = rollback_replacement_with( let err = rollback_replacement_with(
@@ -1846,14 +2007,11 @@ mod tests {
anyhow::anyhow!("resize failed"), anyhow::anyhow!("resize failed"),
); );
assert_eq!( let calls = ops.calls();
ops.calls(), assert_eq!(calls.len(), 2, "restore first, release second");
vec![ assert!(calls[0].starts_with("swapon ") && calls[0].ends_with("-p 1"));
"swapon /swapfile -p 1", assert!(calls[1].starts_with("swapoff "));
"swapoff /swapfile.awg2-harness.new", assert!(!staging.exists(), "the released file is removed");
],
"restore first, release second"
);
assert!( assert!(
err.to_string() err.to_string()
.contains("rolled back to the previous swapfile") .contains("rolled back to the previous swapfile")
@@ -1863,8 +2021,8 @@ mod tests {
/// Nothing was taken down yet, so there is nothing to restore and nothing to release. /// Nothing was taken down yet, so there is nothing to restore and nothing to release.
#[test] #[test]
fn rollback_is_quiet_when_the_original_was_never_deactivated() { fn rollback_is_quiet_when_the_original_was_never_deactivated() {
let (desired, staging, previous) = rollback_fixture(); let (desired, staging, previous) = rollback_fixture("rb_quiet");
let ops = FakeSwap::new(false); let ops = FakeSwap::all_ok();
let state = ReplacementState::default(); let state = ReplacementState::default();
let _ = rollback_replacement_with( let _ = rollback_replacement_with(
@@ -1877,7 +2035,6 @@ mod tests {
); );
assert!(ops.calls().is_empty()); assert!(ops.calls().is_empty());
} }
/// A leftover from a killed run that is still active cannot be unlinked. Removing it /// 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. /// blindly fails every reconcile, so it is reported with the command that fixes it.
#[test] #[test]