Skip to content

Commit

Permalink
Implement image deletion (#4296)
Browse files Browse the repository at this point in the history
Also, make sure that multiple volume construction requests can only be
copied for read-only Regions: the `region` table has a column for the
volume that owns it, and this means there's a one to one mapping for the
volume that references the region read-write. Ensure this in
`volume_checkout_randomize_ids` otherwise multiple volumes could
reference the same regions and this could lead to accounting / cleanup
issues.

Fixes #3033
  • Loading branch information
jmpesp authored Oct 19, 2023
1 parent 463cc1a commit b5e62f6
Show file tree
Hide file tree
Showing 8 changed files with 584 additions and 36 deletions.
37 changes: 37 additions & 0 deletions nexus/db-queries/src/db/datastore/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use nexus_db_model::Name;
use nexus_types::identity::Resource;
use omicron_common::api::external::http_pagination::PaginatedBy;
use omicron_common::api::external::CreateResult;
use omicron_common::api::external::DeleteResult;
use omicron_common::api::external::ListResultVec;
use omicron_common::api::external::ResourceType;
use omicron_common::api::external::UpdateResult;
Expand Down Expand Up @@ -232,4 +233,40 @@ impl DataStore {
})?;
Ok(image)
}

pub async fn silo_image_delete(
&self,
opctx: &OpContext,
authz_image: &authz::SiloImage,
image: SiloImage,
) -> DeleteResult {
opctx.authorize(authz::Action::Delete, authz_image).await?;
self.image_delete(opctx, image.into()).await
}

pub async fn project_image_delete(
&self,
opctx: &OpContext,
authz_image: &authz::ProjectImage,
image: ProjectImage,
) -> DeleteResult {
opctx.authorize(authz::Action::Delete, authz_image).await?;
self.image_delete(opctx, image.into()).await
}

async fn image_delete(
&self,
opctx: &OpContext,
image: Image,
) -> DeleteResult {
use db::schema::image::dsl;
diesel::update(dsl::image)
.filter(dsl::id.eq(image.id()))
.set(dsl::time_deleted.eq(Utc::now()))
.execute_async(&*self.pool_connection_authorized(opctx).await?)
.await
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?;

Ok(())
}
}
18 changes: 17 additions & 1 deletion nexus/db-queries/src/db/datastore/volume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::db::model::Dataset;
use crate::db::model::Region;
use crate::db::model::RegionSnapshot;
use crate::db::model::Volume;
use anyhow::bail;
use async_bb8_diesel::AsyncConnection;
use async_bb8_diesel::AsyncRunQueryDsl;
use chrono::Utc;
Expand Down Expand Up @@ -391,6 +392,16 @@ impl DataStore {
opts,
gen,
} => {
if !opts.read_only {
// Only one volume can "own" a Region, and that volume's
// UUID is recorded in the region table accordingly. It is
// an error to make a copy of a volume construction request
// that references non-read-only Regions.
bail!(
"only one Volume can reference a Region non-read-only!"
);
}

let mut opts = opts.clone();
opts.id = Uuid::new_v4();

Expand All @@ -416,7 +427,10 @@ impl DataStore {
/// Checkout a copy of the Volume from the database using `volume_checkout`,
/// then randomize the UUIDs in the construction request. Because this is a
/// new volume, it is immediately passed to `volume_create` so that the
/// accounting for Crucible resources stays correct.
/// accounting for Crucible resources stays correct. This is only valid for
/// Volumes that reference regions read-only - it's important for accounting
/// purposes that each region in this volume construction request is
/// returned by `read_only_resources_associated_with_volume`.
pub async fn volume_checkout_randomize_ids(
&self,
volume_id: Uuid,
Expand Down Expand Up @@ -469,6 +483,8 @@ impl DataStore {
.eq(0)
// Despite the SQL specifying that this column is NOT NULL,
// this null check is required for this function to work!
// It's possible that the left join of region_snapshot above
// could join zero rows, making this null.
.or(dsl::volume_references.is_null()),
)
// where the volume has already been soft-deleted
Expand Down
29 changes: 19 additions & 10 deletions nexus/src/app/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

//! Images (both project and silo scoped)
use super::Unimpl;
use crate::external_api::params;
use nexus_db_queries::authn;
use nexus_db_queries::authz;
use nexus_db_queries::context::OpContext;
use nexus_db_queries::db;
Expand All @@ -27,6 +27,8 @@ use std::str::FromStr;
use std::sync::Arc;
use uuid::Uuid;

use super::sagas;

impl super::Nexus {
pub(crate) async fn image_lookup<'a>(
&'a self,
Expand Down Expand Up @@ -356,26 +358,33 @@ impl super::Nexus {
}
}

// TODO-MVP: Implement
pub(crate) async fn image_delete(
self: &Arc<Self>,
opctx: &OpContext,
image_lookup: &ImageLookup<'_>,
) -> DeleteResult {
match image_lookup {
let image_param: sagas::image_delete::ImageParam = match image_lookup {
ImageLookup::ProjectImage(lookup) => {
lookup.lookup_for(authz::Action::Delete).await?;
let (_, _, authz_image, image) =
lookup.fetch_for(authz::Action::Delete).await?;
sagas::image_delete::ImageParam::Project { authz_image, image }
}
ImageLookup::SiloImage(lookup) => {
lookup.lookup_for(authz::Action::Delete).await?;
let (_, authz_image, image) =
lookup.fetch_for(authz::Action::Delete).await?;
sagas::image_delete::ImageParam::Silo { authz_image, image }
}
};
let error = Error::InternalError {
internal_message: "Endpoint not implemented".to_string(),

let saga_params = sagas::image_delete::Params {
serialized_authn: authn::saga::Serialized::for_opctx(opctx),
image_param,
};
Err(self
.unimplemented_todo(opctx, Unimpl::ProtectedLookup(error))
.await)

self.execute_saga::<sagas::image_delete::SagaImageDelete>(saga_params)
.await?;

Ok(())
}

/// Converts a project scoped image into a silo scoped image
Expand Down
124 changes: 124 additions & 0 deletions nexus/src/app/sagas/image_delete.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// 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, NexusActionContext, NexusSaga};
use crate::app::sagas;
use crate::app::sagas::declare_saga_actions;
use nexus_db_queries::{authn, authz, db};
use serde::Deserialize;
use serde::Serialize;
use steno::ActionError;
use steno::Node;
use uuid::Uuid;

#[derive(Debug, Deserialize, Serialize)]
pub(crate) enum ImageParam {
Project { authz_image: authz::ProjectImage, image: db::model::ProjectImage },

Silo { authz_image: authz::SiloImage, image: db::model::SiloImage },
}

impl ImageParam {
fn volume_id(&self) -> Uuid {
match self {
ImageParam::Project { image, .. } => image.volume_id,

ImageParam::Silo { image, .. } => image.volume_id,
}
}
}

#[derive(Debug, Deserialize, Serialize)]
pub(crate) struct Params {
pub serialized_authn: authn::saga::Serialized,
pub image_param: ImageParam,
}

declare_saga_actions! {
image_delete;
DELETE_IMAGE_RECORD -> "no_result1" {
+ sid_delete_image_record
}
}

#[derive(Debug)]
pub(crate) struct SagaImageDelete;
impl NexusSaga for SagaImageDelete {
const NAME: &'static str = "image-delete";
type Params = Params;

fn register_actions(registry: &mut ActionRegistry) {
image_delete_register_actions(registry);
}

fn make_saga_dag(
params: &Self::Params,
mut builder: steno::DagBuilder,
) -> Result<steno::Dag, super::SagaInitError> {
builder.append(delete_image_record_action());

const DELETE_VOLUME_PARAMS: &'static str = "delete_volume_params";

let volume_delete_params = sagas::volume_delete::Params {
serialized_authn: params.serialized_authn.clone(),
volume_id: params.image_param.volume_id(),
};
builder.append(Node::constant(
DELETE_VOLUME_PARAMS,
serde_json::to_value(&volume_delete_params).map_err(|e| {
super::SagaInitError::SerializeError(
String::from("volume_id"),
e,
)
})?,
));

let make_volume_delete_dag = || {
let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new(
sagas::volume_delete::SagaVolumeDelete::NAME,
));
sagas::volume_delete::create_dag(subsaga_builder)
};
builder.append(steno::Node::subsaga(
"delete_volume",
make_volume_delete_dag()?,
DELETE_VOLUME_PARAMS,
));

Ok(builder.build()?)
}
}

// image delete saga: action implementations

async fn sid_delete_image_record(
sagactx: NexusActionContext,
) -> Result<(), ActionError> {
let osagactx = sagactx.user_data();
let params = sagactx.saga_params::<Params>()?;
let opctx = crate::context::op_context_for_saga_action(
&sagactx,
&params.serialized_authn,
);

match params.image_param {
ImageParam::Project { authz_image, image } => {
osagactx
.datastore()
.project_image_delete(&opctx, &authz_image, image)
.await
.map_err(ActionError::action_failed)?;
}

ImageParam::Silo { authz_image, image } => {
osagactx
.datastore()
.silo_image_delete(&opctx, &authz_image, image)
.await
.map_err(ActionError::action_failed)?;
}
}

Ok(())
}
4 changes: 4 additions & 0 deletions nexus/src/app/sagas/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use uuid::Uuid;
pub mod disk_create;
pub mod disk_delete;
pub mod finalize_disk;
pub mod image_delete;
pub mod import_blocks_from_url;
mod instance_common;
pub mod instance_create;
Expand Down Expand Up @@ -167,6 +168,9 @@ fn make_action_registry() -> ActionRegistry {
&mut registry,
);
<vpc_create::SagaVpcCreate as NexusSaga>::register_actions(&mut registry);
<image_delete::SagaImageDelete as NexusSaga>::register_actions(
&mut registry,
);

#[cfg(test)]
<test_saga::SagaTest as NexusSaga>::register_actions(&mut registry);
Expand Down
Loading

0 comments on commit b5e62f6

Please sign in to comment.