Skip to content

Commit aa8d7fb

Browse files
committed
Add a try_fail_point! macro and use it in more places
When I was debugging https://issues.redhat.com/browse/OCPBUGS-2866 it would have been quite convenient to have a failpoint set up in the transaction init path, because I could have forcibly injected a sleep there. Add one there, and in a few other places I semi-randomly chose by skimming through the code. The cost of this is very low, but if we find other bugs in the future it will likely be useful. Further, we can and should start using failpoints to forcibly inject errors in our CI tests. Since movement on review of tikv/fail-rs#68 has been slow, we just copy the code into our repo for now.
1 parent 384c1ad commit aa8d7fb

File tree

9 files changed

+69
-22
lines changed

9 files changed

+69
-22
lines changed

rust/src/cxxrsutil.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,12 @@ mod err {
150150
}
151151
}
152152

153+
impl From<Box<dyn std::error::Error + Send + Sync>> for CxxError {
154+
fn from(e: Box<dyn std::error::Error + Send + Sync>) -> Self {
155+
Self(e.to_string())
156+
}
157+
}
158+
153159
impl From<cxx::Exception> for CxxError {
154160
fn from(v: cxx::Exception) -> Self {
155161
Self(format!("{:#}", v))

rust/src/failpoint_bridge.rs

Lines changed: 0 additions & 16 deletions
This file was deleted.

rust/src/failpoints.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//! Wrappers and utilities on top of the `fail` crate.
2+
// SPDX-License-Identifier: Apache-2.0 OR MIT
3+
4+
use anyhow::Result;
5+
6+
/// TODO: Use https://github.com/tikv/fail-rs/pull/68 once it merges
7+
#[macro_export]
8+
macro_rules! try_fail_point {
9+
($name:expr) => {{
10+
if let Some(e) = fail::eval($name, |msg| {
11+
let msg = msg.unwrap_or_else(|| "synthetic failpoint".to_string());
12+
anyhow::Error::msg(msg)
13+
}) {
14+
return Err(From::from(e));
15+
}
16+
}};
17+
($name:expr, $cond:expr) => {{
18+
if $cond {
19+
$crate::try_fail_point!($name);
20+
}
21+
}};
22+
}
23+
24+
/// Expose the `fail::fail_point` macro to C++.
25+
pub fn failpoint(p: &str) -> Result<()> {
26+
ostree_ext::glib::g_debug!("rpm-ostree", "{}", p);
27+
fail::fail_point!(p, |r| {
28+
Err(match r {
29+
Some(ref msg) => anyhow::anyhow!("{}", msg),
30+
None => anyhow::anyhow!("failpoint {}", p),
31+
})
32+
});
33+
Ok(())
34+
}

rust/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ pub mod ffi {
360360
fn generate_object_path(base: &str, next_segment: &str) -> Result<String>;
361361
}
362362

363-
// failpoint_bridge.rs
363+
// failpoints.rs
364364
extern "Rust" {
365365
fn failpoint(p: &str) -> Result<()>;
366366
}
@@ -914,8 +914,8 @@ pub(crate) use daemon::*;
914914
mod deployment_utils;
915915
pub(crate) use deployment_utils::*;
916916
mod dirdiff;
917-
pub mod failpoint_bridge;
918-
use failpoint_bridge::*;
917+
pub mod failpoints;
918+
use failpoints::*;
919919
mod extensions;
920920
pub(crate) use extensions::*;
921921
#[cfg(feature = "fedora-integration")]
@@ -956,6 +956,6 @@ mod testutils;
956956
pub(crate) use self::testutils::*;
957957
mod treefile;
958958
pub use self::treefile::*;
959-
mod utils;
959+
pub(crate) mod utils;
960960
pub use self::utils::*;
961961
mod variant_utils;

rust/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ fn inner_main() -> Result<i32> {
123123
}
124124
// Initialize failpoints
125125
let _scenario = fail::FailScenario::setup();
126-
fail::fail_point!("main");
126+
rpmostree_rust::try_fail_point!("main");
127127
// Call this early on; it invokes e.g. setenv() so must be done
128128
// before we create threads.
129129
rpmostree_rust::ffi::early_main();

rust/src/sysroot_upgrade.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ pub(crate) fn pull_container(
123123
let cancellable = cancellable.glib_reborrow();
124124
let imgref = &OstreeImageReference::try_from(imgref)?;
125125

126+
crate::try_fail_point!("sysroot-upgrade::container-pull");
127+
126128
let r = Handle::current().block_on(async {
127129
crate::utils::run_with_cancellable(
128130
async { pull_container_async(repo, imgref).await },
@@ -141,6 +143,7 @@ pub(crate) fn layer_prune(
141143
c.set_error_if_cancelled()?;
142144
}
143145
tracing::debug!("pruning image layers");
146+
crate::try_fail_point!("sysroot-upgrade::layer-prune");
144147
let n_pruned = ostree_ext::container::store::gc_image_layers(repo)?;
145148
systemd::journal::print(6, &format!("Pruned container image layers: {n_pruned}"));
146149
Ok(())

src/daemon/rpmostreed-transaction.cxx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,17 @@ transaction_execute_thread (GTask *task, gpointer source_object, gpointer task_d
329329
// Further, we join the main Tokio async runtime.
330330
auto guard = rpmostreecxx::rpmostreed_daemon_tokio_enter (rpmostreed_daemon_get ());
331331

332-
if (clazz->execute != NULL)
332+
try
333+
{
334+
rpmostreecxx::failpoint ("transaction::execute");
335+
}
336+
catch (std::exception &e)
337+
{
338+
success = FALSE;
339+
glnx_throw (&local_error, "%s", e.what ());
340+
}
341+
342+
if (success && clazz->execute != NULL)
333343
{
334344
// This try/catch shouldn't be needed; every CXX call should be wrapped with the CXX macro.
335345
// But in case we regress on this, it's better to error out than crash.
@@ -605,6 +615,8 @@ transaction_initable_init (GInitable *initable, GCancellable *cancellable, GErro
605615
return FALSE;
606616
sd_journal_print (LOG_INFO, "Loaded sysroot");
607617

618+
CXX_TRY (rpmostreecxx::failpoint ("transaction::lock"), error);
619+
608620
if (!ostree_sysroot_try_lock (priv->sysroot, &lock_acquired, error))
609621
return FALSE;
610622

src/libpriv/rpmostree-core.cxx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,6 +1416,7 @@ check_goal_solution (RpmOstreeContext *self, GPtrArray *removed_pkgnames,
14161416
GHashTable *replaced_pkgnames, GError **error)
14171417
{
14181418
HyGoal goal = dnf_context_get_goal (self->dnfctx);
1419+
CXX_TRY (rpmostreecxx::failpoint ("core::check-goal-solution"), error);
14191420

14201421
/* check that we're not removing anything we didn't expect */
14211422
{
@@ -4057,6 +4058,8 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
40574058
return FALSE;
40584059
int tmprootfs_dfd = self->tmprootfs_dfd; /* Alias to avoid bigger diff */
40594060

4061+
CXX_TRY (rpmostreecxx::failpoint ("core::assemble"), error);
4062+
40604063
/* In e.g. removing a package we walk librpm which doesn't have canonical
40614064
* /usr, so we need to build up a mapping.
40624065
*/
@@ -4503,6 +4506,8 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
45034506
gboolean
45044507
rpmostree_context_assemble_end (RpmOstreeContext *self, GCancellable *cancellable, GError **error)
45054508
{
4509+
CXX_TRY (rpmostreecxx::failpoint ("core::assemble-end"), error);
4510+
45064511
if (!ensure_tmprootfs_dfd (self, error))
45074512
return FALSE;
45084513
if (self->treefile_rs->get_cliwrap ())
@@ -4534,6 +4539,7 @@ rpmostree_context_commit (RpmOstreeContext *self, const char *parent,
45344539
g_autofree char *ret_commit_checksum = NULL;
45354540

45364541
auto task = rpmostreecxx::progress_begin_task ("Writing OSTree commit");
4542+
CXX_TRY (rpmostreecxx::failpoint ("core::commit"), error);
45374543

45384544
g_auto (RpmOstreeRepoAutoTransaction) txn = {
45394545
0,

src/libpriv/rpmostree-importer.cxx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,8 @@ rpmostree_importer_run (RpmOstreeImporter *self, char **out_csum, char **out_met
677677
if (g_cancellable_set_error_if_cancelled (cancellable, error))
678678
return FALSE;
679679

680+
CXX_TRY (rpmostreecxx::failpoint ("rpm-importer::run"), error);
681+
680682
g_autofree char *metadata_sha256 = NULL;
681683
g_autofree char *csum = NULL;
682684
if (!import_rpm_to_repo (self, &csum, &metadata_sha256, cancellable, error))

0 commit comments

Comments
 (0)