diff --git a/docs/ACCEPTANCE_CHECKLIST.md b/docs/ACCEPTANCE_CHECKLIST.md index 68755d7..e9ff49c 100644 --- a/docs/ACCEPTANCE_CHECKLIST.md +++ b/docs/ACCEPTANCE_CHECKLIST.md @@ -73,6 +73,8 @@ - [ ] Если контейнер временно отсутствует, daemon логирует ошибку reconcile и продолжает работу. - [ ] На старте daemon сбой `sysctl_tuning` или `swap` завершает процесс, а сбой traffic/firewall только логируется — сервис остаётся живым. - [ ] После обрыва потока `docker events` он перезапускается автоматически (проверяется, например, `systemctl restart docker`). +- [ ] После восстановления потока `docker events` ошибка подсистемы снимается: в `events.log` появляется `docker_events=recovered`, а `last_action.state` не остаётся с этой ошибкой навсегда. +- [ ] Успешный reconcile одной подсистемы не гасит незакрытую ошибку другой: при постоянно падающем `traffic` успешный `firewall` не очищает `last_error` и не пишет `firewall=recovered`. - [ ] Параллельный запуск `apply-limits` во время работы daemon не приводит к гонке: второй процесс ждёт блокировку до 10 секунд и затем сообщает `lock busy timeout`. - [ ] После `kill -9` блокировка освобождается ядром автоматически; оставшийся lock-файл не мешает следующему захвату `/run/awg2-harness/traffic.lock`. @@ -145,7 +147,12 @@ - [ ] Если `swapoff` второго чужого swap падает, ранее отключённые чужие swap возвращаются в активное состояние. - [ ] При откате resize swap возвращается с **прежним** priority, а не с новым (проверяется заведомо неприменимым `swap.priority`). - [ ] Kill процесса в середине resize: следующий запуск с активным `.new` или `.previous` даёт понятную ошибку с командой `swapoff`, а не бесконечно падает на удалении активного файла. +- [ ] Kill процесса **после** активации нового swapfile: следующий `ensure-swap` проходит быстрым путём и удаляет неактивный `.previous`; активный `.previous` при этом не трогается. - [ ] После отката в `/proc/swaps` присутствует ровно исходный swap; `.new` отключён и удалён. +- [ ] Откат при недоступном `rename` обратно (например, основной путь занят каталогом): исходный swapfile активируется прямо по пути `.previous`, в `/proc/swaps` есть запись, ошибка содержит `is serving from` и путь обоих файлов. +- [ ] Откат при неподнимающемся исходном swapfile: активируется отложенная замена, ошибка содержит `is left active so the host is not without swap`; ни один из файлов не удалён. +- [ ] Откат при активной замене, которую не удаётся отключить: файлы не перемещаются, ошибка содержит `is left in place and serving`, замена продолжает обслуживать. +- [ ] Формулировка «хост без управляемого swap» появляется только тогда, когда в `/proc/swaps` действительно нет ни одной managed-записи. - [ ] Нечитаемый managed sysctl-файл даёт в `status` причину `managed sysctl file is unreadable`, а не «файл отсутствует». - [ ] При `reboot.enabled = false` файл `/run/awg2-harness/reboot.plan` удалён, `status` не показывает устаревший план. - [ ] После принятия запроса на перезагрузку `reboot.plan` удалён. diff --git a/docs/IMPLEMENTATION_SPEC.md b/docs/IMPLEMENTATION_SPEC.md index 820864b..b6c8727 100644 --- a/docs/IMPLEMENTATION_SPEC.md +++ b/docs/IMPLEMENTATION_SPEC.md @@ -485,6 +485,8 @@ Status обязан показывать наличие таблицы/chain/п - `/run/awg2-harness/last_action.state` — последний успех и последняя ошибка; - `/run/awg2-harness/events.log` — журнал событий, ограниченный 1000 строками и 256 KiB. +Ошибки отслеживаются по подсистемам (`traffic`, `firewall`, `swap`, `sysctl_tuning`, `docker_events`), а не одним общим полем. Общее поле в `last_action.state` показывает ту ошибку, которая ещё не устранена: успешный reconcile одной подсистемы не гасит незакрытую ошибку другой и не объявляет восстановление того, что не падало. Поток `docker events` при успешном переподключении тоже отмечается как восстановленный — иначе один рестарт Docker оставлял бы ошибку в `status` навсегда при исправно работающем новом потоке. + В `events.log` пишутся только **переходы состояния**: первое появление ошибки, изменение текста ошибки и восстановление после неё. Рутинный успех не пишется. Раньше каждый успешный цикл добавлял строку — при интервале 15 секунд это порядка 17 тысяч записей в сутки, каждая с полным чтением, дополнением и перезаписью файла, и на их фоне терялись записи, ради которых журнал и читают. Полная поцикловая картина остаётся в journald через tracing. ## 14. Блокировки @@ -632,14 +634,31 @@ inode = 123456 3. подготовить управляемый swapfile (с учётом владения) 4. swapon управляемого swap 5. убедиться, что он реально появился в списке активных -6. только теперь swapoff чужих swap -7. перезаписать /etc/fstab транзакцией -8. применить swappiness -9. проверить итоговое состояние +6. удалить неактивные остатки прерванной замены (.new, .previous) +7. только теперь swapoff чужих swap +8. перезаписать /etc/fstab транзакцией +9. применить swappiness +10. проверить итоговое состояние ``` Шаги 2–5 не выполняют ни одного деструктивного действия над чужим состоянием. Прежний порядок отключал чужие swap первым шагом, поэтому ошибка в `fallocate`, `dd`, `mkswap` или `swapon` оставляла хост вообще без swap. +Шаг 6 нужен потому, что вычистка остатков перед заменой (шаг 3) не покрывает процесс, убитый уже **после** активации нового файла: следующий запуск видит по основному пути валидный, активный и корректно принадлежащий нам swapfile, идёт быстрым путём и на остатки не смотрит. Размер swapfile обычно сопоставим с объёмом RAM, поэтому «когда-нибудь» здесь не годится. Удаляются только неактивные остатки; активный означает, что ядро всё ещё свопит в него, — это случай прерванного отката, и он требует оператора, а не догадки. + +### Замена активного swapfile + +Живой swapfile нельзя изменить в размере на месте, поэтому изменение `swap.size_mb` выполняется заменой. Новый файл создаётся и активируется, **пока старый ещё работает**; старый затем не удаляется, а откладывается под именем `.awg2-harness.previous`, и до подтверждения нового файла оба лежат на диске. + +Инвариант отката один: откат никогда не оставляет хост без swap. Он сформулирован не как «вернуть исходное состояние любой ценой» — исходное состояние может быть недостижимо, — а как «на хосте должен работать хотя бы один подготовленный swap». Отсюда порядок восстановления: + +1. активную замену сначала выключить; если `swapoff` не удался, она остаётся на месте и **продолжает обслуживать** — двигать файл, в который ядро свопит, значит только рассинхронизировать `/proc/swaps` с файловой системой; +2. неактивную замену не удалять, а отодвинуть под staging-имя: пока исходный файл не подтверждён рабочим, замена — единственная вторая swap-область на хосте; +3. вернуть исходный файл на основной путь и активировать его с **прежним** priority, а не с применяемым: применяемый мог оказаться ровно тем, что ядро отвергло; +4. если `rename` обратно не удался, исходный файл активируется прямо по пути `.awg2-harness.previous` — ядро активирует swap-область по пути, и имя файла для этого значения не имеет; +5. если исходный файл поднять не удалось вовсе, активируется отложенная замена. + +Соответственно у отката есть три деградированных исхода, и в каждом текст ошибки описывает **фактическое** состояние, а не намерение: замена осталась активной по основному пути; исходный swap работает из `.previous` и оба файла целы; ни один кандидат не активировался — хост без управляемого swap. Первые два сопровождаются точными командами, которыми оператор доводит состояние до нормального, когда хост это позволит. + ### Безопасность `swap.path` Проверка разделена на две части. Лексическая не обращается к файловой системе и потому пригодна для `validate-config` на хосте, где файла ещё нет: diff --git a/traffic_core/src/daemon.rs b/traffic_core/src/daemon.rs index bbca7b0..aad3c55 100644 --- a/traffic_core/src/daemon.rs +++ b/traffic_core/src/daemon.rs @@ -195,6 +195,14 @@ fn docker_events_loop(cfg: &Config, state: &Arc>) -> anyh .ok_or_else(|| anyhow::anyhow!("failed to capture docker events stdout"))?; let reader = BufReader::new(stdout); + // The stream is up again. Without this the subsystem had no way back to healthy: the + // outer loop records an error every time `docker events` ends, and a single Docker + // restart therefore left a permanent failure in `status` while the replacement stream + // was running normally. + if set_ok(state, "docker_events", "docker events stream connected") { + append_event("docker_events=recovered"); + } + let result = consume_docker_events(cfg, state, reader); // Without this the finished `docker events` process stays a zombie: dropping a Child @@ -255,37 +263,53 @@ fn wait_for_interface(cfg: &Config, container_name: &str, timeout_secs: u64) -> false } -/// Records a success for `subsystem`, returning whether it recovered from a failure. -/// -/// 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. +impl LastActionState { + /// Records a success for `subsystem`, returning whether it recovered from a failure. + /// + /// 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 record_ok(&mut self, subsystem: &str, msg: &str) -> bool { + let recovered = self.per_subsystem_error.remove(subsystem).is_some(); + self.last_ok_at = Some(now_rfc3339()); + self.last_ok = Some(msg.to_string()); + // The shared field mirrors whatever is still failing, so `status` keeps showing a + // real outstanding failure instead of being cleared by an unrelated success. + self.last_error = self.per_subsystem_error.values().next().cloned(); + if self.last_error.is_none() { + self.last_error_at = None; + } + recovered + } + + /// Records a failure for `subsystem`, returning whether it differs from the one already + /// recorded, so a subsystem failing identically every cycle produces one entry rather + /// than thousands. + fn record_err(&mut self, subsystem: &str, msg: String) -> bool { + let changed = + self.per_subsystem_error.get(subsystem).map(String::as_str) != Some(msg.as_str()); + self.per_subsystem_error + .insert(subsystem.to_string(), msg.clone()); + self.last_error_at = Some(now_rfc3339()); + self.last_error = Some(msg); + changed + } +} + fn set_ok(state: &Arc>, subsystem: &str, msg: &str) -> bool { let Ok(mut s) = state.lock() else { return false; }; - let recovered = s.per_subsystem_error.remove(subsystem).is_some(); - s.last_ok_at = Some(now_rfc3339()); - s.last_ok = Some(msg.to_string()); - // 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(); + let recovered = s.record_ok(subsystem, msg); persist_state(&s); recovered } -/// Records a failure for `subsystem`, returning whether it differs from the one already -/// recorded, so a subsystem failing identically every cycle produces one entry rather than -/// thousands. fn set_err(state: &Arc>, subsystem: &str, msg: String) -> bool { let Ok(mut s) = state.lock() else { return false; }; - 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 = Some(msg); + let changed = s.record_err(subsystem, msg); persist_state(&s); changed } @@ -347,3 +371,59 @@ fn append_event(event: &str) { fn now_rfc3339() -> String { chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true) } + +#[cfg(test)] +mod tests { + use super::*; + + /// One shared error slot meant a successful firewall reconcile erased a traffic failure + /// that was still current, and logged `firewall=recovered` for a subsystem that had + /// never failed. + #[test] + fn a_success_does_not_clear_another_subsystems_failure() { + let mut s = LastActionState::default(); + assert!(s.record_err("traffic", "tc apply failed".into())); + + assert!( + !s.record_ok("firewall", "periodic firewall reconcile ok"), + "firewall never failed, so it cannot have recovered" + ); + assert_eq!( + s.per_subsystem_error.get("traffic").map(String::as_str), + Some("tc apply failed") + ); + assert_eq!( + s.last_error.as_deref(), + Some("tc apply failed"), + "status must keep showing the failure that is still outstanding" + ); + } + + /// The events log records transitions, not cycles: a subsystem failing the same way + /// every 15 seconds would otherwise fill the file within the hour. + #[test] + fn only_a_changed_failure_is_worth_reporting() { + let mut s = LastActionState::default(); + assert!(s.record_err("swap", "no space left".into())); + assert!(!s.record_err("swap", "no space left".into())); + assert!(s.record_err("swap", "permission denied".into())); + } + + /// A Docker restart ends the events stream and records an error. The reconnect has to + /// clear it, once - otherwise the failure stays in `status` forever while the + /// replacement stream runs normally. + #[test] + fn a_reconnected_events_stream_recovers_exactly_once() { + let mut s = LastActionState::default(); + s.record_err("docker_events", "docker events stream ended".into()); + + assert!(s.record_ok("docker_events", "docker events stream connected")); + assert!(!s.record_ok("docker_events", "docker events stream connected")); + assert!(s.per_subsystem_error.is_empty()); + assert!(s.last_error.is_none()); + assert!( + s.last_error_at.is_none(), + "a cleared error must not leave its timestamp behind" + ); + } +} diff --git a/traffic_core/src/swap.rs b/traffic_core/src/swap.rs index 37c9bad..df583e8 100644 --- a/traffic_core/src/swap.rs +++ b/traffic_core/src/swap.rs @@ -195,10 +195,14 @@ pub fn ensure_swap(cfg: &Config) -> anyhow::Result<()> { ); } - // 5. Only now is it safe to take the others down. + // 5. Whatever a killed run left behind is safe to drop now that the managed swap is + // confirmed live. + drop_inactive_transition_files(&desired.path, &active_after); + + // 6. Only now is it safe to take the others down. disable_foreign_swaps(&desired, &active_after)?; - // 6. Persist and verify. + // 7. Persist and verify. rewrite_fstab_exclusive(&desired)?; apply_swappiness(desired.swappiness)?; verify_exclusive_swap_state(&desired)?; @@ -1106,11 +1110,16 @@ fn rollback_replacement_with( let p = &desired.path; let mut notes: Vec = Vec::new(); - let mut staging_active = state.staging_active; + // Both swapfiles are tracked by where they currently are rather than by a flag. A + // rollback can be interrupted at a point where neither is at the path it started from, + // and the kernel activates a swap area by path: a file does not have to sit at the + // managed path to carry load. + let mut replacement_home: Option = None; + let mut replacement_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 + // usable again - which is what the first version did, so a failed reactivation left // nothing to fall back to. if state.new_installed { if state.new_active @@ -1126,37 +1135,54 @@ fn rollback_replacement_with( previous.display() ); } - if let Err(e) = fs::rename(p, staging) { - notes.push(format!( - "could not move the replacement aside to {}: {e}", - staging.display() - )); + match fs::rename(p, staging) { + Ok(()) => replacement_home = Some(staging.to_path_buf()), + Err(e) => { + notes.push(format!( + "could not move the replacement aside to {}: {e}", + staging.display() + )); + replacement_home = Some(p.clone()); + } + } + } else if state.staging_active || staging.exists() { + replacement_home = Some(staging.to_path_buf()); + } + + // Put the original file back where it belongs. Skipped when the replacement could not + // be moved off the managed path: `rename` would unlink it, and that replacement is one + // of the two swapfiles this rollback still has to fall back on. + let mut original_home = if state.original_moved { + previous.to_path_buf() + } else { + p.clone() + }; + if state.original_moved && replacement_home.as_deref() != Some(p.as_path()) { + match fs::rename(previous, p) { + Ok(()) => { + original_home = p.clone(); + if let Err(e) = record_ownership(p) { + notes.push(format!("ownership record not rewritten: {e}")); + } + } + Err(e) => notes.push(format!( + "could not move {} back to {}: {e}", + previous.display(), + p.display() + )), } } - // Put the original file back where it belongs. - if state.original_moved { - 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) { - notes.push(format!("ownership record not rewritten: {e}")); - } - } - - // Reactivate it if we took it down, at the priority it had before. + // Reactivate the original at the priority it had before - from wherever it now is. A + // failed rename back is no reason to leave a healthy swapfile idle: returning here, as + // the first version did, left two intact swapfiles on disk with neither of them running. let mut original_back = state.original_active; - if !state.original_active { - match ops.swapon(p, state.previous_priority) { + if !original_back { + match ops.swapon(&original_home, state.previous_priority) { Ok(()) => original_back = true, Err(e) => notes.push(format!( "could not reactivate {} at priority {}: {e}", - p.display(), + original_home.display(), state.previous_priority )), } @@ -1167,51 +1193,79 @@ fn rollback_replacement_with( // 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, + if !replacement_active + && let Some(home) = replacement_home.as_deref() + && home.exists() + { + match ops.swapon(home, state.previous_priority) { + Ok(()) => replacement_active = true, Err(e) => notes.push(format!( "could not activate {} as a fallback: {e}", - staging.display() + home.display() )), } } let detail = format_notes(¬es); - return if staging_active { - anyhow::anyhow!( + return match replacement_home.filter(|_| replacement_active) { + Some(home) => anyhow::anyhow!( "{cause}; the previous swap could not be restored{detail}; the replacement \ at {} is left active so the host is not without swap. Once the host allows \ it, run `swapon -p {} {}` and then `swapoff {}`", - staging.display(), + home.display(), state.previous_priority, - p.display(), - staging.display() - ) - } else { - anyhow::anyhow!( + original_home.display(), + home.display() + ), + None => 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 replacement 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 staging_active { - match ops.swapoff(staging) { - Ok(()) => { - if let Err(e) = remove_if_present(staging) { - notes.push(format!("inactive staging file left behind: {e}")); + // The original is serving again, but from `.previous`, because the rename back failed. + // The host has swap, so nothing here is urgent - but the managed path does not hold what + // the ownership record describes, and releasing the replacement would take away the only + // other usable swapfile, so both are left exactly as they are for an operator. + if original_home != *p { + // Naming where the replacement ended up matters: when it could not be moved aside it + // is still sitting on the managed path, and an operator told to "put the previous + // file back" would otherwise overwrite it. + let replacement_note = match replacement_home.as_deref() { + Some(home) => format!(" The replacement is at {}.", home.display()), + None => String::new(), + }; + return anyhow::anyhow!( + "{cause}; the previous swapfile could not be moved back to {} and is serving \ + from {} instead{}.{replacement_note} Both swapfiles are intact: once the host \ + allows it, run `swapoff {}`, put it back at {} and re-run ensure-swap", + p.display(), + original_home.display(), + format_notes(¬es), + original_home.display(), + p.display() + ); + } + + // Original confirmed back at the managed path: now the replacement 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 let Some(home) = replacement_home { + if replacement_active { + match ops.swapoff(&home) { + Ok(()) => { + if let Err(e) = remove_if_present(&home) { + notes.push(format!("inactive staging file left behind: {e}")); + } } + Err(e) => notes.push(format!( + "previous swap is restored, but the staging swap {} is still active: {e}", + home.display() + )), } - Err(e) => notes.push(format!( - "previous swap is restored, but the staging swap {} is still active: {e}", - staging.display() - )), + } else if let Err(e) = remove_if_present(&home) { + notes.push(format!("inactive staging file left behind: {e}")); } - } else if let Err(e) = remove_if_present(staging) { - notes.push(format!("inactive staging file left behind: {e}")); } anyhow::anyhow!( @@ -1228,6 +1282,40 @@ fn format_notes(notes: &[String]) -> String { } } +/// Drops `.new`/`.previous` left by a killed run, once the managed swap is confirmed live. +/// +/// [`clear_stale_transition_files`] only runs on the way *into* a resize, so a process killed +/// after the replacement was activated left the old file on disk indefinitely: the next run +/// finds a valid, active, correctly owned swapfile at the managed path, takes the fast path +/// and never looks at the leftovers. A swapfile is commonly sized after RAM, so that is not a +/// rounding error on disk. +/// +/// Only inactive leftovers are removed. An active one means the kernel is still swapping to +/// it - the interrupted-rollback case, which needs an operator rather than a guess and is +/// reported as such by the next resize. +fn drop_inactive_transition_files(path: &Path, active: &[ActiveSwap]) { + for suffix in [STAGING_SUFFIX, PREVIOUS_SUFFIX] { + let Ok(leftover) = suffixed_path(path, suffix) else { + continue; + }; + if !leftover.exists() || active.iter().any(|s| s.name == leftover) { + continue; + } + match remove_if_present(&leftover) { + Ok(()) => tracing::info!( + target: "swap", + "removed {}, left behind by an interrupted swap replacement", + leftover.display() + ), + Err(e) => tracing::warn!( + target: "swap", + "leftover {} from an interrupted swap replacement could not be removed: {e}", + leftover.display() + ), + } + } +} + /// Clears `.new`/`.previous` left by a killed run, refusing to guess when they are active. /// /// An active leftover cannot be unlinked, and blindly retrying the removal every reconcile @@ -1815,6 +1903,8 @@ mod tests { struct FakeSwap { /// Paths whose `swapon` fails; everything else succeeds. swapon_fails_for: Vec, + /// Paths whose `swapoff` fails; everything else succeeds. + swapoff_fails_for: Vec, calls: std::sync::Mutex>, } @@ -1822,6 +1912,14 @@ mod tests { fn failing_for(paths: &[&Path]) -> Self { Self { swapon_fails_for: paths.iter().map(|p| p.to_path_buf()).collect(), + swapoff_fails_for: Vec::new(), + calls: std::sync::Mutex::new(Vec::new()), + } + } + fn swapoff_failing_for(paths: &[&Path]) -> Self { + Self { + swapon_fails_for: Vec::new(), + swapoff_fails_for: paths.iter().map(|p| p.to_path_buf()).collect(), calls: std::sync::Mutex::new(Vec::new()), } } @@ -1849,6 +1947,9 @@ mod tests { .lock() .expect("lock") .push(format!("swapoff {}", path.display())); + if self.swapoff_fails_for.iter().any(|p| p == path) { + anyhow::bail!("swapoff: device or resource busy"); + } Ok(()) } } @@ -2018,6 +2119,128 @@ mod tests { ); } + /// A failed rename back is not a dead end. The old swapfile is intact under its + /// `.previous` name and the kernel activates a swap area by path, so it is brought up + /// where it lies instead of being left idle next to a host with no swap. + #[test] + fn rollback_activates_the_previous_swapfile_in_place_when_it_cannot_be_moved_back() { + let (desired, staging, previous) = rollback_fixture("rb_rename_fail"); + fs::write(&previous, b"original").expect("seed previous"); + // A directory at the managed path makes the rename back fail the way a real EACCES + // or EIO would, without needing to inject a filesystem seam. + fs::create_dir(&desired.path).expect("block the managed path"); + let ops = FakeSwap::all_ok(); + let state = ReplacementState { + staging_active: false, + original_active: false, + original_moved: true, + 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, vec![format!("swapon {} -p 1", previous.display())]); + let msg = err.to_string(); + assert!(msg.contains("is serving from"), "{msg}"); + assert!(!msg.contains("no managed swap"), "{msg}"); + assert!(previous.exists(), "the restored swapfile must survive"); + } + + /// Same failure, with the previous swapfile unusable as well: the replacement is the + /// only candidate left, so it is activated rather than the rollback reporting no swap + /// while two intact swapfiles sit on disk. + #[test] + fn rollback_falls_back_to_the_replacement_when_neither_rename_nor_previous_works() { + let (desired, staging, previous) = rollback_fixture("rb_both_fail"); + fs::write(&previous, b"original").expect("seed previous"); + fs::write(&staging, b"replacement").expect("seed staging"); + fs::create_dir(&desired.path).expect("block the managed path"); + let ops = FakeSwap::failing_for(&[&previous]); + let state = ReplacementState { + staging_active: false, + original_active: false, + original_moved: true, + 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, + vec![ + format!("swapon {} -p 1", previous.display()), + format!("swapon {} -p 1", staging.display()), + ] + ); + assert!( + err.to_string() + .contains("is left active so the host is not without swap") + ); + assert!(staging.exists() && previous.exists()); + } + + /// An installed replacement the kernel refuses to release is the host's only swap. It + /// stays where it is: moving a file the kernel is swapping to only makes /proc/swaps + /// disagree with the filesystem. + #[test] + fn rollback_leaves_an_undetachable_replacement_in_place() { + let (desired, staging, previous) = rollback_fixture("rb_busy"); + fs::write(&desired.path, b"replacement").expect("seed new"); + fs::write(&previous, b"original").expect("seed previous"); + let ops = FakeSwap::swapoff_failing_for(&[&desired.path]); + let state = ReplacementState { + staging_active: false, + original_active: false, + original_moved: true, + new_installed: true, + new_active: true, + previous_priority: 1, + }; + + let err = rollback_replacement_with( + &ops, + &desired, + &staging, + &previous, + &state, + anyhow::anyhow!("verification failed"), + ); + + assert_eq!( + ops.calls(), + vec![format!("swapoff {}", desired.path.display())], + "nothing else is attempted once the live replacement cannot be released" + ); + assert!(!staging.exists(), "the replacement is not moved aside"); + assert_eq!( + fs::read(&desired.path).expect("replacement in place"), + b"replacement".to_vec() + ); + assert_eq!( + fs::read(&previous).expect("previous untouched"), + b"original".to_vec() + ); + assert!(err.to_string().contains("is left in place and serving")); + } + /// 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() { @@ -2035,33 +2258,49 @@ mod tests { ); 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] - fn active_leftover_transition_file_is_reported_not_deleted() { - let dir = scratch("stale_active"); - let staging = dir.join("swapfile.awg2-harness.new"); - fs::write(&staging, b"leftover").expect("seed"); - // Simulates read_active_swaps reporting the leftover as active. - let active = [active(&staging.to_string_lossy(), 1024, 1)]; - let blocked = active.iter().any(|s| s.name == staging); - assert!(blocked, "leftover must be recognised as active"); - assert!(staging.exists(), "an active leftover is never unlinked"); + /// A run killed after the replacement went live leaves the old file on disk, and the + /// next reconcile takes the fast path straight past it. Since a swapfile is usually + /// sized after RAM, "eventually" is not good enough - it is dropped as soon as the + /// managed swap is confirmed active. + #[test] + fn crash_leftovers_are_dropped_once_the_managed_swap_is_live() { + let dir = scratch("leftovers_dropped"); + let main = dir.join("swapfile"); + let staging = dir.join("swapfile.awg2-harness.new"); + let previous = dir.join("swapfile.awg2-harness.previous"); + fs::write(&main, b"managed").expect("seed main"); + fs::write(&staging, b"leftover").expect("seed staging"); + fs::write(&previous, b"leftover").expect("seed previous"); + + drop_inactive_transition_files(&main, &[active(&main.to_string_lossy(), 1024, 1)]); + + assert!(!staging.exists(), "inactive leftover must be removed"); + assert!(!previous.exists(), "inactive leftover must be removed"); + assert!(main.exists(), "the managed swapfile is never touched"); } + /// A leftover that is still active cannot be unlinked, and the kernel is swapping to it. + /// It is left for the interrupted-rollback path to report on the next resize. #[test] - fn inactive_leftover_transition_files_are_cleared() { - let dir = scratch("stale_inactive"); + fn active_leftover_transition_file_is_never_unlinked() { + let dir = scratch("stale_active"); + let main = dir.join("swapfile"); let staging = dir.join("swapfile.awg2-harness.new"); let previous = dir.join("swapfile.awg2-harness.previous"); fs::write(&staging, b"leftover").expect("seed staging"); fs::write(&previous, b"leftover").expect("seed previous"); - remove_if_present(&staging).expect("clear staging"); - remove_if_present(&previous).expect("clear previous"); - assert!(!staging.exists()); - assert!(!previous.exists()); + drop_inactive_transition_files( + &main, + &[ + active(&main.to_string_lossy(), 1024, 1), + active(&previous.to_string_lossy(), 1024, 1), + ], + ); + + assert!(!staging.exists(), "the inactive one still goes"); + assert!(previous.exists(), "an active leftover is never unlinked"); } /// The legacy sidecar is removed only when it carries the marker the old version