From b2a2ba103cf6a1dd38c4a5493ffe758a4e14a6e4 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 15 Dec 2022 22:38:29 -0500 Subject: [PATCH 1/8] [sagas] Make a macro to simplify declaring saga actions --- Cargo.lock | 1 + nexus/Cargo.toml | 1 + nexus/src/app/sagas/disk_create.rs | 93 +++++---------- nexus/src/app/sagas/disk_delete.rs | 38 ++----- nexus/src/app/sagas/instance_create.rs | 123 +++++++------------- nexus/src/app/sagas/instance_migrate.rs | 69 ++++-------- nexus/src/app/sagas/mod.rs | 126 +++++++++++++++++++++ nexus/src/app/sagas/snapshot_create.rs | 138 +++++++---------------- nexus/src/app/sagas/volume_delete.rs | 88 ++++----------- nexus/src/app/sagas/volume_remove_rop.rs | 42 +++---- 10 files changed, 308 insertions(+), 411 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5ed5f3bfa3a..0ae539c2953 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3464,6 +3464,7 @@ dependencies = [ "oximeter-instruments", "oximeter-producer", "parse-display", + "paste", "petgraph", "pq-sys", "propolis-client", diff --git a/nexus/Cargo.toml b/nexus/Cargo.toml index d139b313ad0..278a724c116 100644 --- a/nexus/Cargo.toml +++ b/nexus/Cargo.toml @@ -45,6 +45,7 @@ oso = "0.26" oximeter-client = { path = "../oximeter-client" } oximeter-db = { path = "../oximeter/db/" } parse-display = "0.7.0" +paste = "1.0" # See omicron-rpaths for more about the "pq-sys" dependency. pq-sys = "*" propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "b596e72b6b93bc412d675358f782ae7d53f8bf7a", features = [ "generated" ] } diff --git a/nexus/src/app/sagas/disk_create.rs b/nexus/src/app/sagas/disk_create.rs index 9a66030aebb..16ecdcd051c 100644 --- a/nexus/src/app/sagas/disk_create.rs +++ b/nexus/src/app/sagas/disk_create.rs @@ -9,23 +9,20 @@ use super::{ ActionRegistry, NexusActionContext, NexusSaga, SagaInitError, ACTION_GENERATE_ID, }; -use crate::app::sagas::NexusAction; +use crate::app::sagas::declare_saga_actions; use crate::context::OpContext; use crate::db::identity::{Asset, Resource}; use crate::db::lookup::LookupPath; use crate::external_api::params; use crate::{authn, authz, db}; -use lazy_static::lazy_static; use omicron_common::api::external::Error; use rand::{rngs::StdRng, RngCore, SeedableRng}; use serde::Deserialize; use serde::Serialize; use sled_agent_client::types::{CrucibleOpts, VolumeConstructionRequest}; use std::convert::TryFrom; -use std::sync::Arc; use steno::ActionError; -use steno::ActionFunc; -use steno::{new_action_noop_undo, Node}; +use steno::Node; use uuid::Uuid; // disk create saga: input parameters @@ -39,31 +36,27 @@ pub struct Params { // disk create saga: actions -lazy_static! { - static ref CREATE_DISK_RECORD: NexusAction = ActionFunc::new_action( - "disk-create.create-disk-record", - sdc_create_disk_record, - sdc_create_disk_record_undo - ); - static ref REGIONS_ALLOC: NexusAction = ActionFunc::new_action( - "disk-create.regions-alloc", - sdc_alloc_regions, - sdc_alloc_regions_undo, - ); - static ref REGIONS_ENSURE: NexusAction = ActionFunc::new_action( - "disk-create.regions-ensure", - sdc_regions_ensure, - sdc_regions_ensure_undo, - ); - static ref CREATE_VOLUME_RECORD: NexusAction = ActionFunc::new_action( - "disk-create.create-volume-record", - sdc_create_volume_record, - sdc_create_volume_record_undo, - ); - static ref FINALIZE_DISK_RECORD: NexusAction = new_action_noop_undo( - "disk-create.finalize-disk-record", - sdc_finalize_disk_record - ); +declare_saga_actions! { + disk_create; + CREATE_DISK_RECORD -> "created_disk" { + + sdc_create_disk_record + - sdc_create_disk_record_undo + } + REGIONS_ALLOC -> "datasets_and_regions" { + + sdc_alloc_regions + - sdc_alloc_regions_undo + } + REGIONS_ENSURE -> "regions_ensure" { + + sdc_regions_ensure + - sdc_regions_ensure_undo + } + CREATE_VOLUME_RECORD -> "created_volume" { + + sdc_create_volume_record + - sdc_create_volume_record_undo + } + FINALIZE_DISK_RECORD -> "disk_runtime" { + + sdc_finalize_disk_record + } } // disk create saga: definition @@ -75,11 +68,7 @@ impl NexusSaga for SagaDiskCreate { type Params = Params; fn register_actions(registry: &mut ActionRegistry) { - registry.register(Arc::clone(&*CREATE_DISK_RECORD)); - registry.register(Arc::clone(&*REGIONS_ALLOC)); - registry.register(Arc::clone(&*REGIONS_ENSURE)); - registry.register(Arc::clone(&*CREATE_VOLUME_RECORD)); - registry.register(Arc::clone(&*FINALIZE_DISK_RECORD)); + disk_create_register_actions(registry); } fn make_saga_dag( @@ -98,35 +87,11 @@ impl NexusSaga for SagaDiskCreate { ACTION_GENERATE_ID.as_ref(), )); - builder.append(Node::action( - "created_disk", - "CreateDiskRecord", - CREATE_DISK_RECORD.as_ref(), - )); - - builder.append(Node::action( - "datasets_and_regions", - "RegionsAlloc", - REGIONS_ALLOC.as_ref(), - )); - - builder.append(Node::action( - "regions_ensure", - "RegionsEnsure", - REGIONS_ENSURE.as_ref(), - )); - - builder.append(Node::action( - "created_volume", - "CreateVolumeRecord", - CREATE_VOLUME_RECORD.as_ref(), - )); - - builder.append(Node::action( - "disk_runtime", - "FinalizeDiskRecord", - FINALIZE_DISK_RECORD.as_ref(), - )); + builder.append(create_disk_record_action()); + builder.append(regions_alloc_action()); + builder.append(regions_ensure_action()); + builder.append(create_volume_record_action()); + builder.append(finalize_disk_record_action()); Ok(builder.build()?) } diff --git a/nexus/src/app/sagas/disk_delete.rs b/nexus/src/app/sagas/disk_delete.rs index 9cb8ac04b88..c26f91f4299 100644 --- a/nexus/src/app/sagas/disk_delete.rs +++ b/nexus/src/app/sagas/disk_delete.rs @@ -5,14 +5,10 @@ use super::ActionRegistry; use super::NexusActionContext; use super::NexusSaga; -use crate::app::sagas::NexusAction; -use lazy_static::lazy_static; +use crate::app::sagas::declare_saga_actions; use serde::Deserialize; use serde::Serialize; -use std::sync::Arc; -use steno::new_action_noop_undo; use steno::ActionError; -use steno::Node; use uuid::Uuid; // disk delete saga: input parameters @@ -24,18 +20,17 @@ pub struct Params { // disk delete saga: actions -lazy_static! { - static ref DELETE_DISK_RECORD: NexusAction = new_action_noop_undo( - "disk-delete.delete-disk-record", +declare_saga_actions! { + disk_delete; + DELETE_DISK_RECORD -> "volume_id" { // TODO: See the comment on the "DeleteRegions" step, // we may want to un-delete the disk if we cannot remove // underlying regions. - sdd_delete_disk_record - ); - static ref DELETE_VOLUME: NexusAction = new_action_noop_undo( - "disk-delete.delete-volume", - sdd_delete_volume - ); + + sdd_delete_disk_record + } + DELETE_VOLUME -> "no_result" { + + sdd_delete_volume + } } // disk delete saga: definition @@ -47,24 +42,15 @@ impl NexusSaga for SagaDiskDelete { type Params = Params; fn register_actions(registry: &mut ActionRegistry) { - registry.register(Arc::clone(&*DELETE_DISK_RECORD)); - registry.register(Arc::clone(&*DELETE_VOLUME)); + disk_delete_register_actions(registry); } fn make_saga_dag( _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { - builder.append(Node::action( - "volume_id", - "DeleteDiskRecord", - DELETE_DISK_RECORD.as_ref(), - )); - builder.append(Node::action( - "no_result", - "DeleteVolume", - DELETE_VOLUME.as_ref(), - )); + builder.append(delete_disk_record_action()); + builder.append(delete_volume_action()); Ok(builder.build()?) } } diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 0a65eba18fb..4b200567cda 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -3,8 +3,8 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use super::{NexusActionContext, NexusSaga, SagaInitError, ACTION_GENERATE_ID}; +use crate::app::sagas::declare_saga_actions; use crate::app::sagas::disk_create::{self, SagaDiskCreate}; -use crate::app::sagas::NexusAction; use crate::app::{ MAX_DISKS_PER_INSTANCE, MAX_EXTERNAL_IPS_PER_INSTANCE, MAX_NICS_PER_INSTANCE, @@ -16,7 +16,6 @@ use crate::db::queries::network_interface::InsertError as InsertNicError; use crate::external_api::params; use crate::{authn, authz, db}; use chrono::Utc; -use lazy_static::lazy_static; use nexus_defaults::DEFAULT_PRIMARY_NIC_NAME; use nexus_types::external_api::params::InstanceDiskAttachment; use omicron_common::api::external::Error; @@ -33,10 +32,7 @@ use slog::warn; use std::convert::TryFrom; use std::fmt::Debug; use std::net::Ipv6Addr; -use std::sync::Arc; -use steno::new_action_noop_undo; use steno::ActionError; -use steno::ActionFunc; use steno::Node; use steno::{DagBuilder, SagaName}; use uuid::Uuid; @@ -71,47 +67,40 @@ struct DiskAttachParams { // instance create saga: actions -lazy_static! { - static ref ALLOC_SERVER: NexusAction = new_action_noop_undo( - // TODO-robustness This still needs an undo action, and we should really - // keep track of resources and reservations, etc. See the comment on - // SagaContext::alloc_server() - "instance-create.alloc-server", - sic_alloc_server - ); - static ref ALLOC_PROPOLIS_IP: NexusAction = new_action_noop_undo( - "instance-create.allocate-propolis-ip", - sic_allocate_propolis_ip, - ); - static ref CREATE_INSTANCE_RECORD: NexusAction = ActionFunc::new_action( - "instance-create.create-instance-record", - sic_create_instance_record, - sic_delete_instance_record, - ); - static ref CREATE_NETWORK_INTERFACE: NexusAction = ActionFunc::new_action( - "instance-create.create-network-interface", - sic_create_network_interface, - sic_create_network_interface_undo, - ); - static ref CREATE_SNAT_IP: NexusAction = ActionFunc::new_action( - "instance-create.create-snat-ip", - sic_allocate_instance_snat_ip, - sic_allocate_instance_snat_ip_undo, - ); - static ref CREATE_EXTERNAL_IP: NexusAction = ActionFunc::new_action( - "instance-create.create-external-ip", - sic_allocate_instance_external_ip, - sic_allocate_instance_external_ip_undo, - ); - static ref ATTACH_DISKS_TO_INSTANCE: NexusAction = ActionFunc::new_action( - "instance-create.attach-disks-to-instance", - sic_attach_disk_to_instance, - sic_attach_disk_to_instance_undo, - ); - static ref INSTANCE_ENSURE: NexusAction = new_action_noop_undo( - "instance-create.instance-ensure", - sic_instance_ensure, - ); +declare_saga_actions! { + instance_create; + // TODO-robustness This still needs an undo action, and we should really + // keep track of resources and reservations, etc. See the comment on + // SagaContext::alloc_server() + ALLOC_SERVER -> "server_id" { + + sic_alloc_server + } + ALLOC_PROPOLIS_IP -> "propolis_ip" { + + sic_allocate_propolis_ip + } + CREATE_INSTANCE_RECORD -> "instance_name" { + + sic_create_instance_record + - sic_delete_instance_record + } + CREATE_NETWORK_INTERFACE -> "output" { + + sic_create_network_interface + - sic_create_network_interface_undo + } + CREATE_SNAT_IP -> "snat_ip" { + + sic_allocate_instance_snat_ip + - sic_allocate_instance_snat_ip_undo + } + CREATE_EXTERNAL_IP -> "output" { + + sic_allocate_instance_external_ip + - sic_allocate_instance_external_ip_undo + } + ATTACH_DISKS_TO_INSTANCE -> "attach_disk_output" { + + sic_attach_disk_to_instance + - sic_attach_disk_to_instance_undo + } + INSTANCE_ENSURE -> "instance_ensure" { + + sic_instance_ensure + } } // instance create saga: definition @@ -123,14 +112,7 @@ impl NexusSaga for SagaInstanceCreate { type Params = Params; fn register_actions(registry: &mut super::ActionRegistry) { - registry.register(Arc::clone(&*ALLOC_SERVER)); - registry.register(Arc::clone(&*ALLOC_PROPOLIS_IP)); - registry.register(Arc::clone(&*CREATE_INSTANCE_RECORD)); - registry.register(Arc::clone(&*CREATE_NETWORK_INTERFACE)); - registry.register(Arc::clone(&*CREATE_SNAT_IP)); - registry.register(Arc::clone(&*CREATE_EXTERNAL_IP)); - registry.register(Arc::clone(&*ATTACH_DISKS_TO_INSTANCE)); - registry.register(Arc::clone(&*INSTANCE_ENSURE)); + instance_create_register_actions(registry); } fn make_saga_dag( @@ -152,23 +134,9 @@ impl NexusSaga for SagaInstanceCreate { ACTION_GENERATE_ID.as_ref(), )); - builder.append(Node::action( - "server_id", - "AllocServer", - ALLOC_SERVER.as_ref(), - )); - - builder.append(Node::action( - "propolis_ip", - "AllocatePropolisIp", - ALLOC_PROPOLIS_IP.as_ref(), - )); - - builder.append(Node::action( - "instance_name", - "CreateInstanceRecord", - CREATE_INSTANCE_RECORD.as_ref(), - )); + builder.append(alloc_server_action()); + builder.append(alloc_propolis_ip_action()); + builder.append(create_instance_record_action()); // Helper function for appending subsagas to our parent saga. fn subsaga_append( @@ -250,11 +218,7 @@ impl NexusSaga for SagaInstanceCreate { "CreateSnatIpId", ACTION_GENERATE_ID.as_ref(), )); - builder.append(Node::action( - "snat_ip", - "CreateSnatIp", - CREATE_SNAT_IP.as_ref(), - )); + builder.append(create_snat_ip_action()); // See the comment above where we add nodes for creating NICs. We use // the same pattern here. @@ -329,12 +293,7 @@ impl NexusSaga for SagaInstanceCreate { )?; } - builder.append(Node::action( - "instance_ensure", - "InstanceEnsure", - INSTANCE_ENSURE.as_ref(), - )); - + builder.append(instance_ensure_action()); Ok(builder.build()?) } } diff --git a/nexus/src/app/sagas/instance_migrate.rs b/nexus/src/app/sagas/instance_migrate.rs index eac1c8e6e2d..a94c008d4b7 100644 --- a/nexus/src/app/sagas/instance_migrate.rs +++ b/nexus/src/app/sagas/instance_migrate.rs @@ -4,13 +4,12 @@ use super::instance_create::allocate_sled_ipv6; use super::{NexusActionContext, NexusSaga, ACTION_GENERATE_ID}; -use crate::app::sagas::NexusAction; +use crate::app::sagas::declare_saga_actions; use crate::authn; use crate::context::OpContext; use crate::db::identity::Resource; use crate::db::model::IpKind; use crate::external_api::params; -use lazy_static::lazy_static; use omicron_common::api::external::Error; use omicron_common::api::internal::nexus::InstanceRuntimeState; use serde::Deserialize; @@ -23,9 +22,8 @@ use sled_agent_client::types::InstanceRuntimeStateRequested; use sled_agent_client::types::InstanceStateRequested; use sled_agent_client::types::SourceNatConfig; use std::net::Ipv6Addr; -use std::sync::Arc; use steno::ActionError; -use steno::{new_action_noop_undo, Node}; +use steno::Node; use uuid::Uuid; // instance migrate saga: input parameters @@ -39,26 +37,23 @@ pub struct Params { // instance migrate saga: actions -lazy_static! { - static ref ALLOCATE_PROPOLIS_IP: NexusAction = new_action_noop_undo( - "instance-migrate.allocate-propolis-ip", - sim_allocate_propolis_ip - ); - static ref MIGRATE_PREP: NexusAction = new_action_noop_undo( - "instance-migrate.migrate-prep", - sim_migrate_prep, - ); - static ref INSTANCE_MIGRATE: NexusAction = new_action_noop_undo( - "instance-migrate.instance-migrate", +declare_saga_actions! { + instance_migrate; + ALLOCATE_PROPOLIS_IP -> "dst_propolis_ip" { + + sim_allocate_propolis_ip + } + MIGRATE_PREP -> "migrate_instance" { + + sim_migrate_prep + } + INSTANCE_MIGRATE -> "instance_migrate" { // TODO robustness: This needs an undo action - sim_instance_migrate, - ); - static ref CLEANUP_SOURCE: NexusAction = new_action_noop_undo( - "instance-migrate.cleanup-source", + + sim_instance_migrate + } + CLEANUP_SOURCE -> "cleanup_source" { // TODO robustness: This needs an undo action. Is it even possible // to undo at this point? - sim_cleanup_source, - ); + + sim_cleanup_source + } } // instance migrate saga: definition @@ -70,10 +65,7 @@ impl NexusSaga for SagaInstanceMigrate { type Params = Params; fn register_actions(registry: &mut super::ActionRegistry) { - registry.register(Arc::clone(&*ALLOCATE_PROPOLIS_IP)); - registry.register(Arc::clone(&*MIGRATE_PREP)); - registry.register(Arc::clone(&*INSTANCE_MIGRATE)); - registry.register(Arc::clone(&*CLEANUP_SOURCE)); + instance_migrate_register_actions(registry); } fn make_saga_dag( @@ -92,29 +84,10 @@ impl NexusSaga for SagaInstanceMigrate { ACTION_GENERATE_ID.as_ref(), )); - builder.append(Node::action( - "dst_propolis_ip", - "AllocatePropolisIp", - ALLOCATE_PROPOLIS_IP.as_ref(), - )); - - builder.append(Node::action( - "migrate_instance", - "MigratePrep", - MIGRATE_PREP.as_ref(), - )); - - builder.append(Node::action( - "instance_migrate", - "InstanceMigrate", - INSTANCE_MIGRATE.as_ref(), - )); - - builder.append(Node::action( - "cleanup_source", - "CleanupSource", - CLEANUP_SOURCE.as_ref(), - )); + builder.append(allocate_propolis_ip_action()); + builder.append(migrate_prep_action()); + builder.append(instance_migrate_action()); + builder.append(cleanup_source_action()); Ok(builder.build()?) } diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index 3fdba98c73a..3dcf604f025 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -116,3 +116,129 @@ pub(super) async fn saga_generate_uuid( ) -> Result { Ok(Uuid::new_v4()) } + +macro_rules! __stringify_ident { + ($i:ident) => { + stringify!($i) + }; +} + +macro_rules! __emit_action { + ($node:ident, $output:literal) => { + paste::paste! { + #[allow(dead_code)] + fn [<$node:lower _action>]() -> ::steno::Node { + ::steno::Node::action( + $output, + crate::app::sagas::__stringify_ident!([<$node:camel>]), + $node.as_ref(), + ) + } + } + }; +} + +macro_rules! __action_name { + ($saga:ident, $node:ident) => { + paste::paste! { + concat!( + stringify!($saga), + ".", + crate::app::sagas::__stringify_ident!([<$node:lower>]), + ) + } + }; +} + +/// A macro intended to reduce boilerplate when writing saga actions. +/// +/// For this input: +/// +/// ``` +/// declare_saga_actions! { +/// my_saga; +/// SAGA_NODE1 -> "output1" { +/// + do1 +/// - undo1 +/// } +/// SAGA_NODE2 -> "output2" { +/// + do2 +/// } +/// } +/// ``` +/// +/// We generate the following: +/// - For `SAGA_NODE1`: +/// - A `NexusAction` labeled "my_saga.saga_node1" (containing "do1" and "undo1"). +/// - `fn saga_node1_action() -> steno::Node` referencing this node, with an +/// output named "output1". +/// - For `SAGA_NODE2`: +/// - A `NexusAction` labeled "my_saga.saga_node2" (containing "do2"). +/// - `fn saga_node2_action() -> steno::Node` referencing this node, with an +/// output named "output2". +/// - For `my_saga`: +/// - `fn my_saga_register_actions(...)`, which can be called to implement +/// `NexusSaga::register_actions`. +macro_rules! declare_saga_actions { + // The entrypoint to the macro. + // We expect the input to be of the form: + // + // saga-name; + ($saga:ident; $($tail:tt)*) => { + declare_saga_actions!(S = $saga <> $($tail)*); + }; + // Subsequent lines of the saga action declaration. + // These take the form: + // + // ACTION_NAME -> "output" { + // + action + // - undo_action + // } + // + // However, we also want to propagate the Saga structure and collection of + // all node names, so this is *actually* parsed with a hidden prefix: + // + // S = SagaName <> ... + // + // Basically, everything to the left of "<>" is just us propagating state + // through the macro, and everything to the right of it is user input. + (S = $saga:ident $($nodes:ident),* <> $node:ident -> $out:literal { + $a:ident - $u:ident } $($tail:tt)*) => { + lazy_static::lazy_static! { + static ref $node: crate::app::sagas::NexusAction = ::steno::ActionFunc::new_action( + crate::app::sagas::__action_name!($saga, $node), $a, $u, + ); + } + crate::app::sagas::__emit_action!($node, $out); + declare_saga_actions!(S = $saga $($nodes,)* $node <> $($tail)*); + }; + // Same as the prior match, but without the undo action. + (S = $saga:ident $($nodes:ident),* <> $node:ident -> $out:literal { + $a:ident } $($tail:tt)*) => { + lazy_static::lazy_static! { + static ref $node: crate::app::sagas::NexusAction = ::steno::new_action_noop_undo( + crate::app::sagas::__action_name!($saga, $node), $a, + ); + } + crate::app::sagas::__emit_action!($node, $out); + declare_saga_actions!(S = $saga $($nodes,)* $node <> $($tail)*); + }; + // The end of the macro, which registers all previous generated saga nodes. + // + // We generate a new function, rather than implementing + // "NexusSaga::register_actions", because traits cannot be partially + // implemented, and "make_saga_dag" is not being generated through this + // macro. + (S = $saga:ident $($nodes:ident),* <>) => { + paste::paste! { + fn [<$saga _register_actions>](registry: &mut crate::app::sagas::ActionRegistry) { + $( + registry.register(::std::sync::Arc::clone(&* $nodes )); + )* + } + } + }; +} + +pub(crate) use __action_name; +pub(crate) use __emit_action; +pub(crate) use __stringify_ident; +pub(crate) use declare_saga_actions; diff --git a/nexus/src/app/sagas/snapshot_create.rs b/nexus/src/app/sagas/snapshot_create.rs index b091bf662e0..f4310e7d67b 100644 --- a/nexus/src/app/sagas/snapshot_create.rs +++ b/nexus/src/app/sagas/snapshot_create.rs @@ -85,7 +85,7 @@ use super::{ common_storage::ensure_all_datasets_and_regions, ActionRegistry, NexusActionContext, NexusSaga, SagaInitError, ACTION_GENERATE_ID, }; -use crate::app::sagas::NexusAction; +use crate::app::sagas::declare_saga_actions; use crate::context::OpContext; use crate::db::identity::{Asset, Resource}; use crate::db::lookup::LookupPath; @@ -93,7 +93,6 @@ use crate::external_api::params; use crate::{authn, authz, db}; use anyhow::anyhow; use crucible_agent_client::{types::RegionId, Client as CrucibleAgentClient}; -use lazy_static::lazy_static; use omicron_common::api::external; use omicron_common::api::external::Error; use rand::{rngs::StdRng, RngCore, SeedableRng}; @@ -105,10 +104,7 @@ use sled_agent_client::types::{ }; use slog::info; use std::collections::BTreeMap; -use std::sync::Arc; -use steno::new_action_noop_undo; use steno::ActionError; -use steno::ActionFunc; use steno::Node; use uuid::Uuid; @@ -125,43 +121,35 @@ pub struct Params { // snapshot create saga: actions -lazy_static! { - static ref REGIONS_ALLOC: NexusAction = new_action_noop_undo( - "snapshot-create.regions-alloc", - ssc_alloc_regions, - ); - static ref REGIONS_ENSURE: NexusAction = new_action_noop_undo( - "snapshot-create.regions-ensure", - ssc_regions_ensure, - ); - static ref CREATE_DESTINATION_VOLUME_RECORD: NexusAction = - ActionFunc::new_action( - "snapshot-create.create-destination-volume-record", - ssc_create_destination_volume_record, - ssc_create_destination_volume_record_undo, - ); - static ref CREATE_SNAPSHOT_RECORD: NexusAction = ActionFunc::new_action( - "snapshot-create.create-snapshot-record", - ssc_create_snapshot_record, - ssc_create_snapshot_record_undo, - ); - static ref SEND_SNAPSHOT_REQUEST: NexusAction = new_action_noop_undo( - "snapshot-create.send-snapshot-request", - ssc_send_snapshot_request, - ); - static ref START_RUNNING_SNAPSHOT: NexusAction = new_action_noop_undo( - "snapshot-create.start-running-snapshot", - ssc_start_running_snapshot, - ); - static ref CREATE_VOLUME_RECORD: NexusAction = ActionFunc::new_action( - "snapshot-create.create-volume-record", - ssc_create_volume_record, - ssc_create_volume_record_undo, - ); - static ref FINALIZE_SNAPSHOT_RECORD: NexusAction = new_action_noop_undo( - "snapshot-create.finalize-snapshot-record", - ssc_finalize_snapshot_record, - ); +declare_saga_actions! { + snapshot_create; + REGIONS_ALLOC -> "datasets_and_regions" { + + ssc_alloc_regions + } + REGIONS_ENSURE -> "regions_ensure" { + + ssc_regions_ensure + } + CREATE_DESTINATION_VOLUME_RECORD -> "created_destination_volume" { + + ssc_create_destination_volume_record + - ssc_create_destination_volume_record_undo + } + CREATE_SNAPSHOT_RECORD -> "created_snapshot" { + + ssc_create_snapshot_record + - ssc_create_snapshot_record_undo + } + SEND_SNAPSHOT_REQUEST -> "snapshot_request" { + + ssc_send_snapshot_request + } + START_RUNNING_SNAPSHOT -> "replace_sockets_map" { + + ssc_start_running_snapshot + } + CREATE_VOLUME_RECORD -> "created_volume" { + + ssc_create_volume_record + - ssc_create_volume_record_undo + } + FINALIZE_SNAPSHOT_RECORD -> "finalized_snapshot" { + + ssc_finalize_snapshot_record + } } // snapshot create saga: definition @@ -173,14 +161,7 @@ impl NexusSaga for SagaSnapshotCreate { type Params = Params; fn register_actions(registry: &mut ActionRegistry) { - registry.register(Arc::clone(&*REGIONS_ALLOC)); - registry.register(Arc::clone(&*REGIONS_ENSURE)); - registry.register(Arc::clone(&*CREATE_DESTINATION_VOLUME_RECORD)); - registry.register(Arc::clone(&*CREATE_SNAPSHOT_RECORD)); - registry.register(Arc::clone(&*SEND_SNAPSHOT_REQUEST)); - registry.register(Arc::clone(&*START_RUNNING_SNAPSHOT)); - registry.register(Arc::clone(&*CREATE_VOLUME_RECORD)); - registry.register(Arc::clone(&*FINALIZE_SNAPSHOT_RECORD)); + snapshot_create_register_actions(registry); } fn make_saga_dag( @@ -207,58 +188,17 @@ impl NexusSaga for SagaSnapshotCreate { )); // Allocate region space for snapshot to store blocks post-scrub - builder.append(Node::action( - "datasets_and_regions", - "RegionsAlloc", - REGIONS_ALLOC.as_ref(), - )); - - builder.append(Node::action( - "regions_ensure", - "RegionsEnsure", - REGIONS_ENSURE.as_ref(), - )); - - builder.append(Node::action( - "created_destination_volume", - "CreateDestinationVolumeRecord", - CREATE_DESTINATION_VOLUME_RECORD.as_ref(), - )); - - // Create the Snapshot DB object - builder.append(Node::action( - "created_snapshot", - "CreateSnapshotRecord", - CREATE_SNAPSHOT_RECORD.as_ref(), - )); - - // Send a snapshot request to a sled-agent - builder.append(Node::action( - "snapshot_request", - "SendSnapshotRequest", - SEND_SNAPSHOT_REQUEST.as_ref(), - )); - + builder.append(regions_alloc_action()); + builder.append(regions_ensure_action()); + builder.append(create_destination_volume_record_action()); + builder.append(create_snapshot_record_action()); + builder.append(send_snapshot_request_action()); // Validate with crucible agent and start snapshot downstairs - builder.append(Node::action( - "replace_sockets_map", - "StartRunningSnapshot", - START_RUNNING_SNAPSHOT.as_ref(), - )); - + builder.append(start_running_snapshot_action()); // Copy and modify the disk volume construction request to point to the new // running snapshot - builder.append(Node::action( - "created_volume", - "CreateVolumeRecord", - CREATE_VOLUME_RECORD.as_ref(), - )); - - builder.append(Node::action( - "finalized_snapshot", - "FinalizeSnapshotRecord", - FINALIZE_SNAPSHOT_RECORD.as_ref(), - )); + builder.append(create_volume_record_action()); + builder.append(finalize_snapshot_record_action()); Ok(builder.build()?) } diff --git a/nexus/src/app/sagas/volume_delete.rs b/nexus/src/app/sagas/volume_delete.rs index cb29972885f..e454ae919a6 100644 --- a/nexus/src/app/sagas/volume_delete.rs +++ b/nexus/src/app/sagas/volume_delete.rs @@ -28,16 +28,12 @@ use super::common_storage::delete_crucible_snapshots; use super::ActionRegistry; use super::NexusActionContext; use super::NexusSaga; -use crate::app::sagas::NexusAction; +use crate::app::sagas::declare_saga_actions; use crate::db::datastore::CrucibleResources; -use lazy_static::lazy_static; use nexus_types::identity::Asset; use serde::Deserialize; use serde::Serialize; -use std::sync::Arc; -use steno::new_action_noop_undo; use steno::ActionError; -use steno::Node; use uuid::Uuid; // volume delete saga: input parameters @@ -49,7 +45,8 @@ pub struct Params { // volume delete saga: actions -lazy_static! { +declare_saga_actions! { + volume_delete; // TODO(https://github.com/oxidecomputer/omicron/issues/612): // // We need a way to deal with this operation failing, aside from @@ -57,31 +54,21 @@ lazy_static! { // // What if the Sled goes offline? Nexus must ultimately be // responsible for reconciling this scenario. - - static ref DECREASE_CRUCIBLE_RESOURCE_COUNT: NexusAction = new_action_noop_undo( - "volume-delete.decrease-resource-count", - svd_decrease_crucible_resource_count, - ); - - static ref DELETE_CRUCIBLE_REGIONS: NexusAction = new_action_noop_undo( - "volume-delete.delete-crucible-regions", - svd_delete_crucible_regions, - ); - - static ref DELETE_CRUCIBLE_SNAPSHOTS: NexusAction = new_action_noop_undo( - "volume-delete.delete-crucible-snapshots", - svd_delete_crucible_snapshots, - ); - - static ref DELETE_FREED_CRUCIBLE_REGIONS: NexusAction = new_action_noop_undo( - "volume-delete.delete-freed-crucible-regions", - svd_delete_freed_crucible_regions, - ); - - static ref HARD_DELETE_VOLUME_RECORD: NexusAction = new_action_noop_undo( - "volume-delete.hard-delete-volume-record", - svd_hard_delete_volume_record, - ); + DECREASE_CRUCIBLE_RESOURCE_COUNT -> "crucible_resources_to_delete" { + + svd_decrease_crucible_resource_count + } + DELETE_CRUCIBLE_REGIONS -> "no_result_1" { + + svd_delete_crucible_regions + } + DELETE_CRUCIBLE_SNAPSHOTS -> "no_result_2" { + + svd_delete_crucible_snapshots + } + DELETE_FREED_CRUCIBLE_REGIONS -> "no_result_3" { + + svd_delete_freed_crucible_regions + } + HARD_DELETE_VOLUME_RECORD -> "final_no_result" { + + svd_hard_delete_volume_record + } } // volume delete saga: definition @@ -93,50 +80,23 @@ impl NexusSaga for SagaVolumeDelete { type Params = Params; fn register_actions(registry: &mut ActionRegistry) { - registry.register(Arc::clone(&*DECREASE_CRUCIBLE_RESOURCE_COUNT)); - registry.register(Arc::clone(&*DELETE_CRUCIBLE_REGIONS)); - registry.register(Arc::clone(&*DELETE_CRUCIBLE_SNAPSHOTS)); - registry.register(Arc::clone(&*DELETE_FREED_CRUCIBLE_REGIONS)); - registry.register(Arc::clone(&*HARD_DELETE_VOLUME_RECORD)); + volume_delete_register_actions(registry); } fn make_saga_dag( _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { - builder.append(Node::action( - "crucible_resources_to_delete", - "DecreaseCrucibleResources", - DECREASE_CRUCIBLE_RESOURCE_COUNT.as_ref(), - )); - + builder.append(decrease_crucible_resource_count_action()); builder.append_parallel(vec![ // clean up top level regions for volume - Node::action( - "no_result_1", - "DeleteCrucibleRegions", - DELETE_CRUCIBLE_REGIONS.as_ref(), - ), + delete_crucible_regions_action(), // clean up snapshots no longer referenced by any volume - Node::action( - "no_result_2", - "DeleteCrucibleSnapshots", - DELETE_CRUCIBLE_SNAPSHOTS.as_ref(), - ), + delete_crucible_snapshots_action(), ]); - // clean up regions that were freed by deleting snapshots - builder.append(Node::action( - "no_result_3", - "DeleteFreedCrucibleRegions", - DELETE_FREED_CRUCIBLE_REGIONS.as_ref(), - )); - - builder.append(Node::action( - "final_no_result", - "HardDeleteVolumeRecord", - HARD_DELETE_VOLUME_RECORD.as_ref(), - )); + builder.append(delete_freed_crucible_regions_action()); + builder.append(hard_delete_volume_record_action()); Ok(builder.build()?) } diff --git a/nexus/src/app/sagas/volume_remove_rop.rs b/nexus/src/app/sagas/volume_remove_rop.rs index caaaa302da0..b7eb30cc125 100644 --- a/nexus/src/app/sagas/volume_remove_rop.rs +++ b/nexus/src/app/sagas/volume_remove_rop.rs @@ -4,15 +4,13 @@ use super::{ActionRegistry, NexusActionContext, NexusSaga, SagaInitError}; use crate::app::sagas; -use crate::app::sagas::NexusAction; +use crate::app::sagas::declare_saga_actions; use crate::db; -use lazy_static::lazy_static; use omicron_common::api::external::Error; use serde::Deserialize; use serde::Serialize; use sled_agent_client::types::VolumeConstructionRequest; -use std::sync::Arc; -use steno::{new_action_noop_undo, ActionError, ActionFunc, Node}; +use steno::{ActionError, Node}; use uuid::Uuid; // Volume remove read only parent saga: input parameters @@ -24,7 +22,8 @@ pub struct Params { // Volume remove_read_only_parent saga: actions -lazy_static! { +declare_saga_actions! { + volume_remove_rop; // A read-only parent is a structure in a volume that indicates that the // volume is logically created from this parent. The initial data for the // volume (implicitly) comes from the parent volume. In the background, @@ -43,17 +42,14 @@ lazy_static! { // then delete that temporary volume. // Create the temporary volume - static ref CREATE_TEMP_VOLUME: NexusAction = ActionFunc::new_action( - "volume-remove-rop.create-temp-volume", - svr_create_temp_volume, - svr_create_temp_volume_undo - ); - + CREATE_TEMP_VOLUME -> "temp_volume" { + + svr_create_temp_volume + - svr_create_temp_volume_undo + } // remove the read_only_parent, attach it to the temp volume. - static ref REMOVE_READ_ONLY_PARENT: NexusAction = new_action_noop_undo( - "volume-remove-rop.remove-read-only-parent", - svr_remove_read_only_parent - ); + REMOVE_READ_ONLY_PARENT -> "no_result_1" { + + svr_remove_read_only_parent + } } // volume remove read only parent saga: definition @@ -65,8 +61,7 @@ impl NexusSaga for SagaVolumeRemoveROP { type Params = Params; fn register_actions(registry: &mut ActionRegistry) { - registry.register(Arc::clone(&*CREATE_TEMP_VOLUME)); - registry.register(Arc::clone(&*REMOVE_READ_ONLY_PARENT)); + volume_remove_rop_register_actions(registry); } fn make_saga_dag( @@ -97,18 +92,9 @@ impl NexusSaga for SagaVolumeRemoveROP { )); // Create the temporary volume - builder.append(Node::action( - "temp_volume", - "CreateTempVolume", - CREATE_TEMP_VOLUME.as_ref(), - )); - + builder.append(create_temp_volume_action()); // Remove the read only parent, attach to temp volume - builder.append(Node::action( - "no_result_1", - "RemoveReadOnlyParent", - REMOVE_READ_ONLY_PARENT.as_ref(), - )); + builder.append(remove_read_only_parent_action()); // Build the params for the subsaga to delete the temp volume builder.append(Node::constant( From f399ff7ce3713078e4773853246fc721c09c1a2c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 15 Dec 2022 22:50:41 -0500 Subject: [PATCH 2/8] ignore rustdoc --- nexus/src/app/sagas/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index 3dcf604f025..ece2b8daa3d 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -154,7 +154,7 @@ macro_rules! __action_name { /// /// For this input: /// -/// ``` +/// ```ignore /// declare_saga_actions! { /// my_saga; /// SAGA_NODE1 -> "output1" { From 66cf0c0d800582d601b051c810854e653d7b804a Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 16 Dec 2022 12:15:39 -0500 Subject: [PATCH 3/8] [nexus] Instance Deletion is now a saga --- nexus/authz-macros/src/lib.rs | 2 +- nexus/db-model/src/update_artifact.rs | 4 +- nexus/src/app/instance.rs | 20 +++-- nexus/src/app/sagas/instance_delete.rs | 106 +++++++++++++++++++++++++ nexus/src/app/sagas/mod.rs | 4 + nexus/src/authz/api_resources.rs | 2 +- 6 files changed, 124 insertions(+), 14 deletions(-) create mode 100644 nexus/src/app/sagas/instance_delete.rs diff --git a/nexus/authz-macros/src/lib.rs b/nexus/authz-macros/src/lib.rs index ed5b7f6e2d2..3847f0d8389 100644 --- a/nexus/authz-macros/src/lib.rs +++ b/nexus/authz-macros/src/lib.rs @@ -278,7 +278,7 @@ fn do_authz_resource( Ok(quote! { #[doc = #doc_struct] - #[derive(Clone, Debug)] + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct #resource_name { parent: #parent_resource_name, key: #primary_key_type, diff --git a/nexus/db-model/src/update_artifact.rs b/nexus/db-model/src/update_artifact.rs index 13615e045ec..5170d8e18cb 100644 --- a/nexus/db-model/src/update_artifact.rs +++ b/nexus/db-model/src/update_artifact.rs @@ -7,6 +7,8 @@ use crate::schema::update_available_artifact; use chrono::{DateTime, Utc}; use omicron_common::api::internal; use parse_display::Display; +use serde::Deserialize; +use serde::Serialize; use std::io::Write; impl_enum_wrapper!( @@ -14,7 +16,7 @@ impl_enum_wrapper!( #[diesel(postgres_type(name = "update_artifact_kind"))] pub struct UpdateArtifactKindEnum; - #[derive(Clone, Copy, Debug, Display, AsExpression, FromSqlRow, PartialEq, Eq)] + #[derive(Clone, Copy, Debug, Display, AsExpression, FromSqlRow, PartialEq, Eq, Serialize, Deserialize)] #[display("{0}")] #[diesel(sql_type = UpdateArtifactKindEnum)] pub struct UpdateArtifactKind(pub internal::nexus::UpdateArtifactKind); diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index ec4f0f574ac..b49dc7cf17a 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -265,7 +265,7 @@ impl super::Nexus { // the attached disks do not have any running "upstairs" process running // within the sled. pub async fn project_destroy_instance( - &self, + self: &Arc, opctx: &OpContext, instance_lookup: &lookup::Instance<'_>, ) -> DeleteResult { @@ -275,16 +275,14 @@ impl super::Nexus { let (.., authz_instance) = instance_lookup.lookup_for(authz::Action::Delete).await?; - self.db_datastore - .project_delete_instance(opctx, &authz_instance) - .await?; - self.db_datastore - .instance_delete_all_network_interfaces(opctx, &authz_instance) - .await?; - // Ignore the count of addresses deleted - self.db_datastore - .deallocate_external_ip_by_instance_id(opctx, authz_instance.id()) - .await?; + let saga_params = sagas::instance_delete::Params { + serialized_authn: authn::saga::Serialized::for_opctx(opctx), + authz_instance, + }; + self.execute_saga::( + saga_params, + ) + .await?; Ok(()) } diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs new file mode 100644 index 00000000000..bae309434ca --- /dev/null +++ b/nexus/src/app/sagas/instance_delete.rs @@ -0,0 +1,106 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use super::ActionRegistry; +use super::NexusActionContext; +use super::NexusSaga; +use crate::app::sagas::declare_saga_actions; +use crate::context::OpContext; +use crate::{authn, authz}; +use serde::Deserialize; +use serde::Serialize; +use steno::ActionError; + +// instance delete saga: input parameters + +#[derive(Debug, Deserialize, Serialize)] +pub struct Params { + pub serialized_authn: authn::saga::Serialized, + pub authz_instance: authz::Instance, +} + +// instance delete saga: actions + +declare_saga_actions! { + instance_delete; + INSTANCE_DELETE_RECORD -> "no_result1" { + + sid_delete_instance_record + } + DELETE_NETWORK_INTERFACES -> "no_result2" { + + sid_delete_network_interfaces + } + DEALLOCATE_EXTERNAL_IP -> "no_result3" { + + sid_deallocate_external_ip + } +} + +// instance delete saga: definition + +#[derive(Debug)] +pub struct SagaInstanceDelete; +impl NexusSaga for SagaInstanceDelete { + const NAME: &'static str = "instance-delete"; + type Params = Params; + + fn register_actions(registry: &mut ActionRegistry) { + instance_delete_register_actions(registry); + } + + fn make_saga_dag( + _params: &Self::Params, + mut builder: steno::DagBuilder, + ) -> Result { + builder.append(instance_delete_record_action()); + builder.append(delete_network_interfaces_action()); + builder.append(deallocate_external_ip_action()); + Ok(builder.build()?) + } +} + +// instance delete saga: action implementations + +async fn sid_delete_instance_record( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + osagactx + .datastore() + .project_delete_instance(&opctx, ¶ms.authz_instance) + .await + .map_err(ActionError::action_failed)?; + Ok(()) +} + +async fn sid_delete_network_interfaces( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + osagactx + .datastore() + .instance_delete_all_network_interfaces(&opctx, ¶ms.authz_instance) + .await + .map_err(ActionError::action_failed)?; + Ok(()) +} + +async fn sid_deallocate_external_ip( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + osagactx + .datastore() + .deallocate_external_ip_by_instance_id( + &opctx, + params.authz_instance.id(), + ) + .await + .map_err(ActionError::action_failed)?; + Ok(()) +} diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index ece2b8daa3d..22cadfd9357 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -22,6 +22,7 @@ use uuid::Uuid; pub mod disk_create; pub mod disk_delete; pub mod instance_create; +pub mod instance_delete; pub mod instance_migrate; pub mod snapshot_create; pub mod volume_delete; @@ -95,6 +96,9 @@ fn make_action_registry() -> ActionRegistry { ::register_actions( &mut registry, ); + ::register_actions( + &mut registry, + ); ::register_actions( &mut registry, ); diff --git a/nexus/src/authz/api_resources.rs b/nexus/src/authz/api_resources.rs index 34821072add..b9fba8f9523 100644 --- a/nexus/src/authz/api_resources.rs +++ b/nexus/src/authz/api_resources.rs @@ -150,7 +150,7 @@ impl AuthorizedResource for T { /// This object is used for authorization checks on a Fleet by passing it as the /// `resource` argument to [`crate::context::OpContext::authorize()`]. You /// don't construct a `Fleet` yourself -- use the global [`FLEET`]. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Serialize, Deserialize)] pub struct Fleet; /// Singleton representing the [`Fleet`] itself for authz purposes pub const FLEET: Fleet = Fleet; From 828ed2194cef90f1c2c957a6bec389b7e4de2deb Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 19 Dec 2022 13:36:31 -0500 Subject: [PATCH 4/8] [nexus] Project and VPC creation are now sagas --- nexus/db-model/src/ipv6net.rs | 13 +- nexus/db-model/src/l4_port_range.rs | 6 +- nexus/db-model/src/project.rs | 6 +- nexus/db-model/src/vni.rs | 6 +- nexus/db-model/src/vpc.rs | 13 +- nexus/db-model/src/vpc_firewall_rule.rs | 29 +- nexus/src/app/project.rs | 53 ++-- nexus/src/app/sagas/mod.rs | 6 + nexus/src/app/sagas/project_create.rs | 127 +++++++++ nexus/src/app/sagas/vpc_create.rs | 335 ++++++++++++++++++++++++ nexus/src/app/vpc.rs | 168 ++---------- 11 files changed, 573 insertions(+), 189 deletions(-) create mode 100644 nexus/src/app/sagas/project_create.rs create mode 100644 nexus/src/app/sagas/vpc_create.rs diff --git a/nexus/db-model/src/ipv6net.rs b/nexus/db-model/src/ipv6net.rs index 2bbcb08a4bb..d5eaee1d433 100644 --- a/nexus/db-model/src/ipv6net.rs +++ b/nexus/db-model/src/ipv6net.rs @@ -11,9 +11,20 @@ use ipnetwork::IpNetwork; use omicron_common::api::external; use omicron_common::nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; use rand::{rngs::StdRng, SeedableRng}; +use serde::Deserialize; +use serde::Serialize; use std::net::Ipv6Addr; -#[derive(Clone, Copy, Debug, PartialEq, AsExpression, FromSqlRow)] +#[derive( + Clone, + Copy, + Debug, + PartialEq, + AsExpression, + FromSqlRow, + Serialize, + Deserialize, +)] #[diesel(sql_type = sql_types::Inet)] pub struct Ipv6Net(pub external::Ipv6Net); diff --git a/nexus/db-model/src/l4_port_range.rs b/nexus/db-model/src/l4_port_range.rs index d16d2f8ac5d..40165d7f59e 100644 --- a/nexus/db-model/src/l4_port_range.rs +++ b/nexus/db-model/src/l4_port_range.rs @@ -8,10 +8,14 @@ use diesel::pg::Pg; use diesel::serialize::{self, ToSql}; use diesel::sql_types; use omicron_common::api::external; +use serde::Deserialize; +use serde::Serialize; /// Newtype wrapper around [`external::L4PortRange`] so we can derive /// diesel traits for it -#[derive(Clone, Copy, Debug, AsExpression, FromSqlRow)] +#[derive( + Clone, Copy, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, +)] #[diesel(sql_type = sql_types::Text)] #[repr(transparent)] pub struct L4PortRange(pub external::L4PortRange); diff --git a/nexus/db-model/src/project.rs b/nexus/db-model/src/project.rs index ccfacee44bd..9154915d4ad 100644 --- a/nexus/db-model/src/project.rs +++ b/nexus/db-model/src/project.rs @@ -9,10 +9,14 @@ use db_macros::Resource; use nexus_types::external_api::params; use nexus_types::external_api::views; use nexus_types::identity::Resource; +use serde::Deserialize; +use serde::Serialize; use uuid::Uuid; /// Describes a project within the database. -#[derive(Selectable, Queryable, Insertable, Debug, Resource)] +#[derive( + Selectable, Queryable, Insertable, Debug, Resource, Serialize, Deserialize, +)] #[diesel(table_name = project)] pub struct Project { #[diesel(embed)] diff --git a/nexus/db-model/src/vni.rs b/nexus/db-model/src/vni.rs index 73750d9c31f..36fd42d3e62 100644 --- a/nexus/db-model/src/vni.rs +++ b/nexus/db-model/src/vni.rs @@ -11,8 +11,12 @@ use diesel::serialize; use diesel::serialize::ToSql; use diesel::sql_types; use omicron_common::api::external; +use serde::Deserialize; +use serde::Serialize; -#[derive(Clone, Debug, Copy, AsExpression, FromSqlRow)] +#[derive( + Clone, Debug, Copy, AsExpression, FromSqlRow, Serialize, Deserialize, +)] #[diesel(sql_type = sql_types::Int4)] pub struct Vni(pub external::Vni); diff --git a/nexus/db-model/src/vpc.rs b/nexus/db-model/src/vpc.rs index 0ea38a03d34..8a4dc0e3493 100644 --- a/nexus/db-model/src/vpc.rs +++ b/nexus/db-model/src/vpc.rs @@ -14,9 +14,20 @@ use nexus_types::external_api::params; use nexus_types::external_api::views; use nexus_types::identity::Resource; use omicron_common::api::external; +use serde::Deserialize; +use serde::Serialize; use uuid::Uuid; -#[derive(Queryable, Insertable, Clone, Debug, Selectable, Resource)] +#[derive( + Queryable, + Insertable, + Clone, + Debug, + Selectable, + Resource, + Serialize, + Deserialize, +)] #[diesel(table_name = vpc)] pub struct Vpc { #[diesel(embed)] diff --git a/nexus/db-model/src/vpc_firewall_rule.rs b/nexus/db-model/src/vpc_firewall_rule.rs index d4228e24e12..e3df683e4ab 100644 --- a/nexus/db-model/src/vpc_firewall_rule.rs +++ b/nexus/db-model/src/vpc_firewall_rule.rs @@ -12,6 +12,8 @@ use diesel::serialize::{self, ToSql}; use diesel::sql_types; use nexus_types::identity::Resource; use omicron_common::api::external; +use serde::Deserialize; +use serde::Serialize; use std::io::Write; use uuid::Uuid; @@ -20,7 +22,7 @@ impl_enum_wrapper!( #[diesel(postgres_type(name = "vpc_firewall_rule_status"))] pub struct VpcFirewallRuleStatusEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] #[diesel(sql_type = VpcFirewallRuleStatusEnum)] pub struct VpcFirewallRuleStatus(pub external::VpcFirewallRuleStatus); @@ -35,7 +37,7 @@ impl_enum_wrapper!( #[diesel(postgres_type(name = "vpc_firewall_rule_direction"))] pub struct VpcFirewallRuleDirectionEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] #[diesel(sql_type = VpcFirewallRuleDirectionEnum)] pub struct VpcFirewallRuleDirection(pub external::VpcFirewallRuleDirection); @@ -50,7 +52,7 @@ impl_enum_wrapper!( #[diesel(postgres_type(name = "vpc_firewall_rule_action"))] pub struct VpcFirewallRuleActionEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] #[diesel(sql_type = VpcFirewallRuleActionEnum)] pub struct VpcFirewallRuleAction(pub external::VpcFirewallRuleAction); @@ -65,7 +67,7 @@ impl_enum_wrapper!( #[diesel(postgres_type(name = "vpc_firewall_rule_protocol"))] pub struct VpcFirewallRuleProtocolEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] #[diesel(sql_type = VpcFirewallRuleProtocolEnum)] pub struct VpcFirewallRuleProtocol(pub external::VpcFirewallRuleProtocol); @@ -78,7 +80,7 @@ NewtypeDeref! { () pub struct VpcFirewallRuleProtocol(external::VpcFirewallRuleP /// Newtype wrapper around [`external::VpcFirewallRuleTarget`] so we can derive /// diesel traits for it -#[derive(Clone, Debug, AsExpression, FromSqlRow)] +#[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] #[diesel(sql_type = sql_types::Text)] #[repr(transparent)] pub struct VpcFirewallRuleTarget(pub external::VpcFirewallRuleTarget); @@ -113,7 +115,7 @@ where /// Newtype wrapper around [`external::VpcFirewallRuleHostFilter`] so we can derive /// diesel traits for it -#[derive(Clone, Debug, AsExpression, FromSqlRow)] +#[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize)] #[diesel(sql_type = sql_types::Text)] #[repr(transparent)] pub struct VpcFirewallRuleHostFilter(pub external::VpcFirewallRuleHostFilter); @@ -148,7 +150,9 @@ where /// Newtype wrapper around [`external::VpcFirewallRulePriority`] so we can derive /// diesel traits for it -#[derive(Clone, Copy, Debug, AsExpression, FromSqlRow)] +#[derive( + Clone, Copy, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, +)] #[repr(transparent)] #[diesel(sql_type = sql_types::Int4)] pub struct VpcFirewallRulePriority(pub external::VpcFirewallRulePriority); @@ -180,7 +184,16 @@ where } } -#[derive(Queryable, Insertable, Clone, Debug, Selectable, Resource)] +#[derive( + Queryable, + Insertable, + Clone, + Debug, + Selectable, + Resource, + Serialize, + Deserialize, +)] #[diesel(table_name = vpc_firewall_rule)] pub struct VpcFirewallRule { #[diesel(embed)] diff --git a/nexus/src/app/project.rs b/nexus/src/app/project.rs index 07c4c73ceec..8722ad4973e 100644 --- a/nexus/src/app/project.rs +++ b/nexus/src/app/project.rs @@ -4,6 +4,8 @@ //! Project APIs, contained within organizations +use crate::app::sagas; +use crate::authn; use crate::authz; use crate::context::OpContext; use crate::db; @@ -13,17 +15,17 @@ use crate::db::model::Name; use crate::external_api::params; use crate::external_api::shared; use anyhow::Context; -use nexus_defaults as defaults; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; -use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::NameOrId; use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; +use std::sync::Arc; use uuid::Uuid; impl super::Nexus { @@ -69,7 +71,7 @@ impl super::Nexus { } } pub async fn project_create( - &self, + self: &Arc, opctx: &OpContext, organization_name: &Name, new_project: ¶ms::ProjectCreate, @@ -79,42 +81,21 @@ impl super::Nexus { .lookup_for(authz::Action::CreateChild) .await?; - // Create a project. - let db_project = - db::model::Project::new(authz_org.id(), new_project.clone()); - let db_project = self - .db_datastore - .project_create(opctx, &authz_org, db_project) - .await?; - - // TODO: We probably want to have "project creation" and "default VPC - // creation" co-located within a saga for atomicity. - // - // Until then, we just perform the operations sequentially. - - // Create a default VPC associated with the project. - // TODO-correctness We need to be using the project_id we just created. - // project_create() should return authz::Project and we should use that - // here. - let _ = self - .project_create_vpc( - opctx, - &organization_name, - &new_project.identity.name.clone().into(), - ¶ms::VpcCreate { - identity: IdentityMetadataCreateParams { - name: "default".parse().unwrap(), - description: "Default VPC".to_string(), - }, - ipv6_prefix: Some(defaults::random_vpc_ipv6_prefix()?), - // TODO-robustness this will need to be None if we decide to - // handle the logic around name and dns_name by making - // dns_name optional - dns_name: "default".parse().unwrap(), - }, + let saga_params = sagas::project_create::Params { + serialized_authn: authn::saga::Serialized::for_opctx(opctx), + project_create: new_project.clone(), + authz_org, + }; + let saga_outputs = self + .execute_saga::( + saga_params, ) .await?; + let db_project = saga_outputs + .lookup_node_output::("project") + .map_err(|e| Error::internal_error(&format!("{:#}", &e))) + .internal_context("looking up output from project create saga")?; Ok(db_project) } diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index 22cadfd9357..e13a1bf974b 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -24,9 +24,11 @@ pub mod disk_delete; pub mod instance_create; pub mod instance_delete; pub mod instance_migrate; +pub mod project_create; pub mod snapshot_create; pub mod volume_delete; pub mod volume_remove_rop; +pub mod vpc_create; pub mod common_storage; @@ -102,6 +104,9 @@ fn make_action_registry() -> ActionRegistry { ::register_actions( &mut registry, ); + ::register_actions( + &mut registry, + ); ::register_actions( &mut registry, ); @@ -111,6 +116,7 @@ fn make_action_registry() -> ActionRegistry { ::register_actions( &mut registry, ); + ::register_actions(&mut registry); registry } diff --git a/nexus/src/app/sagas/project_create.rs b/nexus/src/app/sagas/project_create.rs new file mode 100644 index 00000000000..85383b761d0 --- /dev/null +++ b/nexus/src/app/sagas/project_create.rs @@ -0,0 +1,127 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use super::ActionRegistry; +use super::NexusActionContext; +use super::NexusSaga; +use crate::app::sagas; +use crate::app::sagas::declare_saga_actions; +use crate::context::OpContext; +use crate::db::lookup::LookupPath; +use crate::external_api::params; +use crate::{authn, authz, db}; +use nexus_defaults as defaults; +use nexus_types::identity::Resource; +use omicron_common::api::external::IdentityMetadataCreateParams; +use serde::Deserialize; +use serde::Serialize; +use steno::ActionError; + +// project create saga: input parameters + +#[derive(Debug, Deserialize, Serialize)] +pub struct Params { + pub serialized_authn: authn::saga::Serialized, + pub project_create: params::ProjectCreate, + pub authz_org: authz::Organization, +} + +// project create saga: actions + +declare_saga_actions! { + project_create; + PROJECT_CREATE_RECORD -> "project" { + + spc_create_record + } + PROJECT_CREATE_VPC_PARAMS -> "vpc_create_params" { + + spc_create_vpc_params + } +} + +// project create saga: definition + +#[derive(Debug)] +pub struct SagaProjectCreate; +impl NexusSaga for SagaProjectCreate { + const NAME: &'static str = "project-create"; + type Params = Params; + + fn register_actions(registry: &mut ActionRegistry) { + project_create_register_actions(registry); + } + + fn make_saga_dag( + _params: &Self::Params, + mut builder: steno::DagBuilder, + ) -> Result { + builder.append(project_create_record_action()); + builder.append(project_create_vpc_params_action()); + + let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( + sagas::vpc_create::SagaVpcCreate::NAME, + )); + builder.append(steno::Node::subsaga( + "vpc", + sagas::vpc_create::create_dag(subsaga_builder)?, + "vpc_create_params", + )); + Ok(builder.build()?) + } +} + +// project create saga: action implementations + +async fn spc_create_record( + sagactx: NexusActionContext, +) -> Result { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + + let db_project = + db::model::Project::new(params.authz_org.id(), params.project_create); + osagactx + .datastore() + .project_create(&opctx, ¶ms.authz_org, db_project) + .await + .map_err(ActionError::action_failed) +} + +async fn spc_create_vpc_params( + sagactx: NexusActionContext, +) -> Result { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + + let project_id = sagactx.lookup::("project")?.id(); + let ipv6_prefix = Some( + defaults::random_vpc_ipv6_prefix() + .map_err(ActionError::action_failed)?, + ); + + let (.., authz_project) = LookupPath::new(&opctx, osagactx.datastore()) + .project_id(project_id) + .lookup_for(authz::Action::CreateChild) + .await + .map_err(ActionError::action_failed)?; + + let vpc_create = params::VpcCreate { + identity: IdentityMetadataCreateParams { + name: "default".parse().unwrap(), + description: "Default VPC".to_string(), + }, + ipv6_prefix, + // TODO-robustness this will need to be None if we decide to + // handle the logic around name and dns_name by making + // dns_name optional + dns_name: "default".parse().unwrap(), + }; + let saga_params = sagas::vpc_create::Params { + serialized_authn: authn::saga::Serialized::for_opctx(&opctx), + vpc_create, + authz_project, + }; + Ok(saga_params) +} diff --git a/nexus/src/app/sagas/vpc_create.rs b/nexus/src/app/sagas/vpc_create.rs new file mode 100644 index 00000000000..854cc8cfb37 --- /dev/null +++ b/nexus/src/app/sagas/vpc_create.rs @@ -0,0 +1,335 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use super::ActionRegistry; +use super::NexusActionContext; +use super::NexusSaga; +use super::ACTION_GENERATE_ID; +use crate::app::sagas::declare_saga_actions; +use crate::context::OpContext; +use crate::db::model::VpcRouterKind; +use crate::db::queries::vpc_subnet::SubnetError; +use crate::external_api::params; +use crate::{authn, authz, db}; +use nexus_defaults as defaults; +use omicron_common::api::external; +use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::RouteDestination; +use omicron_common::api::external::RouteTarget; +use omicron_common::api::external::RouterRouteCreateParams; +use omicron_common::api::external::RouterRouteKind; +use serde::Deserialize; +use serde::Serialize; +use steno::ActionError; +use steno::Node; +use uuid::Uuid; + +// vpc create saga: input parameters + +#[derive(Debug, Deserialize, Serialize)] +pub struct Params { + pub serialized_authn: authn::saga::Serialized, + pub vpc_create: params::VpcCreate, + pub authz_project: authz::Project, +} + +// vpc create saga: actions + +declare_saga_actions! { + vpc_create; + VPC_CREATE_VPC -> "vpc" { + + svc_create_vpc + } + VPC_CREATE_ROUTER -> "router" { + + svc_create_router + } + VPC_CREATE_ROUTE -> "route" { + + svc_create_route + } + VPC_CREATE_SUBNET -> "subnet" { + + svc_create_subnet + } + VPC_UPDATE_FIREWALL -> "firewall" { + + svc_update_firewall + } + VPC_NOTIFY_SLEDS -> "no_result" { + + svc_notify_sleds + } +} + +// vpc create saga: definition + +/// Identical to [SagaVpcCreate::make_saga_dag], but using types +/// to identify that parameters do not need to be supplied as input. +pub fn create_dag( + mut builder: steno::DagBuilder, +) -> Result { + builder.append(Node::action( + "vpc_id", + "GenerateVpcId", + ACTION_GENERATE_ID.as_ref(), + )); + builder.append(Node::action( + "system_router_id", + "GenerateSystemRouterId", + ACTION_GENERATE_ID.as_ref(), + )); + builder.append(Node::action( + "default_route_id", + "GenerateDefaultRouteId", + ACTION_GENERATE_ID.as_ref(), + )); + builder.append(Node::action( + "default_subnet_id", + "GenerateDefaultSubnetId", + ACTION_GENERATE_ID.as_ref(), + )); + builder.append(vpc_create_vpc_action()); + builder.append(vpc_create_router_action()); + builder.append(vpc_create_route_action()); + builder.append(vpc_create_subnet_action()); + builder.append(vpc_update_firewall_action()); + builder.append(vpc_notify_sleds_action()); + + Ok(builder.build()?) +} + +#[derive(Debug)] +pub struct SagaVpcCreate; +impl NexusSaga for SagaVpcCreate { + const NAME: &'static str = "vpc-create"; + type Params = Params; + + fn register_actions(registry: &mut ActionRegistry) { + vpc_create_register_actions(registry); + } + + fn make_saga_dag( + _params: &Self::Params, + builder: steno::DagBuilder, + ) -> Result { + create_dag(builder) + } +} + +// vpc create saga: action implementations + +async fn svc_create_vpc( + sagactx: NexusActionContext, +) -> Result<(authz::Vpc, db::model::Vpc), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let vpc_id = sagactx.lookup::("vpc_id")?; + let system_router_id = sagactx.lookup::("system_router_id")?; + + // TODO: This is both fake and utter nonsense. It should be eventually + // replaced with the proper behavior for creating the default route + // which may not even happen here. Creating the vpc, its system router, + // and that routers default route should all be a part of the same + // transaction. + let vpc = db::model::IncompleteVpc::new( + vpc_id, + params.authz_project.id(), + system_router_id, + params.vpc_create.clone(), + ) + .map_err(ActionError::action_failed)?; + let (authz_vpc, db_vpc) = osagactx + .datastore() + .project_create_vpc(&opctx, ¶ms.authz_project, vpc) + .await + .map_err(ActionError::action_failed)?; + Ok((authz_vpc, db_vpc)) +} + +async fn svc_create_router( + sagactx: NexusActionContext, +) -> Result { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let vpc_id = sagactx.lookup::("vpc_id")?; + let system_router_id = sagactx.lookup::("system_router_id")?; + let (authz_vpc, _) = + sagactx.lookup::<(authz::Vpc, db::model::Vpc)>("vpc")?; + + // TODO: Ultimately when the VPC is created a system router w/ an + // appropriate setup should also be created. Given that the underlying + // systems aren't wired up yet this is a naive implementation to + // populate the database with a starting router. + let router = db::model::VpcRouter::new( + system_router_id, + vpc_id, + VpcRouterKind::System, + params::VpcRouterCreate { + identity: IdentityMetadataCreateParams { + name: "system".parse().unwrap(), + description: "Routes are automatically added to this \ + router as vpc subnets are created" + .into(), + }, + }, + ); + let (authz_router, _) = osagactx + .datastore() + .vpc_create_router(&opctx, &authz_vpc, router) + .await + .map_err(ActionError::action_failed)?; + Ok(authz_router) +} + +async fn svc_create_route( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let default_route_id = sagactx.lookup::("default_route_id")?; + let system_router_id = sagactx.lookup::("system_router_id")?; + let authz_router = sagactx.lookup::("router")?; + + let route = db::model::RouterRoute::new( + default_route_id, + system_router_id, + RouterRouteKind::Default, + RouterRouteCreateParams { + identity: IdentityMetadataCreateParams { + name: "default".parse().unwrap(), + description: "The default route of a vpc".to_string(), + }, + target: RouteTarget::InternetGateway("outbound".parse().unwrap()), + destination: RouteDestination::Vpc( + params.vpc_create.identity.name.clone(), + ), + }, + ); + + osagactx + .datastore() + .router_create_route(&opctx, &authz_router, route) + .await + .map_err(ActionError::action_failed)?; + Ok(()) +} + +async fn svc_create_subnet( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + + let vpc_id = sagactx.lookup::("vpc_id")?; + let (authz_vpc, db_vpc) = + sagactx.lookup::<(authz::Vpc, db::model::Vpc)>("vpc")?; + let default_subnet_id = sagactx.lookup::("default_subnet_id")?; + + // Allocate the first /64 sub-range from the requested or created + // prefix. + let ipv6_block = external::Ipv6Net( + ipnetwork::Ipv6Network::new(db_vpc.ipv6_prefix.network(), 64) + .map_err(|_| { + external::Error::internal_error( + "Failed to allocate default IPv6 subnet", + ) + }) + .map_err(ActionError::action_failed)?, + ); + + let subnet = db::model::VpcSubnet::new( + default_subnet_id, + vpc_id, + IdentityMetadataCreateParams { + name: "default".parse().unwrap(), + description: format!( + "The default subnet for {}", + params.vpc_create.identity.name + ), + }, + *defaults::DEFAULT_VPC_SUBNET_IPV4_BLOCK, + ipv6_block, + ); + + // Create the subnet record in the database. Overlapping IP ranges + // should be translated into an internal error. That implies that + // there's already an existing VPC Subnet, but we're explicitly creating + // the _first_ VPC in the project. Something is wrong, and likely a bug + // in our code. + osagactx + .datastore() + .vpc_create_subnet(&opctx, &authz_vpc, subnet) + .await + .map_err(|err| match err { + SubnetError::OverlappingIpRange(ip) => { + let ipv4_block = &defaults::DEFAULT_VPC_SUBNET_IPV4_BLOCK; + let log = sagactx.user_data().log(); + error!( + log, + concat!( + "failed to create default VPC Subnet, IP address ", + "range '{}' overlaps with existing", + ), + ip; + "vpc_id" => ?vpc_id, + "subnet_id" => ?default_subnet_id, + "ipv4_block" => ?**ipv4_block, + "ipv6_block" => ?ipv6_block, + ); + external::Error::internal_error( + "Failed to create default VPC Subnet, \ + found overlapping IP address ranges", + ) + } + SubnetError::External(e) => e, + }) + .map_err(ActionError::action_failed)?; + + Ok(()) +} + +async fn svc_update_firewall( + sagactx: NexusActionContext, +) -> Result, ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + + let (authz_vpc, _) = + sagactx.lookup::<(authz::Vpc, db::model::Vpc)>("vpc")?; + let rules = osagactx + .nexus() + .default_firewall_rules_for_vpc( + authz_vpc.id(), + params.vpc_create.identity.name.clone().into(), + ) + .await + .map_err(ActionError::action_failed)?; + osagactx + .datastore() + .vpc_update_firewall_rules(&opctx, &authz_vpc, rules.clone()) + .await + .map_err(ActionError::action_failed)?; + + Ok(rules) +} + +async fn svc_notify_sleds( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let (_, db_vpc) = sagactx.lookup::<(authz::Vpc, db::model::Vpc)>("vpc")?; + let rules = + sagactx.lookup::>("firewall")?; + + osagactx + .nexus() + .send_sled_agents_firewall_rules(&opctx, &db_vpc, &rules) + .await + .map_err(ActionError::action_failed)?; + + Ok(()) +} diff --git a/nexus/src/app/vpc.rs b/nexus/src/app/vpc.rs index 121e73d05e3..82982aa4e03 100644 --- a/nexus/src/app/vpc.rs +++ b/nexus/src/app/vpc.rs @@ -4,6 +4,8 @@ //! VPCs and firewall rules +use crate::app::sagas; +use crate::authn; use crate::authz; use crate::context::OpContext; use crate::db; @@ -11,8 +13,6 @@ use crate::db::identity::Asset; use crate::db::identity::Resource; use crate::db::lookup::LookupPath; use crate::db::model::Name; -use crate::db::model::VpcRouterKind; -use crate::db::queries::vpc_subnet::SubnetError; use crate::external_api::params; use nexus_defaults as defaults; use omicron_common::api::external; @@ -20,14 +20,10 @@ use omicron_common::api::external::CreateResult; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; -use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; -use omicron_common::api::external::RouteDestination; -use omicron_common::api::external::RouteTarget; -use omicron_common::api::external::RouterRouteCreateParams; -use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use sled_agent_client::types::IpNet; @@ -37,13 +33,13 @@ use futures::future::join_all; use ipnetwork::IpNetwork; use std::collections::{HashMap, HashSet}; use std::net::IpAddr; +use std::sync::Arc; use uuid::Uuid; impl super::Nexus { // VPCs - pub async fn project_create_vpc( - &self, + self: &Arc, opctx: &OpContext, organization_name: &Name, project_name: &Name, @@ -54,140 +50,32 @@ impl super::Nexus { .project_name(project_name) .lookup_for(authz::Action::CreateChild) .await?; - let vpc_id = Uuid::new_v4(); - let system_router_id = Uuid::new_v4(); - let default_route_id = Uuid::new_v4(); - let default_subnet_id = Uuid::new_v4(); - - // TODO: This is both fake and utter nonsense. It should be eventually - // replaced with the proper behavior for creating the default route - // which may not even happen here. Creating the vpc, its system router, - // and that routers default route should all be a part of the same - // transaction. - let vpc = db::model::IncompleteVpc::new( - vpc_id, - authz_project.id(), - system_router_id, - params.clone(), - )?; - let (authz_vpc, db_vpc) = self - .db_datastore - .project_create_vpc(opctx, &authz_project, vpc) - .await?; - - // TODO: Ultimately when the VPC is created a system router w/ an - // appropriate setup should also be created. Given that the underlying - // systems aren't wired up yet this is a naive implementation to - // populate the database with a starting router. Eventually this code - // should be replaced with a saga that'll handle creating the VPC and - // its underlying system - let router = db::model::VpcRouter::new( - system_router_id, - vpc_id, - VpcRouterKind::System, - params::VpcRouterCreate { - identity: IdentityMetadataCreateParams { - name: "system".parse().unwrap(), - description: "Routes are automatically added to this \ - router as vpc subnets are created" - .into(), - }, - }, - ); - let (authz_router, _) = self - .db_datastore - .vpc_create_router(&opctx, &authz_vpc, router) - .await?; - let route = db::model::RouterRoute::new( - default_route_id, - system_router_id, - RouterRouteKind::Default, - RouterRouteCreateParams { - identity: IdentityMetadataCreateParams { - name: "default".parse().unwrap(), - description: "The default route of a vpc".to_string(), - }, - target: RouteTarget::InternetGateway( - "outbound".parse().unwrap(), - ), - destination: RouteDestination::Vpc( - params.identity.name.clone(), - ), - }, - ); - - self.db_datastore - .router_create_route(opctx, &authz_router, route) - .await?; - // Allocate the first /64 sub-range from the requested or created - // prefix. - let ipv6_block = external::Ipv6Net( - ipnetwork::Ipv6Network::new(db_vpc.ipv6_prefix.network(), 64) - .map_err(|_| { - external::Error::internal_error( - "Failed to allocate default IPv6 subnet", - ) - })?, - ); + self.project_create_vpc_by_authz(opctx, &authz_project, params).await + } - // TODO: batch this up with everything above - let subnet = db::model::VpcSubnet::new( - default_subnet_id, - vpc_id, - IdentityMetadataCreateParams { - name: "default".parse().unwrap(), - description: format!( - "The default subnet for {}", - params.identity.name - ), - }, - *defaults::DEFAULT_VPC_SUBNET_IPV4_BLOCK, - ipv6_block, - ); + pub async fn project_create_vpc_by_authz( + self: &Arc, + opctx: &OpContext, + authz_project: &authz::Project, + params: ¶ms::VpcCreate, + ) -> CreateResult { + opctx.authorize(authz::Action::CreateChild, authz_project).await?; - // Create the subnet record in the database. Overlapping IP ranges - // should be translated into an internal error. That implies that - // there's already an existing VPC Subnet, but we're explicitly creating - // the _first_ VPC in the project. Something is wrong, and likely a bug - // in our code. - self.db_datastore - .vpc_create_subnet(opctx, &authz_vpc, subnet) - .await - .map_err(|err| match err { - SubnetError::OverlappingIpRange(ip) => { - let ipv4_block = &defaults::DEFAULT_VPC_SUBNET_IPV4_BLOCK; - error!( - self.log, - concat!( - "failed to create default VPC Subnet, IP address ", - "range '{}' overlaps with existing", - ), - ip; - "vpc_id" => ?vpc_id, - "subnet_id" => ?default_subnet_id, - "ipv4_block" => ?**ipv4_block, - "ipv6_block" => ?ipv6_block, - ); - external::Error::internal_error( - "Failed to create default VPC Subnet, \ - found overlapping IP address ranges", - ) - } - SubnetError::External(e) => e, - })?; + let saga_params = sagas::vpc_create::Params { + serialized_authn: authn::saga::Serialized::for_opctx(opctx), + vpc_create: params.clone(), + authz_project: authz_project.clone(), + }; - // Save and send the default firewall rules for the new VPC. - let rules = self - .default_firewall_rules_for_vpc( - authz_vpc.id(), - params.identity.name.clone().into(), - ) + let saga_outputs = self + .execute_saga::(saga_params) .await?; - self.db_datastore - .vpc_update_firewall_rules(opctx, &authz_vpc, rules.clone()) - .await?; - self.send_sled_agents_firewall_rules(opctx, &db_vpc, &rules).await?; + + let (_, db_vpc) = saga_outputs + .lookup_node_output::<(authz::Vpc, db::model::Vpc)>("vpc") + .map_err(|e| Error::internal_error(&format!("{:#}", &e))) + .internal_context("looking up output from VPC create saga")?; Ok(db_vpc) } @@ -352,7 +240,7 @@ impl super::Nexus { /// Customize the default firewall rules for a particular VPC /// by replacing the name `default` with the VPC's actual name. - async fn default_firewall_rules_for_vpc( + pub(crate) async fn default_firewall_rules_for_vpc( &self, vpc_id: Uuid, vpc_name: Name, @@ -395,7 +283,7 @@ impl super::Nexus { Ok(rules) } - async fn send_sled_agents_firewall_rules( + pub(crate) async fn send_sled_agents_firewall_rules( &self, opctx: &OpContext, vpc: &db::model::Vpc, From 6abce0865dfb225b3581f05df7a544b2561168f8 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 19 Dec 2022 19:24:14 -0500 Subject: [PATCH 5/8] Move some lazy_static to once_cell --- Cargo.lock | 1 + nexus/Cargo.toml | 1 + nexus/src/app/sagas/mod.rs | 35 ++++++++++++++++++----------------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3289d308691..b82dfde07f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3452,6 +3452,7 @@ dependencies = [ "omicron-rpaths", "omicron-sled-agent", "omicron-test-utils", + "once_cell", "openapi-lint", "openapiv3", "openssl", diff --git a/nexus/Cargo.toml b/nexus/Cargo.toml index d400ce4a9b2..8a4cf107f5f 100644 --- a/nexus/Cargo.toml +++ b/nexus/Cargo.toml @@ -37,6 +37,7 @@ newtype_derive = "0.1.6" # integration tests. nexus-test-interface = { path = "test-interface" } num-integer = "0.1.45" +once_cell = "1.16.0" # must match samael's crate! openssl = "0.10" openssl-sys = "0.9" diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index ece2b8daa3d..36556ad053d 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -10,7 +10,7 @@ // easier it will be to test, version, and update in deployed systems. use crate::saga_interface::SagaContext; -use lazy_static::lazy_static; +use once_cell::sync::Lazy; use std::sync::Arc; use steno::new_action_noop_undo; use steno::ActionContext; @@ -79,12 +79,11 @@ impl From for omicron_common::api::external::Error { } } -lazy_static! { - pub(super) static ref ACTION_GENERATE_ID: NexusAction = - new_action_noop_undo("common.uuid_generate", saga_generate_uuid); - pub static ref ACTION_REGISTRY: Arc = - Arc::new(make_action_registry()); -} +pub(super) static ACTION_GENERATE_ID: Lazy = Lazy::new(|| { + new_action_noop_undo("common.uuid_generate", saga_generate_uuid) +}); +pub static ACTION_REGISTRY: Lazy> = + Lazy::new(|| Arc::new(make_action_registry())); fn make_action_registry() -> ActionRegistry { let mut registry = steno::ActionRegistry::new(); @@ -203,21 +202,23 @@ macro_rules! declare_saga_actions { // Basically, everything to the left of "<>" is just us propagating state // through the macro, and everything to the right of it is user input. (S = $saga:ident $($nodes:ident),* <> $node:ident -> $out:literal { + $a:ident - $u:ident } $($tail:tt)*) => { - lazy_static::lazy_static! { - static ref $node: crate::app::sagas::NexusAction = ::steno::ActionFunc::new_action( - crate::app::sagas::__action_name!($saga, $node), $a, $u, - ); - } + static $node: ::once_cell::sync::Lazy = + ::once_cell::sync::Lazy::new(|| { + ::steno::ActionFunc::new_action( + crate::app::sagas::__action_name!($saga, $node), $a, $u, + ) + }); crate::app::sagas::__emit_action!($node, $out); declare_saga_actions!(S = $saga $($nodes,)* $node <> $($tail)*); }; // Same as the prior match, but without the undo action. (S = $saga:ident $($nodes:ident),* <> $node:ident -> $out:literal { + $a:ident } $($tail:tt)*) => { - lazy_static::lazy_static! { - static ref $node: crate::app::sagas::NexusAction = ::steno::new_action_noop_undo( - crate::app::sagas::__action_name!($saga, $node), $a, - ); - } + static $node: ::once_cell::sync::Lazy = + ::once_cell::sync::Lazy::new(|| { + ::steno::new_action_noop_undo( + crate::app::sagas::__action_name!($saga, $node), $a, + ) + }); crate::app::sagas::__emit_action!($node, $out); declare_saga_actions!(S = $saga $($nodes,)* $node <> $($tail)*); }; From 29a3ab5a00001777ef2a1a738c81a549051fb37b Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 20 Dec 2022 09:54:22 -0500 Subject: [PATCH 6/8] Extend docs --- nexus/src/app/sagas/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index 36556ad053d..f4a3da50df5 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -151,6 +151,13 @@ macro_rules! __action_name { /// A macro intended to reduce boilerplate when writing saga actions. /// +/// This macro aims to reduce this boilerplate, by requiring only the following: +/// - The name of the saga +/// - The name of each action +/// - The output of each action +/// - The "forward" action function +/// - (Optional) The "undo" action function +/// /// For this input: /// /// ```ignore From 2f89973dea2c8fb81c841e7c715743487ae0b105 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 21 Dec 2022 09:49:42 -0500 Subject: [PATCH 7/8] [vpc_create] Add undo actions --- nexus/db-model/src/ipv4net.rs | 13 +++++- nexus/db-model/src/vpc_subnet.rs | 13 +++++- nexus/src/app/sagas/vpc_create.rs | 76 +++++++++++++++++++++++++++---- nexus/src/app/vpc_subnet.rs | 3 +- nexus/src/db/datastore/vpc.rs | 12 ++++- 5 files changed, 103 insertions(+), 14 deletions(-) diff --git a/nexus/db-model/src/ipv4net.rs b/nexus/db-model/src/ipv4net.rs index c5b45e4064f..8c868b3b647 100644 --- a/nexus/db-model/src/ipv4net.rs +++ b/nexus/db-model/src/ipv4net.rs @@ -10,9 +10,20 @@ use diesel::sql_types; use ipnetwork::IpNetwork; use omicron_common::api::external; use omicron_common::nexus_config::NUM_INITIAL_RESERVED_IP_ADDRESSES; +use serde::Deserialize; +use serde::Serialize; use std::net::Ipv4Addr; -#[derive(Clone, Copy, Debug, PartialEq, AsExpression, FromSqlRow)] +#[derive( + Clone, + Copy, + Debug, + PartialEq, + AsExpression, + FromSqlRow, + Serialize, + Deserialize, +)] #[diesel(sql_type = sql_types::Inet)] pub struct Ipv4Net(pub external::Ipv4Net); diff --git a/nexus/db-model/src/vpc_subnet.rs b/nexus/db-model/src/vpc_subnet.rs index a572f6f535e..2cc74c177bf 100644 --- a/nexus/db-model/src/vpc_subnet.rs +++ b/nexus/db-model/src/vpc_subnet.rs @@ -14,10 +14,21 @@ use nexus_types::external_api::params; use nexus_types::external_api::views; use nexus_types::identity::Resource; use omicron_common::api::external; +use serde::Deserialize; +use serde::Serialize; use std::net::IpAddr; use uuid::Uuid; -#[derive(Queryable, Insertable, Clone, Debug, Selectable, Resource)] +#[derive( + Queryable, + Insertable, + Clone, + Debug, + Selectable, + Resource, + Serialize, + Deserialize, +)] #[diesel(table_name = vpc_subnet)] pub struct VpcSubnet { #[diesel(embed)] diff --git a/nexus/src/app/sagas/vpc_create.rs b/nexus/src/app/sagas/vpc_create.rs index 854cc8cfb37..9c5b668c5be 100644 --- a/nexus/src/app/sagas/vpc_create.rs +++ b/nexus/src/app/sagas/vpc_create.rs @@ -8,13 +8,13 @@ use super::NexusSaga; use super::ACTION_GENERATE_ID; use crate::app::sagas::declare_saga_actions; use crate::context::OpContext; -use crate::db::model::VpcRouterKind; use crate::db::queries::vpc_subnet::SubnetError; use crate::external_api::params; use crate::{authn, authz, db}; use nexus_defaults as defaults; use omicron_common::api::external; use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::LookupType; use omicron_common::api::external::RouteDestination; use omicron_common::api::external::RouteTarget; use omicron_common::api::external::RouterRouteCreateParams; @@ -40,15 +40,19 @@ declare_saga_actions! { vpc_create; VPC_CREATE_VPC -> "vpc" { + svc_create_vpc + - svc_create_vpc_undo } VPC_CREATE_ROUTER -> "router" { + svc_create_router + - svc_create_router_undo } VPC_CREATE_ROUTE -> "route" { + svc_create_route + - svc_create_route_undo } VPC_CREATE_SUBNET -> "subnet" { + svc_create_subnet + - svc_create_subnet_undo } VPC_UPDATE_FIREWALL -> "firewall" { + svc_update_firewall @@ -124,11 +128,6 @@ async fn svc_create_vpc( let vpc_id = sagactx.lookup::("vpc_id")?; let system_router_id = sagactx.lookup::("system_router_id")?; - // TODO: This is both fake and utter nonsense. It should be eventually - // replaced with the proper behavior for creating the default route - // which may not even happen here. Creating the vpc, its system router, - // and that routers default route should all be a part of the same - // transaction. let vpc = db::model::IncompleteVpc::new( vpc_id, params.authz_project.id(), @@ -144,6 +143,21 @@ async fn svc_create_vpc( Ok((authz_vpc, db_vpc)) } +async fn svc_create_vpc_undo( + sagactx: NexusActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let (authz_vpc, db_vpc) = + sagactx.lookup::<(authz::Vpc, db::model::Vpc)>("vpc")?; + osagactx + .datastore() + .project_delete_vpc(&opctx, &db_vpc, &authz_vpc) + .await?; + Ok(()) +} + async fn svc_create_router( sagactx: NexusActionContext, ) -> Result { @@ -162,7 +176,7 @@ async fn svc_create_router( let router = db::model::VpcRouter::new( system_router_id, vpc_id, - VpcRouterKind::System, + db::model::VpcRouterKind::System, params::VpcRouterCreate { identity: IdentityMetadataCreateParams { name: "system".parse().unwrap(), @@ -180,6 +194,18 @@ async fn svc_create_router( Ok(authz_router) } +async fn svc_create_router_undo( + sagactx: NexusActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let authz_router = sagactx.lookup::("router")?; + + osagactx.datastore().vpc_delete_router(&opctx, &authz_router).await?; + Ok(()) +} + async fn svc_create_route( sagactx: NexusActionContext, ) -> Result<(), ActionError> { @@ -214,9 +240,26 @@ async fn svc_create_route( Ok(()) } +async fn svc_create_route_undo( + sagactx: NexusActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let authz_router = sagactx.lookup::("router")?; + let route_id = sagactx.lookup::("default_route_id")?; + let authz_route = authz::RouterRoute::new( + authz_router, + route_id, + LookupType::ById(route_id), + ); + osagactx.datastore().router_delete_route(&opctx, &authz_route).await?; + Ok(()) +} + async fn svc_create_subnet( sagactx: NexusActionContext, -) -> Result<(), ActionError> { +) -> Result<(authz::VpcSubnet, db::model::VpcSubnet), ActionError> { let osagactx = sagactx.user_data(); let params = sagactx.saga_params::()?; let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); @@ -284,8 +327,23 @@ async fn svc_create_subnet( } SubnetError::External(e) => e, }) - .map_err(ActionError::action_failed)?; + .map_err(ActionError::action_failed) +} + +async fn svc_create_subnet_undo( + sagactx: NexusActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + + let (authz_subnet, db_subnet) = + sagactx.lookup::<(authz::VpcSubnet, db::model::VpcSubnet)>("subnet")?; + osagactx + .datastore() + .vpc_delete_subnet(&opctx, &db_subnet, &authz_subnet) + .await?; Ok(()) } diff --git a/nexus/src/app/vpc_subnet.rs b/nexus/src/app/vpc_subnet.rs index 144e8abf793..7e5d9840de3 100644 --- a/nexus/src/app/vpc_subnet.rs +++ b/nexus/src/app/vpc_subnet.rs @@ -155,7 +155,7 @@ impl super::Nexus { Err(result.unwrap_err().into_external()) } Err(SubnetError::External(e)) => Err(e), - Ok(subnet) => Ok(subnet), + Ok((.., subnet)) => Ok(subnet), } } Some(ipv6_block) => { @@ -178,6 +178,7 @@ impl super::Nexus { self.db_datastore .vpc_create_subnet(opctx, &authz_vpc, subnet) .await + .map(|(.., subnet)| subnet) .map_err(SubnetError::into_external) } } diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 181743e3383..0c439fffb5e 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -379,14 +379,22 @@ impl DataStore { opctx: &OpContext, authz_vpc: &authz::Vpc, subnet: VpcSubnet, - ) -> Result { + ) -> Result<(authz::VpcSubnet, VpcSubnet), SubnetError> { opctx .authorize(authz::Action::CreateChild, authz_vpc) .await .map_err(SubnetError::External)?; assert_eq!(authz_vpc.id(), subnet.vpc_id); - self.vpc_create_subnet_raw(subnet).await + let db_subnet = self.vpc_create_subnet_raw(subnet).await?; + Ok(( + authz::VpcSubnet::new( + authz_vpc.clone(), + db_subnet.id(), + LookupType::ById(db_subnet.id()), + ), + db_subnet, + )) } pub(crate) async fn vpc_create_subnet_raw( From b9fef96af9424f1bf96fcac29cebccce3ebf79a5 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 21 Dec 2022 15:13:10 -0500 Subject: [PATCH 8/8] [vpc_create] Add tests for idempotency, fix things --- nexus/src/app/sagas/vpc_create.rs | 336 ++++++++++++++++++++++++++++++ nexus/src/db/datastore/vpc.rs | 4 + 2 files changed, 340 insertions(+) diff --git a/nexus/src/app/sagas/vpc_create.rs b/nexus/src/app/sagas/vpc_create.rs index 9c5b668c5be..5a03b4627bf 100644 --- a/nexus/src/app/sagas/vpc_create.rs +++ b/nexus/src/app/sagas/vpc_create.rs @@ -56,6 +56,7 @@ declare_saga_actions! { } VPC_UPDATE_FIREWALL -> "firewall" { + svc_update_firewall + - svc_update_firewall_undo } VPC_NOTIFY_SLEDS -> "no_result" { + svc_notify_sleds @@ -373,6 +374,21 @@ async fn svc_update_firewall( Ok(rules) } +async fn svc_update_firewall_undo( + sagactx: NexusActionContext, +) -> Result<(), anyhow::Error> { + let osagactx = sagactx.user_data(); + let params = sagactx.saga_params::()?; + let opctx = OpContext::for_saga_action(&sagactx, ¶ms.serialized_authn); + let (authz_vpc, _) = + sagactx.lookup::<(authz::Vpc, db::model::Vpc)>("vpc")?; + osagactx + .datastore() + .vpc_update_firewall_rules(&opctx, &authz_vpc, vec![]) + .await?; + Ok(()) +} + async fn svc_notify_sleds( sagactx: NexusActionContext, ) -> Result<(), ActionError> { @@ -391,3 +407,323 @@ async fn svc_notify_sleds( Ok(()) } + +#[cfg(test)] +mod test { + use crate::{ + app::saga::create_saga_dag, app::sagas::vpc_create::Params, + app::sagas::vpc_create::SagaVpcCreate, authn::saga::Serialized, authz, + context::OpContext, db::datastore::DataStore, db::lookup::LookupPath, + external_api::params, + }; + use async_bb8_diesel::{AsyncRunQueryDsl, OptionalExtension}; + use diesel::{ExpressionMethods, QueryDsl, SelectableHelper}; + use dropshot::test_util::ClientTestContext; + use nexus_test_utils::resource_helpers::create_organization; + use nexus_test_utils::resource_helpers::create_project; + use nexus_test_utils::resource_helpers::populate_ip_pool; + use nexus_test_utils_macros::nexus_test; + use omicron_common::api::external::IdentityMetadataCreateParams; + use omicron_common::api::external::Name; + use omicron_common::api::external::NameOrId; + use uuid::Uuid; + + type ControlPlaneTestContext = + nexus_test_utils::ControlPlaneTestContext; + + const ORG_NAME: &str = "test-org"; + const PROJECT_NAME: &str = "springfield-squidport"; + + async fn create_org_and_project(client: &ClientTestContext) -> Uuid { + populate_ip_pool(&client, "default", None).await; + create_organization(&client, ORG_NAME).await; + let project = create_project(client, ORG_NAME, PROJECT_NAME).await; + project.identity.id + } + + // Helper for creating VPC create parameters + fn new_test_params( + opctx: &OpContext, + authz_project: authz::Project, + ) -> Params { + Params { + serialized_authn: Serialized::for_opctx(opctx), + vpc_create: params::VpcCreate { + identity: IdentityMetadataCreateParams { + name: "my-vpc".parse().unwrap(), + description: "My VPC".to_string(), + }, + ipv6_prefix: None, + dns_name: "abc".parse().unwrap(), + }, + authz_project, + } + } + + fn test_opctx(cptestctx: &ControlPlaneTestContext) -> OpContext { + OpContext::for_tests( + cptestctx.logctx.log.new(o!()), + cptestctx.server.apictx.nexus.datastore().clone(), + ) + } + + async fn get_authz_project( + cptestctx: &ControlPlaneTestContext, + project_id: Uuid, + action: authz::Action, + ) -> authz::Project { + let nexus = &cptestctx.server.apictx.nexus; + let project_selector = + params::ProjectSelector::new(None, NameOrId::Id(project_id)); + let opctx = test_opctx(&cptestctx); + let (.., authz_project) = nexus + .project_lookup(&opctx, &project_selector) + .expect("Invalid parameters constructing project lookup") + .lookup_for(action) + .await + .expect("Project does not exist"); + authz_project + } + + async fn delete_project_vpc_defaults( + cptestctx: &ControlPlaneTestContext, + project_id: Uuid, + ) { + let opctx = test_opctx(&cptestctx); + let datastore = cptestctx.server.apictx.nexus.datastore(); + let default_name = Name::try_from("default".to_string()).unwrap(); + let system_name = Name::try_from("system".to_string()).unwrap(); + + // Default Subnet + let (.., authz_subnet, subnet) = LookupPath::new(&opctx, &datastore) + .project_id(project_id) + .vpc_name(&default_name.clone().into()) + .vpc_subnet_name(&default_name.clone().into()) + .fetch() + .await + .expect("Failed to fetch default Subnet"); + datastore + .vpc_delete_subnet(&opctx, &subnet, &authz_subnet) + .await + .expect("Failed to delete default Subnet"); + + // Default route + let (.., authz_route, _route) = LookupPath::new(&opctx, &datastore) + .project_id(project_id) + .vpc_name(&default_name.clone().into()) + .vpc_router_name(&system_name.clone().into()) + .router_route_name(&default_name.clone().into()) + .fetch() + .await + .expect("Failed to fetch default route"); + datastore + .router_delete_route(&opctx, &authz_route) + .await + .expect("Failed to delete default route"); + + // System router + let (.., authz_router, _router) = LookupPath::new(&opctx, &datastore) + .project_id(project_id) + .vpc_name(&default_name.clone().into()) + .vpc_router_name(&system_name.into()) + .fetch() + .await + .expect("Failed to fetch system router"); + datastore + .vpc_delete_router(&opctx, &authz_router) + .await + .expect("Failed to delete system router"); + + // Default VPC & Firewall Rules + let (.., authz_vpc, vpc) = LookupPath::new(&opctx, &datastore) + .project_id(project_id) + .vpc_name(&default_name.into()) + .fetch() + .await + .expect("Failed to fetch default VPC"); + datastore + .vpc_delete_all_firewall_rules(&opctx, &authz_vpc) + .await + .expect("Failed to delete all firewall rules for VPC"); + datastore + .project_delete_vpc(&opctx, &vpc, &authz_vpc) + .await + .expect("Failed to delete VPC"); + } + + async fn verify_clean_slate(datastore: &DataStore) { + assert!(no_vpcs_exist(datastore).await); + assert!(no_routers_exist(datastore).await); + assert!(no_routes_exist(datastore).await); + assert!(no_subnets_exist(datastore).await); + assert!(no_firewall_rules_exist(datastore).await); + } + + async fn no_vpcs_exist(datastore: &DataStore) -> bool { + use crate::db::model::Vpc; + use crate::db::schema::vpc::dsl; + + dsl::vpc + .filter(dsl::time_deleted.is_null()) + .select(Vpc::as_select()) + .first_async::(datastore.pool_for_tests().await.unwrap()) + .await + .optional() + .unwrap() + .map(|vpc| { + eprintln!("VPC exists: {vpc:?}"); + }) + .is_none() + } + + async fn no_routers_exist(datastore: &DataStore) -> bool { + use crate::db::model::VpcRouter; + use crate::db::schema::vpc_router::dsl; + + dsl::vpc_router + .filter(dsl::time_deleted.is_null()) + .select(VpcRouter::as_select()) + .first_async::(datastore.pool_for_tests().await.unwrap()) + .await + .optional() + .unwrap() + .map(|router| { + eprintln!("Router exists: {router:?}"); + }) + .is_none() + } + + async fn no_routes_exist(datastore: &DataStore) -> bool { + use crate::db::model::RouterRoute; + use crate::db::schema::router_route::dsl; + + dsl::router_route + .filter(dsl::time_deleted.is_null()) + .select(RouterRoute::as_select()) + .first_async::( + datastore.pool_for_tests().await.unwrap(), + ) + .await + .optional() + .unwrap() + .map(|route| { + eprintln!("Route exists: {route:?}"); + }) + .is_none() + } + + async fn no_subnets_exist(datastore: &DataStore) -> bool { + use crate::db::model::VpcSubnet; + use crate::db::schema::vpc_subnet::dsl; + + dsl::vpc_subnet + .filter(dsl::time_deleted.is_null()) + .select(VpcSubnet::as_select()) + .first_async::(datastore.pool_for_tests().await.unwrap()) + .await + .optional() + .unwrap() + .map(|subnet| { + eprintln!("Subnet exists: {subnet:?}"); + }) + .is_none() + } + + async fn no_firewall_rules_exist(datastore: &DataStore) -> bool { + use crate::db::model::VpcFirewallRule; + use crate::db::schema::vpc_firewall_rule::dsl; + + dsl::vpc_firewall_rule + .filter(dsl::time_deleted.is_null()) + .select(VpcFirewallRule::as_select()) + .first_async::( + datastore.pool_for_tests().await.unwrap(), + ) + .await + .optional() + .unwrap() + .map(|rule| { + eprintln!("Firewall rule exists: {rule:?}"); + }) + .is_none() + } + + #[nexus_test(server = crate::Server)] + async fn test_saga_basic_usage_succeeds( + cptestctx: &ControlPlaneTestContext, + ) { + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + let project_id = create_org_and_project(&client).await; + delete_project_vpc_defaults(&cptestctx, project_id).await; + + // Before running the test, confirm we have no records of any VPCs. + verify_clean_slate(nexus.datastore()).await; + + // Build the saga DAG with the provided test parameters + let opctx = test_opctx(&cptestctx); + let authz_project = get_authz_project( + &cptestctx, + project_id, + authz::Action::CreateChild, + ) + .await; + let params = new_test_params(&opctx, authz_project); + let dag = create_saga_dag::(params).unwrap(); + let runnable_saga = nexus.create_runnable_saga(dag).await.unwrap(); + + // Actually run the saga + nexus.run_saga(runnable_saga).await.unwrap(); + } + + #[nexus_test(server = crate::Server)] + async fn test_action_failure_can_unwind( + cptestctx: &ControlPlaneTestContext, + ) { + let log = &cptestctx.logctx.log; + + let client = &cptestctx.external_client; + let nexus = &cptestctx.server.apictx.nexus; + let project_id = create_org_and_project(&client).await; + delete_project_vpc_defaults(&cptestctx, project_id).await; + + // Build the saga DAG with the provided test parameters + let opctx = test_opctx(&cptestctx); + let authz_project = get_authz_project( + &cptestctx, + project_id, + authz::Action::CreateChild, + ) + .await; + let params = new_test_params(&opctx, authz_project); + let dag = create_saga_dag::(params).unwrap(); + + for node in dag.get_nodes() { + // Create a new saga for this node. + info!( + log, + "Creating new saga which will fail at index {:?}", node.index(); + "node_name" => node.name().as_ref(), + "label" => node.label(), + ); + + let runnable_saga = + nexus.create_runnable_saga(dag.clone()).await.unwrap(); + + // Inject an error instead of running the node. + // + // This should cause the saga to unwind. + nexus + .sec() + .saga_inject_error(runnable_saga.id(), node.index()) + .await + .unwrap(); + nexus + .run_saga(runnable_saga) + .await + .expect_err("Saga should have failed"); + + verify_clean_slate(nexus.datastore()).await; + } + } +} diff --git a/nexus/src/db/datastore/vpc.rs b/nexus/src/db/datastore/vpc.rs index 0c439fffb5e..ba951d7730d 100644 --- a/nexus/src/db/datastore/vpc.rs +++ b/nexus/src/db/datastore/vpc.rs @@ -283,6 +283,7 @@ impl DataStore { .filter(dsl::vpc_id.eq(authz_vpc.id())) .set(dsl::time_deleted.eq(now)); + let rules_is_empty = rules.is_empty(); let insert_new_query = Vpc::insert_resource( authz_vpc.id(), diesel::insert_into(dsl::vpc_firewall_rule).values(rules), @@ -306,6 +307,9 @@ impl DataStore { // The generation count update on the vpc table row will take a // write lock on the row, ensuring that the vpc was not deleted // concurently. + if rules_is_empty { + return Ok(vec![]); + } insert_new_query .insert_and_get_results_async(&conn) .await