Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nexus] Add tests for instance_create unwind safety #1843

Merged
merged 25 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
02c50b5
VERY wip; basically just refactoring Nexus interfaces for testability
smklein Oct 10, 2022
971f0b8
hacks
smklein Oct 10, 2022
f8081d1
Merge branch 'main' into disk-create-idempotency
smklein Oct 10, 2022
7bcf9d6
Merge branch 'main' into disk-create-idempotency
smklein Oct 17, 2022
31bbc22
Add unwind test, add undo actions
smklein Oct 19, 2022
a8f9e5d
Undo the saga interface changes
smklein Oct 19, 2022
85862c5
[nexus] Add tests for instance_create idempotency
smklein Oct 20, 2022
e44438d
Make nexus-test-utils no longer depend on Nexus, for the most part.
smklein Oct 26, 2022
f210c6d
cargo doc didn't like my ascii art
smklein Oct 26, 2022
0d1f8bf
Add prefix
smklein Oct 26, 2022
3ad8db3
Merge branch 'main' into disk-create-idempotency
smklein Oct 26, 2022
5915192
Merge branch 'disk-create-idempotency' into instance-create-saga-tests
smklein Oct 26, 2022
5422a0f
Merge updates, using generic nexus-test-utils and test-only DB access
smklein Oct 26, 2022
35a1b73
Merge branch 'main' into disk-create-idempotency
smklein Nov 1, 2022
6cc0e03
Merge branch 'main' into disk-create-idempotency
smklein Nov 1, 2022
dbd6276
Merge branch 'disk-create-idempotency' into instance-create-saga-tests
smklein Nov 1, 2022
320a380
Use new steno interface
smklein Nov 2, 2022
de670d1
Merge branch 'main' into disk-create-idempotency
smklein Nov 2, 2022
bd5cd0a
Use new steno API to automatically iterate over all nodes
smklein Nov 2, 2022
d63fed2
fmt
smklein Nov 2, 2022
6d8f5e7
Merge branch 'disk-create-idempotency' into instance-create-saga-tests
smklein Nov 2, 2022
4fe4896
fmt
smklein Nov 2, 2022
5c765fc
Merge branch 'main' into disk-create-idempotency
smklein Nov 2, 2022
2d06f71
Fix steno version
smklein Nov 2, 2022
301bcac
Merge branch 'disk-create-idempotency' into instance-create-saga-tests
smklein Nov 2, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ serde_json = "1.0"
serde_with = "2.0.1"
slog = { version = "2.5", features = [ "max_level_trace", "release_max_level_debug" ] }
smf = "0.2"
steno = "0.2"
steno = { git = "https://github.com/oxidecomputer/steno", branch = "node-access" }
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, I'd use a new rev of steno instead before merging. Same elsewhere in this PR

thiserror = "1.0"
tokio = { version = "1.21", features = [ "full" ] }
tokio-postgres = { version = "0.7", features = [ "with-chrono-0_4", "with-uuid-1" ] }
Expand Down
4 changes: 3 additions & 1 deletion nexus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ serde_urlencoded = "0.7.1"
serde_with = "2.0.1"
sled-agent-client = { path = "../sled-agent-client" }
slog-dtrace = "0.2"
steno = "0.2"
steno = { git = "https://github.com/oxidecomputer/steno", branch = "node-access" }
tempfile = "3.3"
thiserror = "1.0"
toml = "0.5.9"
Expand Down Expand Up @@ -121,7 +121,9 @@ itertools = "0.10.5"
nexus-test-utils-macros = { path = "test-utils-macros" }
nexus-test-utils = { path = "test-utils" }
omicron-test-utils = { path = "../test-utils" }
omicron-sled-agent = { path = "../sled-agent" }
openapiv3 = "1.0"
petgraph = "0.6.2"
regex = "1.6.0"
subprocess = "0.2.9"
term = "0.7"
Expand Down
2 changes: 1 addition & 1 deletion nexus/db-model/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
uuid = { version = "1.2.1", features = ["serde", "v4"] }

steno = "0.2"
steno = { git = "https://github.com/oxidecomputer/steno", branch = "node-access" }

db-macros = { path = "../db-macros" }
omicron-common = { path = "../../common" }
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ mod organization;
mod oximeter;
mod project;
mod rack;
mod saga;
pub mod saga;
mod session;
mod silo;
mod sled;
Expand All @@ -46,7 +46,7 @@ mod vpc_subnet;

// Sagas are not part of the "Nexus" implementation, but they are
// application logic.
mod sagas;
pub mod sagas;

// TODO: When referring to API types, we should try to include
// the prefix unless it is unambiguous.
Expand Down
76 changes: 58 additions & 18 deletions nexus/src/app/saga.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::authz;
use crate::context::OpContext;
use crate::saga_interface::SagaContext;
use anyhow::Context;
use futures::future::BoxFuture;
use futures::StreamExt;
use omicron_common::api::external;
use omicron_common::api::external::DataPageParams;
Expand All @@ -24,9 +25,32 @@ use steno::DagBuilder;
use steno::SagaDag;
use steno::SagaId;
use steno::SagaName;
use steno::SagaResult;
use steno::SagaResultOk;
use uuid::Uuid;

pub struct RunnableSaga {
id: SagaId,
fut: BoxFuture<'static, SagaResult>,
}

impl RunnableSaga {
pub fn id(&self) -> SagaId {
self.id
}
}

pub fn create_saga_dag<N: NexusSaga>(
params: N::Params,
) -> Result<SagaDag, Error> {
let builder = DagBuilder::new(SagaName::new(N::NAME));
let dag = N::make_saga_dag(&params, builder)?;
let params = serde_json::to_value(&params).map_err(|e| {
SagaInitError::SerializeError(String::from("saga params"), e)
})?;
Ok(SagaDag::new(dag, params))
}

impl super::Nexus {
pub async fn sagas_list(
&self,
Expand Down Expand Up @@ -66,27 +90,18 @@ impl super::Nexus {
})?
}

/// Given a saga type and parameters, create a new saga and execute it.
pub(crate) async fn execute_saga<N: NexusSaga>(
pub async fn create_runnable_saga(
self: &Arc<Self>,
params: N::Params,
) -> Result<SagaResultOk, Error> {
let saga = {
let builder = DagBuilder::new(SagaName::new(N::NAME));
let dag = N::make_saga_dag(&params, builder)?;
let params = serde_json::to_value(&params).map_err(|e| {
SagaInitError::SerializeError(String::from("saga params"), e)
})?;
SagaDag::new(dag, params)
};

dag: SagaDag,
) -> Result<RunnableSaga, Error> {
// Construct the context necessary to execute this saga.
let saga_id = SagaId(Uuid::new_v4());
let saga_logger = self.log.new(o!(
"saga_name" => saga.saga_name().to_string(),
"saga_name" => dag.saga_name().to_string(),
"saga_id" => saga_id.to_string()
));
let saga_context = Arc::new(Arc::new(SagaContext::new(
Arc::clone(self),
self.clone(),
saga_logger,
Arc::clone(&self.authz),
)));
Expand All @@ -95,7 +110,7 @@ impl super::Nexus {
.saga_create(
saga_id,
saga_context,
Arc::new(saga),
Arc::new(dag),
ACTION_REGISTRY.clone(),
)
.await
Expand All @@ -106,14 +121,20 @@ impl super::Nexus {
// Steno.
Error::internal_error(&format!("{:#}", error))
})?;
Ok(RunnableSaga { id: saga_id, fut: future })
}

pub async fn run_saga(
&self,
runnable_saga: RunnableSaga,
) -> Result<SagaResultOk, Error> {
self.sec_client
.saga_start(saga_id)
.saga_start(runnable_saga.id)
.await
.context("starting saga")
.map_err(|error| Error::internal_error(&format!("{:#}", error)))?;

let result = future.await;
let result = runnable_saga.fut.await;
result.kind.map_err(|saga_error| {
saga_error
.error_source
Expand All @@ -125,4 +146,23 @@ impl super::Nexus {
))
})
}

pub fn sec(&self) -> &steno::SecClient {
&self.sec_client
}

/// Given a saga type and parameters, create a new saga and execute it.
pub(crate) async fn execute_saga<N: NexusSaga>(
self: &Arc<Self>,
params: N::Params,
) -> Result<SagaResultOk, Error> {
// Construct the DAG specific to this saga.
let dag = create_saga_dag::<N>(params)?;

// Register the saga with the saga executor.
let runnable_saga = self.create_runnable_saga(dag).await?;

// Actually run the saga to completion.
self.run_saga(runnable_saga).await
}
}
Loading