Skip to content

Commit

Permalink
repair: Rework to be more introspectable
Browse files Browse the repository at this point in the history
I plan to "productize" this repair code a bit more in OpenShift at
least, and I think other admins may want to do similar outside
of that too.

In order to make that more reliable:

- Better split the "fsck/--dry-run" path from "repair" i.e mutation
- Introduce a `--write-result-to` argument that outputs
  JSON.  This allows us to better distinguish the tristate of
  "OK" from "corruption detected" to "tool failed for some other reason"

Further:

- Drop the check for derived commits, let's just *always* check
  the inodes because it's not very expensive in the end and
  it's just really useful to do.
- Add checks for whether the booted deployment is potentially
  affected, as this is important information; we'll need to redeploy
  and reboot if so
  • Loading branch information
cgwalters committed Jul 21, 2023
1 parent 07480ff commit 5dbd332
Show file tree
Hide file tree
Showing 3 changed files with 195 additions and 120 deletions.
60 changes: 50 additions & 10 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use fn_error_context::context;
use ostree::{cap_std, gio, glib};
use std::collections::BTreeMap;
use std::ffi::OsString;
use std::io::BufWriter;
use std::path::PathBuf;
use std::process::Command;
use tokio::sync::mpsc::Receiver;
Expand Down Expand Up @@ -348,7 +349,7 @@ pub(crate) enum ContainerImageOpts {
}

/// Options for deployment repair.
#[derive(Debug, Subcommand)]
#[derive(Debug, Parser)]
pub(crate) enum ProvisionalRepairOpts {
AnalyzeInodes {
/// Path to the repository
Expand All @@ -358,6 +359,10 @@ pub(crate) enum ProvisionalRepairOpts {
/// Print additional information
#[clap(long)]
verbose: bool,

/// Serialize the repair result to this file as JSON
#[clap(long)]
write_result_to: Option<Utf8PathBuf>,
},

Repair {
Expand All @@ -369,6 +374,10 @@ pub(crate) enum ProvisionalRepairOpts {
#[clap(long)]
dry_run: bool,

/// Serialize the repair result to this file as JSON
#[clap(long)]
write_result_to: Option<Utf8PathBuf>,

/// Print additional information
#[clap(long)]
verbose: bool,
Expand Down Expand Up @@ -787,6 +796,17 @@ fn container_remount_sysroot(sysroot: &Utf8Path) -> Result<()> {
Ok(())
}

#[context("Serializing to output file")]
fn handle_serialize_to_file<T: serde::Serialize>(path: Option<&Utf8Path>, obj: T) -> Result<()> {
if let Some(path) = path {
let mut out = std::fs::File::create(path)
.map(BufWriter::new)
.with_context(|| anyhow::anyhow!("Opening {path} for writing"))?;
serde_json::to_writer(&mut out, &obj).context("Serializing output")?;
}
Ok(())
}

/// Parse the provided arguments and execute.
/// Calls [`structopt::clap::Error::exit`] on failure, printing the error message and aborting the program.
pub async fn run_from_iter<I>(args: I) -> Result<()>
Expand Down Expand Up @@ -1027,28 +1047,48 @@ where
#[cfg(feature = "docgen")]
Opt::Man(manopts) => crate::docgen::generate_manpages(&manopts.directory),
Opt::ProvisionalRepair(opts) => match opts {
ProvisionalRepairOpts::AnalyzeInodes { repo, verbose } => {
ProvisionalRepairOpts::AnalyzeInodes {
repo,
verbose,
write_result_to,
} => {
let repo = parse_repo(&repo)?;
match crate::repair::check_inode_collision(&repo, verbose)? {
crate::repair::InodeCheckResult::Okay => {
println!("OK: No colliding objects found.");
}
crate::repair::InodeCheckResult::PotentialCorruption(n) => {
eprintln!("warning: {} potentially colliding inodes found", n.len());
}
let check_res = crate::repair::check_inode_collision(&repo, verbose)?;
handle_serialize_to_file(write_result_to.as_deref(), &check_res)?;
if check_res.collisions.is_empty() {
println!("OK: No colliding objects found.");
} else {
eprintln!(
"warning: {} potentially colliding inodes found",
check_res.collisions.len()
);
}
Ok(())
}
ProvisionalRepairOpts::Repair {
sysroot,
verbose,
dry_run,
write_result_to,
} => {
container_remount_sysroot(&sysroot)?;
let sysroot = &ostree::Sysroot::new(Some(&gio::File::for_path(&sysroot)));
sysroot.load(gio::Cancellable::NONE)?;
let sysroot = &SysrootLock::new_from_sysroot(sysroot).await?;
crate::repair::auto_repair_inode_collision(sysroot, dry_run, verbose)
let result = crate::repair::analyze_for_repair(sysroot, verbose)?;
if let Some(write_result_to) = write_result_to.as_deref() {
let mut out = std::fs::File::create(write_result_to)
.map(BufWriter::new)
.with_context(|| {
anyhow::anyhow!("Opening {write_result_to} for writing")
})?;
serde_json::to_writer(&mut out, &result).context("Serializing output")?;
}
if dry_run {
result.check()
} else {
result.repair(sysroot)
}
}
},
}
Expand Down
8 changes: 4 additions & 4 deletions lib/src/container/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,13 +1385,12 @@ fn compare_commit_trees(
pub(crate) fn verify_container_image(
sysroot: &SysrootLock,
imgref: &ImageReference,
state: &LayeredImageState,
colliding_inodes: &BTreeSet<u64>,
verbose: bool,
) -> Result<bool> {
let cancellable = gio::Cancellable::NONE;
let repo = &sysroot.repo();
let state =
query_image_ref(repo, imgref)?.ok_or_else(|| anyhow!("Expected present image {imgref}"))?;
let merge_commit = state.merge_commit.as_str();
let merge_commit_root = repo.read_commit(merge_commit, gio::Cancellable::NONE)?.0;
let merge_commit_root = merge_commit_root
Expand All @@ -1402,9 +1401,10 @@ pub(crate) fn verify_container_image(
// This shouldn't happen anymore
let config = state
.configuration
.ok_or_else(|| anyhow!("Missing configuration for image {imgref}"))?;
.as_ref()
.ok_or_else(|| anyhow!("Missing configuration for image"))?;
let (commit_layer, _component_layers, remaining_layers) =
parse_manifest_layout(&state.manifest, &config)?;
parse_manifest_layout(&state.manifest, config)?;

let mut comparison_state = CompareState::default();

Expand Down
Loading

0 comments on commit 5dbd332

Please sign in to comment.