fix: make the reboot plan trustworthy and report unreadable sysctl files [P2]
Three diagnostic defects from the follow-up audit. 1. The reboot plan was entirely best-effort. persist_plan discarded the directory creation, the write and every error. The plan file is the only thing on the host that says when it is going to reboot, so a failed write produced a scheduler sitting on a timer that nothing could report. It now creates the directory, writes through a temp file with sync_all and rename, and returns Result. The plan also outlived its meaning in two places. With reboot.enabled = false the scheduler exited without touching a plan written when it was still enabled, so status kept advertising a reboot that would never happen. And after systemd accepted the reboot request the plan stayed behind, leaving a past timestamp looking pending. Both paths now clear it; an absent plan is the desired end state, not an error. 2. sysctl status turned a read failure into "file absent". inspect_sysctl_tuning used fs::read_to_string(..).ok() for both managed files, so an unreadable file was reported as missing - a diagnostic stating the opposite of the truth, in the module whose whole job is to report drift accurately. Reads now go through ManagedFile::read, which distinguishes absent from unreadable, and an unreadable file produces its own drift reason naming the path and the error. 3. README described the removed ExecStartPre. It still said the service runs ensure-swap before starting the daemon. The daemon performs it as the first step of its own startup sequence. Docs also state explicitly what survives swap.enabled = false, which was implied but never written: the swapfile, its activation and the /etc/fstab entry all remain, so swap comes back after a reboot. Only the vm.swappiness persistence is removed. Acceptance checklist gained seven checks covering the priority-only path not recreating the file, swap remaining active throughout a resize, foreign-swap rollback, the unreadable status, and both reboot-plan lifecycle cases. cargo test --workspace: 179 + 4 passed. clippy -D warnings clean.
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
use std::{collections::BTreeMap, fs, path::Path, process::Command, time::Duration};
|
||||
use std::{collections::BTreeMap, path::Path, process::Command, time::Duration};
|
||||
|
||||
use crate::{config::Config, lock::FileLock, managed_file::ManagedFile};
|
||||
|
||||
@@ -92,6 +92,29 @@ pub fn ensure_sysctl_tuning(cfg: &Config) -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Status for a host where a managed file exists but cannot be read.
|
||||
///
|
||||
/// Distinct from "missing": the operator needs to know the difference between a file this
|
||||
/// package has not written yet and one it cannot inspect.
|
||||
fn unreadable_status(cfg: &Config, plan: &RenderPlan, detail: String) -> SysctlTuningStatus {
|
||||
SysctlTuningStatus {
|
||||
enabled: cfg.sysctl_tuning.enabled,
|
||||
apply_on_daemon_start: cfg.sysctl_tuning.apply_on_daemon_start,
|
||||
tcp_bbr_mode: cfg.sysctl_tuning.tcp_bbr.clone(),
|
||||
conf_path: SYSCTL_CONF_PATH.to_string(),
|
||||
modules_load_path: BBR_MODULES_LOAD_PATH.to_string(),
|
||||
persisted_present: false,
|
||||
persisted_matches: false,
|
||||
modules_load_present: false,
|
||||
modules_load_matches: false,
|
||||
bbr_available: plan.bbr_available,
|
||||
bbr_active: plan.bbr_active,
|
||||
current: Vec::new(),
|
||||
drift_reason: Some(format!("managed sysctl file is unreadable: {detail}")),
|
||||
warnings: vec![format!("managed sysctl file is unreadable: {detail}")],
|
||||
}
|
||||
}
|
||||
|
||||
pub fn inspect_sysctl_tuning(cfg: &Config) -> anyhow::Result<SysctlTuningStatus> {
|
||||
let plan = match build_render_plan(cfg, false) {
|
||||
Ok(plan) => plan,
|
||||
@@ -114,13 +137,33 @@ pub fn inspect_sysctl_tuning(cfg: &Config) -> anyhow::Result<SysctlTuningStatus>
|
||||
});
|
||||
}
|
||||
};
|
||||
let persisted_raw = fs::read_to_string(SYSCTL_CONF_PATH).ok();
|
||||
// `.ok()` here used to turn a read failure into "the file is absent", so an
|
||||
// unreadable managed file was reported as missing - a diagnostic saying the opposite
|
||||
// of what is true. A read error now surfaces as its own drift reason.
|
||||
let persisted_read = managed(Path::new(SYSCTL_CONF_PATH)).read();
|
||||
let modules_load_read = managed(Path::new(BBR_MODULES_LOAD_PATH)).read();
|
||||
if let Err(e) = &persisted_read {
|
||||
return Ok(unreadable_status(
|
||||
cfg,
|
||||
&plan,
|
||||
format!("{SYSCTL_CONF_PATH}: {e}"),
|
||||
));
|
||||
}
|
||||
if let Err(e) = &modules_load_read {
|
||||
return Ok(unreadable_status(
|
||||
cfg,
|
||||
&plan,
|
||||
format!("{BBR_MODULES_LOAD_PATH}: {e}"),
|
||||
));
|
||||
}
|
||||
let persisted_raw = persisted_read.unwrap_or_default();
|
||||
let modules_load_raw = modules_load_read.unwrap_or_default();
|
||||
|
||||
let persisted_present = persisted_raw.is_some();
|
||||
let persisted_matches = persisted_raw
|
||||
.as_deref()
|
||||
.map(|v| normalize(v) == normalize(&plan.persisted))
|
||||
.unwrap_or(false);
|
||||
let modules_load_raw = fs::read_to_string(BBR_MODULES_LOAD_PATH).ok();
|
||||
let modules_load_present = modules_load_raw.is_some();
|
||||
let modules_load_expected = plan.modules_load_persisted.as_deref();
|
||||
let modules_load_matches = match (modules_load_raw.as_deref(), modules_load_expected) {
|
||||
@@ -450,6 +493,7 @@ mod tests {
|
||||
AmneziaConfig, AntiTorrentConfig, HostFirewallConfig, RebootConfig, SwapConfig,
|
||||
TrafficConfig,
|
||||
};
|
||||
use std::fs;
|
||||
|
||||
fn cfg() -> Config {
|
||||
Config {
|
||||
|
||||
Reference in New Issue
Block a user