Skip to content

Commit

Permalink
Merge #1312
Browse files Browse the repository at this point in the history 
1312: Idempotency fixes r=tiagolobocastro a=tiagolobocastro

fix(rpc/api): update to latest api

Fixes missing nexus objects from shutdown and fault operation.
Add support for listing by uuid.
Fixes missing pool type check.

Signed-off-by: Tiago Castro <[email protected]>



Co-authored-by: Tiago Castro <[email protected]>
  • Loading branch information
@tiagolobocastro
mayastor-bors and tiagolobocastro committed Feb 6, 2023
2 parents 3a5b1d2 + 93e185d commit b0734db
Show file tree
Hide file tree
Showing 18 changed files with 167 additions and 105 deletions.
1 change: 1 addition & 0 deletions io-engine-tests/src/nexus.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ pub async fn list_nexuses(rpc: SharedRpcHandle) -> Result<Vec<Nexus>, Status> {
.nexus
.list_nexus(ListNexusOptions {
name: None,
uuid: None,
})
.await
.map(|r| r.into_inner().nexus_list)
Expand Down
1 change: 1 addition & 0 deletions io-engine-tests/src/pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ pub async fn list_pools(rpc: SharedRpcHandle) -> Result<Vec<Pool>, Status> {
.list_pools(ListPoolOptions {
name: None,
pooltype: None,
uuid: None,
})
.await
.map(|r| r.into_inner().pools)
Expand Down
1 change: 1 addition & 0 deletions io-engine-tests/src/replica.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ pub async fn list_replicas(
.list_replicas(ListReplicaOptions {
name: None,
poolname: None,
uuid: None,
})
.await
.map(|r| r.into_inner().replicas)
Expand Down
5 changes: 5 additions & 0 deletions io-engine/src/bdev/dev.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ pub fn device_lookup(name: &str) -> Option<Box<dyn BlockDevice>> {
nvmx::lookup_by_name(name).or_else(|| SpdkBlockDevice::lookup_by_name(name))
}

/// Lookup up device name by its uri.
pub fn device_name(uri: &str) -> Result<String, BdevError> {
Ok(uri::parse(uri)?.get_name())
}

pub async fn device_create(uri: &str) -> Result<String, BdevError> {
uri::parse(uri)?.create().await
}
Expand Down
19 changes: 18 additions & 1 deletion io-engine/src/bdev/nexus/nexus_bdev_children.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use super::{
};

use crate::{
bdev::{device_create, device_destroy, device_lookup},
bdev::{dev::device_name, device_create, device_destroy, device_lookup},
bdev_api::BdevError,
core::{
device_cmd_queue,
Expand Down Expand Up @@ -238,6 +238,16 @@ impl<'n> Nexus<'n> {
}
}

/// Checks if the nexus contains the given child uri.
pub fn contains_child_uri(&self, uri: &str) -> bool {
self.children_iter().any(|c| c.uri() == uri)
}
/// Checks if the nexus contains the given child name.
pub fn contains_child_name(&self, name: &str) -> bool {
self.children_iter()
.any(|c| device_name(c.uri()).ok().as_deref() == Some(name))
}

/// Destroy child with given uri.
/// If the child does not exist the method returns success.
pub async fn remove_child(
Expand All @@ -246,6 +256,13 @@ impl<'n> Nexus<'n> {
) -> Result<(), Error> {
info!("{:?}: remove child request: '{}'", self, uri);

if !self.contains_child_uri(uri) {
return Err(Error::ChildNotFound {
child: uri.to_string(),
name: self.name.to_string(),
});
}

self.check_nexus_operation(NexusOperation::ReplicaRemove)?;

if self.child_count() == 1 {
Expand Down
29 changes: 22 additions & 7 deletions io-engine/src/bdev/nexus/nexus_bdev_error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,18 @@ impl From<NvmfError> for Error {
impl From<Error> for tonic::Status {
fn from(e: Error) -> Self {
match e {
Error::NexusNotFound {
..
} => Status::not_found(e.to_string()),
Error::InvalidUuid {
..
} => Status::invalid_argument(e.to_string()),
Error::InvalidKey {
..
} => Status::invalid_argument(e.to_string()),
Error::InvalidShareProtocol {
..
} => Status::invalid_argument(e.to_string()),
Error::InvalidReservation {
..
} => Status::invalid_argument(e.to_string()),
Error::AlreadyShared {
..
} => Status::invalid_argument(e.to_string()),
Expand All @@ -276,24 +279,36 @@ impl From<Error> for tonic::Status {
Error::ChildGeometry {
..
} => Status::invalid_argument(e.to_string()),
Error::ChildTooSmall {
..
} => Status::invalid_argument(e.to_string()),
Error::OpenChild {
..
} => Status::invalid_argument(e.to_string()),
Error::OperationNotAllowed {
..
} => Status::failed_precondition(e.to_string()),
Error::DestroyLastChild {
..
} => Status::failed_precondition(e.to_string()),
Error::DestroyLastHealthyChild {
..
} => Status::failed_precondition(e.to_string()),
Error::ChildNotFound {
..
} => Status::not_found(e.to_string()),
Error::RebuildJobNotFound {
..
} => Status::not_found(e.to_string()),
Error::OperationNotAllowed {
Error::NexusNotFound {
..
} => Status::failed_precondition(e.to_string()),
Error::ChildTooSmall {
} => Status::not_found(e.to_string()),
Error::ChildAlreadyExists {
..
} => Status::invalid_argument(e.to_string()),
} => Status::already_exists(e.to_string()),
Error::NameExists {
..
} => Status::already_exists(e.to_string()),
e => Status::new(Code::Internal, e.verbose()),
}
}
Expand Down
1 change: 1 addition & 0 deletions io-engine/src/bin/io-engine-client/v0/nexus_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,7 @@ async fn nexus_children_2(
.nexus
.list_nexus(v1::nexus::ListNexusOptions {
name: None,
uuid: None,
})
.await
.context(GrpcStatus)?;
Expand Down
3 changes: 3 additions & 0 deletions io-engine/src/bin/io-engine-client/v1/nexus_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ async fn nexus_destroy(
.nexus
.list_nexus(v1::nexus::ListNexusOptions {
name: None,
uuid: None,
})
.await
.context(GrpcStatus)?;
Expand Down Expand Up @@ -464,6 +465,7 @@ async fn nexus_list(
.nexus
.list_nexus(v1::nexus::ListNexusOptions {
name: None,
uuid: None,
})
.await
.context(GrpcStatus)?;
Expand Down Expand Up @@ -540,6 +542,7 @@ async fn nexus_children_2(
.nexus
.list_nexus(v1::nexus::ListNexusOptions {
name: None,
uuid: None,
})
.await
.context(GrpcStatus)?;
Expand Down
1 change: 1 addition & 0 deletions io-engine/src/bin/io-engine-client/v1/pool_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ async fn list(
.list_pools(v1rpc::pool::ListPoolOptions {
name: None,
pooltype: None,
uuid: None,
})
.await
.context(GrpcStatus)?;
Expand Down
66 changes: 12 additions & 54 deletions io-engine/src/bin/io-engine-client/v1/replica_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,54 +59,6 @@ pub fn subcommands<'a, 'b>() -> App<'a, 'b> {
),
);

// let create_v2 = SubCommand::with_name("create2")
// .about("Create replica on pool")
// .arg(
// Arg::with_name("pool")
// .required(true)
// .index(1)
// .help("Storage pool name"))
// .arg(
// Arg::with_name("name")
// .required(true)
// .index(2)
// .help("Replica name"))
// .arg(
// Arg::with_name("uuid")
// .required(true).index(3)
// .help("Unique replica uuid"))
// .arg(
// Arg::with_name("protocol")
// .short("p")
// .long("protocol")
// .takes_value(true)
// .value_name("PROTOCOL")
// .help("Name of a protocol (nvmf) used for sharing the
// replica (default none)")) .arg(
// Arg::with_name("size")
// .short("s")
// .long("size")
// .takes_value(true)
// .required(true)
// .value_name("NUMBER")
// .help("Size of the replica"))
// .arg(
// Arg::with_name("thin")
// .short("t")
// .long("thin")
// .takes_value(false)
// .help("Whether replica is thin provisioned (default
// false)")) .arg(
// Arg::with_name("allowed-host")
// .long("allowed-host")
// .takes_value(true)
// .multiple(true)
// .required(false)
// .help(
// "NQN of hosts which are allowed to connect to the
// target", ),
// );

let destroy = SubCommand::with_name("destroy")
.about("Destroy replica")
.arg(
Expand Down Expand Up @@ -222,13 +174,14 @@ async fn replica_create(
size: size.get_bytes() as u64,
allowed_hosts,
};
//let response = ctx.client.create_replica(rq).await.context(GrpcStatus)?;

let response = ctx
.v1
.replica
.create_replica(request)
.await
.context(GrpcStatus)?;

match ctx.output {
OutputFormat::Json => {
println!(
Expand Down Expand Up @@ -258,18 +211,22 @@ async fn replica_destroy(
})?
.to_owned();

let _ = ctx.v1.replica.destroy_replica(
v1_rpc::replica::DestroyReplicaRequest {
let _ = ctx
.v1
.replica
.destroy_replica(v1_rpc::replica::DestroyReplicaRequest {
uuid: uuid.clone(),
},
);
})
.await
.context(GrpcStatus)?;

match ctx.output {
OutputFormat::Json => {}
OutputFormat::Default => {
println!("replica: {} is deleted", &uuid);
}
};
}

Ok(())
}

Expand All @@ -283,6 +240,7 @@ async fn replica_list(
.list_replicas(v1_rpc::replica::ListReplicaOptions {
name: None,
poolname: None,
uuid: None,
})
.await
.context(GrpcStatus)?;
Expand Down
14 changes: 13 additions & 1 deletion io-engine/src/grpc/v0/mayastor_grpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl TryFrom<CreatePoolRequest> for PoolArgs {
}
}

impl From<LvsError> for Status {
impl From<LvsError> for tonic::Status {
fn from(e: LvsError) -> Self {
match e {
LvsError::Import {
Expand All @@ -217,6 +217,18 @@ impl From<LvsError> for Status {
Status::invalid_argument(e.to_string())
}
}
LvsError::RepDestroy {
source, ..
} => {
if source == Errno::ENOENT {
Status::not_found(e.to_string())
} else {
Status::internal(e.to_string())
}
}
LvsError::RepExists {
..
} => Status::already_exists(e.to_string()),
LvsError::ReplicaShareProtocol {
..
} => Status::invalid_argument(e.to_string()),
Expand Down
Loading

0 comments on commit b0734db

Please sign in to comment.