From f637f6b0b11c4d929ea0760d5060bd34fccc88b1 Mon Sep 17 00:00:00 2001 From: Vinnie Magro Date: Wed, 3 Jul 2024 07:29:50 -0700 Subject: [PATCH] [antlir2][errors] integrate prebuilt and compile actions with new error handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: One of the most common antlir build errors is Eden failing to setup redirections, so let's integrate `antlir2_receive` and `antlir2 compile` with the new action error handler lib from the previous diff. Test Plan: ```name="break antlir2-out" ❯ sudo mount -t tmpfs tmpfs antlir2-out ``` ```name="Try to download an image" ❯ buck2 build --show-output fbcode//metalos/services/base:base.prebuilt.stripped Action failed: fbcode//metalos/services/base:base.prebuilt.stripped--prebuilt (antlir2_prebuilt_layer) Local command returned non-zero exit code 1 Reproduce locally: `env -- 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/fbcode/7fb6662f2bd64550/antlir2_prebuilt_layer' 'RUST_LOG= ...... n/fbcode/6a600a0fd69ee02b/metalos/services/base/__base.prebuilt.stripped--prebuilt__/subvol_symlink' (run `buck2 log what-failed` to get the full command)` stdout: T0701 13:46:00.927001 633289 fbcode/antlir/antlir2/antlir2_receive/src/main.rs:103] [receive] setting up WorkingVolume T0701 13:46:00.927459 633289 fbcode/antlir/antlir2/antlir2_rootless/src/lib.rs:81] [receive] setegid(100) T0701 13:46:00.927525 633289 fbcode/antlir/antlir2/antlir2_rootless/src/lib.rs:88] [receive] seteuid(115203) T0701 13:46:00.927752 633289 fbcode/antlir/antlir2/antlir2_receive/src/main.rs:96] [receive, prepare_dst{working_volume: WorkingVolume { path: "antlir2-out" }}] WorkingVolume gave us new path antlir2-out/2bcaf9b0e3674363b422bdaa2959e2fc I0701 13:46:00.927844 633289 fbcode/antlir/antlir2/antlir2_receive/src/main.rs:93] [receive, prepare_dst{working_volume: WorkingVolume { path: "antlir2-out" }}] return: "antlir2-out/2bcaf9b0e3674363b422bdaa2959e2fc" T0701 13:46:00.927959 633289 fbcode/antlir/antlir2/antlir2_rootless/src/lib.rs:110] [receive] escalating privileges to root T0701 13:46:00.928137 633289 fbcode/antlir/antlir2/antlir2_rootless/src/lib.rs:158] [receive] dropping privileges T0701 13:46:00.928191 633289 fbcode/antlir/antlir2/antlir2_rootless/src/lib.rs:160] [receive] setegid(100) T0701 13:46:00.928242 633289 fbcode/antlir/antlir2/antlir2_rootless/src/lib.rs:169] [receive] seteuid(115203) stderr: antlir2_error_handler: {"category":"btrfs","message":"not a btrfs filesystem","locations":[]} Error: Btrfs(NotBtrfs) Buck UI: https://www.internalfb.com/buck2/418376a8-3c44-40bb-85b8-83a409cee4b5 Note: Using experimental modern dice Network: Up: 0B Down: 0B (reSessionID-10d9e049-e084-410f-ac80-46aec10461a7) Command: build. Jobs completed: 4. Time elapsed: 0.2s. Cache hits: 0%. Commands: 1 (cached: 0, remote: 0, local: 1) BUILD FAILED Failed to build 'fbcode//metalos/services/base:base.prebuilt.stripped--prebuilt (cfg:linux-x86_64-fbcode-platform010-compat-clang15-asan-ubsan-dev#6a600a0fd69ee02b)' ``` ```name="Category gets reported to scuba" ❯ scuba -e $'SELECT `category`, `identifier`, `sub_error_categories`, `target_name`, `target_package` FROM `buck2_action_errors` WHERE 1719262033 <= `time` AND `time` <= 1719866832 AND ((`uuid`) IN (\'7ac68dd9-bd85-473e-ac19-1f588d6c785e\')) LIMIT 100;' +------------------------+------------+----------------------+----------------------------------+-------------------------------+ | category | identifier | sub_error_categories | target_name | target_package | +------------------------+------------+----------------------+----------------------------------+-------------------------------+ | antlir2_prebuilt_layer | | ["btrfs"] | base.prebuilt.stripped--prebuilt | fbcode//metalos/services/base | +------------------------+------------+----------------------+----------------------------------+-------------------------------+ 1 row(s) in set (0.56 sec) ``` Reviewed By: sergeyfd Differential Revision: D59243781 fbshipit-source-id: 1dc4906f604055443c633edebe1d71519e6cab98 --- antlir/antlir2/antlir2/BUCK | 1 + antlir/antlir2/antlir2/src/cmd/compile.rs | 5 +- antlir/antlir2/antlir2/src/main.rs | 25 ++++++++- antlir/antlir2/antlir2_btrfs/src/lib.rs | 11 +++- antlir/antlir2/antlir2_receive/BUCK | 2 + antlir/antlir2/antlir2_receive/src/main.rs | 64 +++++++++++++++++----- antlir/antlir2/bzl/image/layer.bzl | 2 + antlir/antlir2/bzl/image/prebuilt.bzl | 3 + 8 files changed, 91 insertions(+), 22 deletions(-) diff --git a/antlir/antlir2/antlir2/BUCK b/antlir/antlir2/antlir2/BUCK index 0095e37e60..fe8f5aad38 100644 --- a/antlir/antlir2/antlir2/BUCK +++ b/antlir/antlir2/antlir2/BUCK @@ -23,6 +23,7 @@ rust_binary( "//antlir/antlir2/antlir2_compile:antlir2_compile", "//antlir/antlir2/antlir2_depgraph:antlir2_depgraph", "//antlir/antlir2/antlir2_depgraph_if:antlir2_depgraph_if", + "//antlir/antlir2/antlir2_error_handler:antlir2_error_handler", "//antlir/antlir2/antlir2_facts:antlir2_facts", "//antlir/antlir2/antlir2_features:antlir2_features", "//antlir/antlir2/antlir2_isolate:antlir2_isolate", diff --git a/antlir/antlir2/antlir2/src/cmd/compile.rs b/antlir/antlir2/antlir2/src/cmd/compile.rs index 3f0a68ffd0..01c9d0b926 100644 --- a/antlir/antlir2/antlir2/src/cmd/compile.rs +++ b/antlir/antlir2/antlir2/src/cmd/compile.rs @@ -93,10 +93,7 @@ impl Compile { pub(crate) fn run(self, rootless: Rootless) -> Result<()> { // this must happen before unshare let working_volume = match self.working_format { - WorkingFormat::Btrfs => Some( - WorkingVolume::ensure(self.working_dir.clone()) - .context("while setting up WorkingVolume")?, - ), + WorkingFormat::Btrfs => Some(WorkingVolume::ensure(self.working_dir.clone())?), WorkingFormat::Overlayfs => None, }; diff --git a/antlir/antlir2/antlir2/src/main.rs b/antlir/antlir2/antlir2/src/main.rs index d41887c63f..2dc06fbece 100644 --- a/antlir/antlir2/antlir2/src/main.rs +++ b/antlir/antlir2/antlir2/src/main.rs @@ -9,7 +9,6 @@ use std::fs::File; use std::path::PathBuf; -use std::process::ExitStatus; use anyhow::Context; use clap::Parser; @@ -22,12 +21,12 @@ mod cmd; #[derive(Debug, Error)] pub enum Error { + #[error("failed to setup working volume: {0}")] + WorkingVolume(#[from] antlir2_working_volume::Error), #[error(transparent)] Compile(#[from] antlir2_compile::Error), #[error(transparent)] Depgraph(#[from] antlir2_depgraph::Error), - #[error("subprocess exited with {0}")] - Subprocess(ExitStatus), #[error(transparent)] Btrfs(#[from] antlir2_btrfs::Error), #[error(transparent)] @@ -90,6 +89,19 @@ enum Subcommand { Depgraph(cmd::Depgraph), } +impl Error { + fn category(&self) -> Option<&'static str> { + match self { + Error::WorkingVolume(_) => Some("working_volume"), + Error::Compile(_) => Some("compile_feature"), + Error::Depgraph(_) => Some("depgraph"), + Error::Btrfs(_) => Some("btrfs"), + Error::Rootless(_) => Some("rootless"), + _ => None, + } + } +} + fn main() -> Result<()> { let args = Args::parse(); @@ -113,6 +125,13 @@ fn main() -> Result<()> { error!("{e:#?}"); eprintln!("{}", format!("{e:#?}").red()); eprintln!("{}", e.to_string().red()); + if let Some(category) = e.category() { + antlir2_error_handler::SubError::builder() + .category(category) + .message(e) + .build() + .log(); + } std::process::exit(1); } Ok(()) diff --git a/antlir/antlir2/antlir2_btrfs/src/lib.rs b/antlir/antlir2/antlir2_btrfs/src/lib.rs index adbfa36a60..f2ed749238 100644 --- a/antlir/antlir2/antlir2_btrfs/src/lib.rs +++ b/antlir/antlir2/antlir2_btrfs/src/lib.rs @@ -10,7 +10,9 @@ use std::ffi::OsStr; use std::fmt::Debug; +use std::fs::OpenOptions; use std::os::fd::AsRawFd; +use std::os::fd::RawFd; use std::os::unix::ffi::OsStrExt; use std::path::Path; use std::path::PathBuf; @@ -77,8 +79,8 @@ fn name_bytes(name: &OsStr) -> [u8; L] { buf } -fn ensure_is_btrfs(dir: &Dir) -> Result<()> { - let statfs = fstatfs(dir).map_err(std::io::Error::from)?; +fn ensure_is_btrfs(fd: &impl AsRawFd) -> Result<()> { + let statfs = fstatfs(fd).map_err(std::io::Error::from)?; if statfs.filesystem_type() != BTRFS_SUPER_MAGIC { return Err(Error::NotBtrfs); } @@ -299,6 +301,11 @@ impl Debug for Info { } } +pub fn ensure_path_is_on_btrfs(path: impl AsRef) -> Result<()> { + let fd = OpenOptions::new().read(true).open(path)?; + ensure_is_btrfs(&fd) +} + #[cfg(test)] #[allow(non_upper_case_globals)] mod tests { diff --git a/antlir/antlir2/antlir2_receive/BUCK b/antlir/antlir2/antlir2_receive/BUCK index c5cdc5fd5c..affe54d7c2 100644 --- a/antlir/antlir2/antlir2_receive/BUCK +++ b/antlir/antlir2/antlir2_receive/BUCK @@ -13,11 +13,13 @@ rust_binary( # @oss-disable "tar", "tempfile", + "thiserror", "tracing", "tracing-glog", "tracing-subscriber", "//antlir/antlir2/antlir2_btrfs:antlir2_btrfs", "//antlir/antlir2/antlir2_cas_dir:antlir2_cas_dir", + "//antlir/antlir2/antlir2_error_handler:antlir2_error_handler", "//antlir/antlir2/antlir2_isolate:antlir2_isolate", "//antlir/antlir2/antlir2_rootless:antlir2_rootless", "//antlir/antlir2/antlir2_working_volume:antlir2_working_volume", diff --git a/antlir/antlir2/antlir2_receive/src/main.rs b/antlir/antlir2/antlir2_receive/src/main.rs index 2b930ec232..4cabcd09eb 100644 --- a/antlir/antlir2/antlir2_receive/src/main.rs +++ b/antlir/antlir2/antlir2_receive/src/main.rs @@ -14,9 +14,7 @@ use antlir2_btrfs::Subvolume; use antlir2_cas_dir::CasDir; use antlir2_working_volume::WorkingVolume; use anyhow::anyhow; -use anyhow::ensure; use anyhow::Context; -use anyhow::Result; use clap::Parser; use clap::ValueEnum; use tracing::trace; @@ -65,14 +63,39 @@ struct SetupArgs { working_dir: PathBuf, } +#[derive(Debug, thiserror::Error)] +enum Error { + #[error("failed to setup working volume: {0}")] + WorkingVolume(#[from] antlir2_working_volume::Error), + #[error(transparent)] + Btrfs(#[from] antlir2_btrfs::Error), + #[error(transparent)] + Rootless(#[from] antlir2_rootless::Error), + #[error("{0:#?}")] + IO(#[from] std::io::Error), + #[error("{0:#?}")] + Uncategorized(#[from] anyhow::Error), +} + +type Result = std::result::Result; + +impl Error { + fn category(&self) -> Option<&'static str> { + match self { + Error::WorkingVolume(_) => Some("working_volume"), + Error::Btrfs(_) => Some("btrfs"), + Error::Rootless(_) => Some("rootless"), + _ => None, + } + } +} + impl Receive { /// Make sure that the working directory exists and clean up any existing /// version of the subvolume that we're receiving. #[tracing::instrument(skip(self), ret, err(Debug))] fn prepare_dst(&self, working_volume: &WorkingVolume) -> Result { - let dst = working_volume - .allocate_new_path() - .context("while allocating new path for subvol")?; + let dst = working_volume.allocate_new_path()?; trace!("WorkingVolume gave us new path {}", dst.display()); Ok(dst) @@ -81,8 +104,7 @@ impl Receive { #[tracing::instrument(name = "receive", skip(self))] pub(crate) fn run(self) -> Result<()> { trace!("setting up WorkingVolume"); - let working_volume = WorkingVolume::ensure(self.setup.working_dir.clone()) - .context("while setting up WorkingVolume")?; + let working_volume = WorkingVolume::ensure(self.setup.working_dir.clone())?; let rootless = if self.rootless { antlir2_rootless::unshare_new_userns().context("while setting up userns")?; @@ -99,6 +121,10 @@ impl Receive { match self.format { Format::Sendstream => { + // make sure that working_dir is btrfs before we try to invoke + // 'btrfs' so that we can fail with a nicely categorized error + antlir2_btrfs::ensure_path_is_on_btrfs(&self.setup.working_dir)?; + let recv_tmp = tempfile::tempdir_in(&self.setup.working_dir)?; let mut cmd = Command::new(self.btrfs); cmd.arg("--quiet") @@ -111,18 +137,20 @@ impl Receive { } trace!("receiving sendstream: {cmd:?}"); let res = cmd.spawn()?.wait()?; - ensure!(res.success(), "btrfs-receive failed"); + if !res.success() { + return Err(anyhow!("btrfs-receive failed").into()); + } let entries: Vec<_> = std::fs::read_dir(&recv_tmp) .context("while reading tmp dir")? .map(|r| { r.map(|entry| entry.path()) .context("while iterating tmp dir") }) - .collect::>()?; + .collect::>()?; if entries.len() != 1 { - return Err(anyhow!( - "did not get exactly one subvolume received: {entries:?}" - )); + return Err( + anyhow!("did not get exactly one subvolume received: {entries:?}").into(), + ); } trace!("opening received subvol: {}", entries[0].display()); @@ -214,5 +242,15 @@ fn main() -> Result<()> { .with(tracing_subscriber::EnvFilter::from_default_env()) .init(); - args.run() + let res = args.run(); + if let Err(e) = &res { + if let Some(category) = e.category() { + antlir2_error_handler::SubError::builder() + .category(category) + .message(e) + .build() + .log(); + } + } + res } diff --git a/antlir/antlir2/bzl/image/layer.bzl b/antlir/antlir2/bzl/image/layer.bzl index c0d3acaf90..53641cc359 100644 --- a/antlir/antlir2/bzl/image/layer.bzl +++ b/antlir/antlir2/bzl/image/layer.bzl @@ -5,6 +5,7 @@ load("@prelude//utils:expect.bzl", "expect") load("@prelude//utils:selects.bzl", "selects") +load("//antlir/antlir2/antlir2_error_handler:handler.bzl", "antlir2_error_handler") load("//antlir/antlir2/antlir2_overlayfs:overlayfs.bzl", "OverlayFs", "OverlayLayer", "get_antlir2_use_overlayfs") load("//antlir/antlir2/antlir2_rootless:package.bzl", "get_antlir2_rootless") load("//antlir/antlir2/bzl:build_phase.bzl", "BuildPhase", "verify_build_phases") @@ -117,6 +118,7 @@ def _compile( local_only = not ctx.attrs._overlayfs, # the old output is used to clean up the local subvolume no_outputs_cleanup = not ctx.attrs._overlayfs, + error_handler = antlir2_error_handler, ) return LayerContents( diff --git a/antlir/antlir2/bzl/image/prebuilt.bzl b/antlir/antlir2/bzl/image/prebuilt.bzl index 7053e6ae24..739a6ddb18 100644 --- a/antlir/antlir2/bzl/image/prebuilt.bzl +++ b/antlir/antlir2/bzl/image/prebuilt.bzl @@ -4,6 +4,7 @@ # LICENSE file in the root directory of this source tree. load("@prelude//utils:selects.bzl", "selects") +load("//antlir/antlir2/antlir2_error_handler:handler.bzl", "antlir2_error_handler") load("//antlir/antlir2/antlir2_overlayfs:overlayfs.bzl", "get_antlir2_use_overlayfs") load("//antlir/antlir2/antlir2_rootless:cfg.bzl", "rootless_cfg") load("//antlir/antlir2/antlir2_rootless:package.bzl", "get_antlir2_rootless") @@ -95,6 +96,7 @@ def _impl(ctx: AnalysisContext) -> list[Provider]: cmd_args("--working-format=overlayfs") if ctx.attrs._overlayfs else cmd_args(), ), category = "antlir2_prebuilt_layer", + identifier = format, # needs local subvolumes local_only = not ctx.attrs._overlayfs, # the old output is used to clean up the local subvolume @@ -102,6 +104,7 @@ def _impl(ctx: AnalysisContext) -> list[Provider]: env = { "RUST_LOG": "antlir2=trace", }, + error_handler = antlir2_error_handler, ) contents = LayerContents(subvol_symlink = subvol_symlink)