From ec21731b0047c42a948bdd5df8b8288479367244 Mon Sep 17 00:00:00 2001 From: Hrudaya Date: Thu, 27 Jul 2023 09:25:33 +0000 Subject: [PATCH 1/2] feat(clone): add bdd case for snapshots and clone with io-engine-test framework Signed-off-by: Hrudaya --- io-engine-tests/src/lib.rs | 1 + io-engine-tests/src/replica.rs | 1 + io-engine-tests/src/snapshot.rs | 199 +++++++++++++++++++++++++++++++ io-engine/tests/snapshot_lvol.rs | 89 ++++++++++++++ 4 files changed, 290 insertions(+) create mode 100644 io-engine-tests/src/snapshot.rs diff --git a/io-engine-tests/src/lib.rs b/io-engine-tests/src/lib.rs index 50f9f08f9..9912d7692 100644 --- a/io-engine-tests/src/lib.rs +++ b/io-engine-tests/src/lib.rs @@ -32,6 +32,7 @@ pub mod nvme; pub mod nvmf; pub mod pool; pub mod replica; +pub mod snapshot; pub use compose::MayastorTest; diff --git a/io-engine-tests/src/replica.rs b/io-engine-tests/src/replica.rs index 997bd21dc..6b22d806d 100644 --- a/io-engine-tests/src/replica.rs +++ b/io-engine-tests/src/replica.rs @@ -14,6 +14,7 @@ use mayastor_api::v1::replica::{ ReplicaType, ShareReplicaRequest, }; + use tonic::{Code, Status}; #[derive(Clone)] diff --git a/io-engine-tests/src/snapshot.rs b/io-engine-tests/src/snapshot.rs new file mode 100644 index 000000000..f8ae8f294 --- /dev/null +++ b/io-engine-tests/src/snapshot.rs @@ -0,0 +1,199 @@ +use super::{compose::rpc::v1::SharedRpcHandle, generate_uuid}; + +use mayastor_api::v1::snapshot::{ + CreateReplicaSnapshotRequest, + CreateReplicaSnapshotResponse, + CreateSnapshotCloneRequest, + ListSnapshotCloneRequest, + // ListSnapshotCloneResponse, + ListSnapshotsRequest, + // ListSnapshotsResponse, + Replica, + SnapshotInfo, + SnapshotQueryType, +}; +use tonic::Status; + +pub struct ReplicaSnapshotBuilder { + pub rpc: SharedRpcHandle, + pub replica_uuid: Option, + pub snapshot_uuid: Option, + pub snapshot_name: Option, + pub entity_id: Option, + pub txn_id: Option, +} +impl ReplicaSnapshotBuilder { + pub fn new(rpc: SharedRpcHandle) -> Self { + Self { + rpc, + replica_uuid: None, + snapshot_uuid: None, + snapshot_name: None, + entity_id: None, + txn_id: None, + } + } + pub fn with_replica_uuid(mut self, replica_uuid: &str) -> Self { + self.replica_uuid = Some(replica_uuid.to_owned()); + self + } + pub fn with_snapshot_uuid(mut self) -> Self { + self.snapshot_uuid = Some(generate_uuid()); + self + } + pub fn with_snapshot_name(mut self, snap_name: &str) -> Self { + self.snapshot_name = Some(snap_name.to_owned()); + self + } + pub fn with_entity_id(mut self, entity_id: &str) -> Self { + self.entity_id = Some(entity_id.to_owned()); + self + } + pub fn with_txn_id(mut self, txn_id: &str) -> Self { + self.txn_id = Some(txn_id.to_owned()); + self + } + pub fn snapshot_uuid(&self) -> String { + self.snapshot_uuid + .as_ref() + .expect("Snapshot UUID must be set") + .clone() + } + pub fn replica_uuid(&self) -> String { + self.replica_uuid + .as_ref() + .expect("Replica UUID must be set") + .clone() + } + pub fn snapshot_name(&self) -> String { + self.snapshot_name + .as_ref() + .expect("Snapshot name must be set") + .clone() + } + pub fn rpc(&self) -> SharedRpcHandle { + self.rpc.clone() + } + pub async fn create_replica_snapshot( + &mut self, + ) -> Result { + self.rpc() + .lock() + .await + .snapshot + .create_replica_snapshot(CreateReplicaSnapshotRequest { + replica_uuid: self.replica_uuid(), + snapshot_uuid: self.snapshot_uuid(), + snapshot_name: self.snapshot_name(), + entity_id: self.entity_id.as_ref().unwrap().to_string(), + txn_id: self.txn_id.as_ref().unwrap().to_string(), + }) + .await + .map(|r| r.into_inner()) + } + pub async fn get_snapshots(&self) -> Result, Status> { + Ok(list_snapshot(self.rpc()) + .await + .expect("List Snapshot Failed") + .into_iter() + .filter(|s| s.source_uuid == self.replica_uuid()) + .collect::>()) + } +} +pub async fn list_snapshot( + rpc: SharedRpcHandle, +) -> Result, Status> { + rpc.lock() + .await + .snapshot + .list_snapshot(ListSnapshotsRequest { + source_uuid: None, + snapshot_uuid: None, + snapshot_query_type: SnapshotQueryType::AllSnapshots as i32, + }) + .await + .map(|r| r.into_inner().snapshots) +} + +pub struct SnapshotCloneBuilder { + pub rpc: SharedRpcHandle, + pub snapshot_uuid: Option, + pub clone_name: Option, + pub clone_uuid: Option, +} +impl SnapshotCloneBuilder { + pub fn new(rpc: SharedRpcHandle) -> Self { + Self { + rpc, + snapshot_uuid: None, + clone_name: None, + clone_uuid: None, + } + } + pub fn with_snapshot_uuid(mut self, snapshot_uuid: &str) -> Self { + self.snapshot_uuid = Some(snapshot_uuid.to_owned()); + self + } + pub fn with_clone_name(mut self, clone_name: &str) -> Self { + self.clone_name = Some(clone_name.to_owned()); + self + } + pub fn with_clone_uuid(mut self, clone_uuid: &str) -> Self { + self.clone_uuid = Some(clone_uuid.to_owned()); + self + } + pub fn rpc(&self) -> SharedRpcHandle { + self.rpc.clone() + } + pub fn snapshot_uuid(&self) -> String { + self.snapshot_uuid + .as_ref() + .expect("snapshot_uuid must be set") + .clone() + } + pub fn clone_name(&self) -> String { + self.clone_name + .as_ref() + .expect("clone_name must be set") + .clone() + } + pub fn clone_uuid(&self) -> String { + self.clone_uuid + .as_ref() + .expect("clone_uuid must be set") + .clone() + } + pub async fn create_snapshot_clone(&mut self) -> Result { + self.rpc() + .lock() + .await + .snapshot + .create_snapshot_clone(CreateSnapshotCloneRequest { + snapshot_uuid: self.snapshot_uuid(), + clone_name: self.clone_name(), + clone_uuid: self.clone_uuid(), + }) + .await + .map(|r| r.into_inner()) + } + pub async fn get_clones(&self) -> Result, Status> { + Ok(list_snapshot_clone(self.rpc()) + .await + .expect("List Clone Failed") + .into_iter() + .filter(|s| s.snapshot_uuid == Some(self.snapshot_uuid())) + .collect::>()) + } +} +pub async fn list_snapshot_clone( + rpc: SharedRpcHandle, +) -> Result, Status> { + rpc.lock() + .await + .snapshot + .list_snapshot_clone(ListSnapshotCloneRequest { + snapshot_uuid: None, + }) + .await + .map(|r| r.into_inner().replicas) +} diff --git a/io-engine/tests/snapshot_lvol.rs b/io-engine/tests/snapshot_lvol.rs index fd81ef9f4..b6e026060 100755 --- a/io-engine/tests/snapshot_lvol.rs +++ b/io-engine/tests/snapshot_lvol.rs @@ -1,5 +1,18 @@ pub mod common; +use common::{ + compose::{rpc::v1::GrpcConnect, Binary, Builder}, + pool::PoolBuilder, + replica::ReplicaBuilder, + snapshot::ReplicaSnapshotBuilder, +}; +use io_engine_tests::{ + file_io::DataSize, + nvmf::test_write_to_nvmf, + replica::validate_replicas, + snapshot::SnapshotCloneBuilder, +}; + use once_cell::sync::OnceCell; use common::{bdev_io, compose::MayastorTest}; @@ -1384,3 +1397,79 @@ async fn test_delete_snapshot_with_valid_clone_fail_1() { }) .await; } + +#[tokio::test] +async fn test_snapshot_verify_restore_data() { + common::composer_init(); + + let test = Builder::new() + .name("cargo-test") + .network("10.1.0.0/16") + .unwrap() + .add_container_bin( + "ms", + Binary::from_dbg("io-engine").with_args(vec!["-l", "1,2,3,4"]), + ) + .with_clean(true) + .build() + .await + .unwrap(); + + let conn = GrpcConnect::new(&test); + + let ms = conn.grpc_handle_shared("ms").await.unwrap(); + + const POOL_SIZE: u64 = 200; + const REPL_SIZE: u64 = 40; + + let mut pool = PoolBuilder::new(ms.clone()) + .with_name("pool0") + .with_new_uuid() + .with_malloc("mem0", POOL_SIZE); + let mut repl_1 = ReplicaBuilder::new(ms.clone()) + .with_pool(&pool) + .with_name("repl1") + .with_new_uuid() + .with_size_mb(REPL_SIZE) + .with_thin(false); + // Create pool. + pool.create().await.unwrap(); + // Create replica. + repl_1.create().await.unwrap(); + // Share replica. + repl_1.share().await.unwrap(); + // Write some data to replica. + test_write_to_nvmf( + &repl_1.nvmf_location(), + DataSize::from_bytes(0), + 30, + DataSize::from_mb(1), + ) + .await + .unwrap(); + // Create snapshot for the replica. + let mut snap_1 = ReplicaSnapshotBuilder::new(ms.clone()) + .with_replica_uuid(repl_1.uuid().as_str()) + .with_snapshot_uuid() + .with_snapshot_name("snap1") + .with_entity_id("snap1_e1") + .with_txn_id("snap1-t1"); + snap_1.create_replica_snapshot().await.unwrap(); + + // Create a clone from the replica. + let mut clone_1 = SnapshotCloneBuilder::new(ms.clone()) + .with_snapshot_uuid(snap_1.snapshot_uuid().as_str()) + .with_clone_name("clone1") + .with_clone_uuid(Uuid::new_v4().to_string().as_str()); + clone_1.create_snapshot_clone().await.unwrap(); + + // Create restore object. + let mut restore_1 = ReplicaBuilder::new(ms.clone()) + .with_uuid(clone_1.clone_uuid().as_str()) + .with_name(clone_1.clone_name().as_str()); + + restore_1.share().await.unwrap(); + + // Check the original replica and restore clone is identical. + validate_replicas(&vec![repl_1.clone(), restore_1.clone()]).await; +} From ff2bebf3d496910fd67e5ba81a021d734caf8332 Mon Sep 17 00:00:00 2001 From: Hrudaya Date: Fri, 28 Jul 2023 16:10:00 +0000 Subject: [PATCH 2/2] feat(snapshot/clone): handle different query options for list snapshot and listreplica Signed-off-by: Hrudaya --- .../bin/io-engine-client/v1/snapshot_cli.rs | 4 +- io-engine/src/grpc/v1/replica.rs | 45 +++++++- io-engine/src/grpc/v1/snapshot.rs | 103 +++++++++++++----- io-engine/src/lvs/lvs_lvol.rs | 2 +- io-engine/tests/snapshot_lvol.rs | 2 +- 5 files changed, 122 insertions(+), 34 deletions(-) diff --git a/io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs b/io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs index ac825f8ec..3406f9853 100644 --- a/io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs +++ b/io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs @@ -11,7 +11,6 @@ use colored_json::ToColoredJson; use mayastor_api::v1 as v1_rpc; use snafu::ResultExt; use tonic::Status; -use v1_rpc::snapshot::*; pub async fn handler( ctx: Context, @@ -412,7 +411,8 @@ async fn list(mut ctx: Context, matches: &ArgMatches<'_>) -> crate::Result<()> { let request = v1_rpc::snapshot::ListSnapshotsRequest { source_uuid, snapshot_uuid, - snapshot_query_type: SnapshotQueryType::AllSnapshots as i32, + snapshot_query_type: v1_rpc::snapshot::SnapshotQueryType::AllSnapshots + as i32, }; let response = ctx diff --git a/io-engine/src/grpc/v1/replica.rs b/io-engine/src/grpc/v1/replica.rs index d6fd87c15..22cea8f75 100644 --- a/io-engine/src/grpc/v1/replica.rs +++ b/io-engine/src/grpc/v1/replica.rs @@ -122,7 +122,45 @@ impl ReplicaService { } } } - +fn filter_replicas_by_replica_type( + replicas: Vec, + replica_type: Option, +) -> Vec { + let Some(replica_type) = replica_type else { + return replicas + }; + replicas + .into_iter() + .filter_map(|r| match replica_type { + // AllReplicas + ReplicaType::AllReplicas => Some(r), + // AllReplicasExceptSnapshots + ReplicaType::AllReplicasExceptSnapshots => { + if !r.is_snapshot { + Some(r) + } else { + None + } + } + // OnlySnapshotClones + ReplicaType::OnlySnapshotClones => { + if r.is_clone { + Some(r) + } else { + None + } + } + // OnlyReplicas + ReplicaType::OnlyReplicas => { + if !r.is_snapshot && !r.is_clone { + Some(r) + } else { + None + } + } + }) + .collect() +} #[tonic::async_trait] impl ReplicaRpc for ReplicaService { #[named] @@ -313,7 +351,10 @@ impl ReplicaRpc for ReplicaService { } else if let Some(uuid) = args.uuid { replicas.retain(|r| r.uuid == uuid); } - + let replicas = filter_replicas_by_replica_type( + replicas, + ReplicaType::from_i32(args.replicatype), + ); Ok(ListReplicasResponse { replicas, }) diff --git a/io-engine/src/grpc/v1/snapshot.rs b/io-engine/src/grpc/v1/snapshot.rs index 8509d6ce2..447722bc3 100644 --- a/io-engine/src/grpc/v1/snapshot.rs +++ b/io-engine/src/grpc/v1/snapshot.rs @@ -308,6 +308,54 @@ impl SnapshotService { } } +/// Filter snapshots based on snapshot_query_type came in gRPC request. +fn filter_snapshots_by_snapshot_query_type( + snapshot_list: Vec, + snap_query_type: Option, +) -> Vec { + let Some(snap_query_type) = snap_query_type else { + return snapshot_list + }; + snapshot_list + .into_iter() + .filter_map(|s| match snap_query_type { + // AllSnapshots + SnapshotQueryType::AllSnapshots => Some(s), + // AllSnapshotsExceptDiscardedSnapshots + SnapshotQueryType::AllSnapshotsExceptDiscardedSnapshots => { + if !s.discarded_snapshot { + Some(s) + } else { + None + } + } + // OnlyDiscardedSnapshots + SnapshotQueryType::OnlyDiscardedSnapshots => { + if s.discarded_snapshot { + Some(s) + } else { + None + } + } + // OnlyInvalidSnapshots + SnapshotQueryType::OnlyInvalidSnapshots => { + if !s.valid_snapshot { + Some(s) + } else { + None + } + } + // OnlyUsableSnapshots + SnapshotQueryType::OnlyUsableSnapshots => { + if !s.discarded_snapshot && s.valid_snapshot { + Some(s) + } else { + None + } + } + }) + .collect() +} #[tonic::async_trait] impl SnapshotRpc for SnapshotService { #[named] @@ -458,10 +506,11 @@ impl SnapshotRpc for SnapshotService { let args = request.into_inner(); trace!("{:?}", args); let rx = rpc_submit(async move { + let snapshots: Vec; // if snapshot_uuid is input, get specific snapshot result - if let Some(snapshot_uuid) = args.snapshot_uuid { + if let Some(ref snapshot_uuid) = args.snapshot_uuid { let lvol = match UntypedBdev::lookup_by_uuid_str( - &snapshot_uuid, + snapshot_uuid, ) { Some(bdev) => Lvol::try_from(bdev)?, None => { @@ -473,49 +522,47 @@ impl SnapshotRpc for SnapshotService { }) } }; - let snapshots = lvol + snapshots = lvol .list_snapshot_by_snapshot_uuid() .into_iter() .map(SnapshotInfo::from) .collect(); - Ok(ListSnapshotsResponse { - snapshots, - }) - } else if let Some(replica_uuid) = args.source_uuid { + } else if let Some(ref replica_uuid) = args.source_uuid { // if replica_uuid is valid, filter snapshot based // on source_uuid - let lvol = match UntypedBdev::lookup_by_uuid_str( - &replica_uuid, - ) { - Some(bdev) => Lvol::try_from(bdev)?, - None => { - return Err(LvsError::Invalid { - source: Errno::ENOENT, - msg: format!( - "Replica {replica_uuid} not found", - ), - }) - } - }; - let snapshots = lvol + let lvol = + match UntypedBdev::lookup_by_uuid_str(replica_uuid) + { + Some(bdev) => Lvol::try_from(bdev)?, + None => { + return Err(LvsError::Invalid { + source: Errno::ENOENT, + msg: format!( + "Replica {replica_uuid} not found", + ), + }) + } + }; + snapshots = lvol .list_snapshot_by_source_uuid() .into_iter() .map(SnapshotInfo::from) .collect(); - Ok(ListSnapshotsResponse { - snapshots, - }) } else { // if source_uuid is not input, list all snapshot // present in system - let snapshots = Lvol::list_all_snapshots() + snapshots = Lvol::list_all_snapshots() .into_iter() .map(SnapshotInfo::from) .collect(); - Ok(ListSnapshotsResponse { - snapshots, - }) } + let snapshots = filter_snapshots_by_snapshot_query_type( + snapshots, + SnapshotQueryType::from_i32(args.snapshot_query_type), + ); + Ok(ListSnapshotsResponse { + snapshots, + }) })?; rx.await .map_err(|_| Status::cancelled("cancelled"))? diff --git a/io-engine/src/lvs/lvs_lvol.rs b/io-engine/src/lvs/lvs_lvol.rs index ba957802b..41f604ef5 100644 --- a/io-engine/src/lvs/lvs_lvol.rs +++ b/io-engine/src/lvs/lvs_lvol.rs @@ -982,7 +982,7 @@ impl LvsLvol for Lvol { // If destroy replica is a snapshot clone and it is the last // clone from the snapshot, destroy the snapshot - // if it is already marked as discardedSnapshot. + // if it is already marked as discarded snapshot. if let Some(snapshot_lvol) = snapshot_lvol { if snapshot_lvol.list_clones_by_snapshot_uuid().is_empty() && snapshot_lvol.is_discarded_snapshot() diff --git a/io-engine/tests/snapshot_lvol.rs b/io-engine/tests/snapshot_lvol.rs index b6e026060..3daab8b6c 100755 --- a/io-engine/tests/snapshot_lvol.rs +++ b/io-engine/tests/snapshot_lvol.rs @@ -1408,7 +1408,7 @@ async fn test_snapshot_verify_restore_data() { .unwrap() .add_container_bin( "ms", - Binary::from_dbg("io-engine").with_args(vec!["-l", "1,2,3,4"]), + Binary::from_dbg("io-engine").with_args(vec!["-l", "1"]), ) .with_clean(true) .build()