fix: replica destroy when pool not found

Should return failed preconditions rather than NotFound.

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

io-engine/src/grpc/v1/lvm/pool.rs

@@ -3,12 +3,12 @@ use crate::{
     grpc::{
         acquire_subsystem_lock,
         lvm_enabled,
-        v1::pool::{PoolGrpc, PoolProbe},
+        v1::pool::{PoolGrpc, PoolIdProbe, PoolSvcRpc},
         GrpcResult,
     },
     lvm::{CmnQueryArgs, Error as LvmError, VolumeGroup},
     lvs::Lvs,
-    pool_backend::PoolArgs,
+    pool_backend::{PoolArgs, PoolBackend},
 };
 use io_engine_api::v1::pool::*;
 use std::{convert::TryFrom, fmt::Debug};
@@ -28,16 +28,16 @@ impl PoolService {
     }
     /// Probe the LVM Pool service for a pool.
     pub(crate) async fn probe(
-        probe: &PoolProbe,
+        probe: &PoolIdProbe,
     ) -> Result<bool, tonic::Status> {
         if !MayastorFeatures::get_features().lvm() {
             return Ok(false);
         }
 
         let query = match probe {
-            PoolProbe::Uuid(uuid) => CmnQueryArgs::ours().uuid(uuid),
-            PoolProbe::UuidOrName(uuid) => CmnQueryArgs::ours().uuid(uuid),
-            PoolProbe::NameUuid {
+            PoolIdProbe::Uuid(uuid) => CmnQueryArgs::ours().uuid(uuid),
+            PoolIdProbe::UuidOrName(uuid) => CmnQueryArgs::ours().uuid(uuid),
+            PoolIdProbe::NameUuid {
                 name,
                 uuid,
             } => CmnQueryArgs::ours().named(name).uuid_opt(uuid),
@@ -101,6 +101,13 @@ async fn find_pool(
     Ok(pool)
 }
 
+#[async_trait::async_trait]
+impl PoolSvcRpc for PoolService {
+    fn kind(&self) -> PoolBackend {
+        PoolBackend::Lvm
+    }
+}
+
 #[tonic::async_trait]
 impl PoolRpc for PoolService {
     async fn create_pool(

io-engine/src/grpc/v1/lvs/pool.rs

@@ -4,11 +4,11 @@ use crate::{
         acquire_subsystem_lock,
         rpc_submit,
         rpc_submit_ext,
-        v1::pool::{PoolGrpc, PoolProbe},
+        v1::pool::{PoolGrpc, PoolIdProbe, PoolSvcRpc},
         GrpcResult,
     },
     lvs::{BsError, Lvs, LvsError},
-    pool_backend::PoolArgs,
+    pool_backend::{PoolArgs, PoolBackend},
 };
 use io_engine_api::v1::pool::*;
 use std::{convert::TryFrom, fmt::Debug};
@@ -27,15 +27,17 @@ impl PoolService {
         Self {}
     }
     /// Probe the LVS Pool service for a pool.
-    pub(crate) async fn probe(probe: PoolProbe) -> Result<bool, tonic::Status> {
+    pub(crate) async fn probe(
+        probe: PoolIdProbe,
+    ) -> Result<bool, tonic::Status> {
         let rx = rpc_submit_ext(async move {
             match probe {
-                PoolProbe::Uuid(uuid) => Lvs::lookup_by_uuid(&uuid).is_some(),
-                PoolProbe::UuidOrName(id) => {
+                PoolIdProbe::Uuid(uuid) => Lvs::lookup_by_uuid(&uuid).is_some(),
+                PoolIdProbe::UuidOrName(id) => {
                     Lvs::lookup_by_uuid(&id).is_some()
                         || Lvs::lookup(&id).is_some()
                 }
-                PoolProbe::NameUuid {
+                PoolIdProbe::NameUuid {
                     name,
                     uuid,
                 } => match uuid {
@@ -88,7 +90,7 @@ async fn find_pool(
     let Some(pool) = Lvs::lookup(name) else {
         return Err(LvsError::PoolNotFound {
             source: BsError::LvsNotFound {},
-            msg: format!("Destroy failed as pool {name} was not found"),
+            msg: format!("Pool {name} was not found"),
         }
         .into());
     };
@@ -105,6 +107,13 @@ async fn find_pool(
     Ok(pool)
 }
 
+#[async_trait::async_trait]
+impl PoolSvcRpc for PoolService {
+    fn kind(&self) -> PoolBackend {
+        PoolBackend::Lvs
+    }
+}
+
 #[tonic::async_trait]
 impl PoolRpc for PoolService {
     async fn create_pool(

io-engine/src/grpc/v1/lvs/replica.rs

@@ -12,7 +12,7 @@ use crate::{
         v1::{pool::PoolGrpc, replica::ReplicaGrpc},
         GrpcResult,
     },
-    lvs::{LvsError, BsError, Lvol, Lvs, LvsLvol},
+    lvs::{BsError, Lvol, Lvs, LvsError, LvsLvol},
 };
 use io_engine_api::v1::{pool::PoolType, replica::*};
 use std::convert::TryFrom;

io-engine/src/grpc/v1/pool.rs

@@ -33,11 +33,37 @@ struct UnixStream(tokio::net::UnixStream);
 
 /// Probe for pools using this criteria.
 #[derive(Debug, Clone)]
-pub enum PoolProbe {
+pub enum PoolIdProbe {
     Uuid(String),
     UuidOrName(String),
     NameUuid { name: String, uuid: Option<String> },
 }
+impl PoolIdProbe {
+    fn name_uuid(name: &str, uuid: &Option<String>) -> Self {
+        Self::NameUuid {
+            name: name.to_owned(),
+            uuid: uuid.to_owned(),
+        }
+    }
+}
+
+/// The different types of probing.
+pub(crate) enum ProbeType {
+    /// Probing the specific pool type directly.
+    ByKind(PoolBackend),
+    /// Probing using identifiers.
+    ById(PoolIdProbe),
+}
+impl From<PoolIdProbe> for ProbeType {
+    fn from(value: PoolIdProbe) -> Self {
+        Self::ById(value)
+    }
+}
+impl From<PoolBackend> for ProbeType {
+    fn from(value: PoolBackend) -> Self {
+        Self::ByKind(value)
+    }
+}
 
 /// RPC service for mayastor pool operations
 #[derive(Debug, Clone)]
@@ -219,6 +245,11 @@ impl Default for PoolService {
     }
 }
 
+#[async_trait::async_trait]
+pub trait PoolSvcRpc: PoolRpc {
+    fn kind(&self) -> PoolBackend;
+}
+
 pub(crate) struct PoolGrpc<P: PoolOps> {
     pool: P,
 }
@@ -296,46 +327,32 @@ impl PoolService {
             client_context: std::sync::Arc::new(tokio::sync::RwLock::new(None)),
         }
     }
-    /// Return a backend for the given type.
-    fn backend(
-        &self,
-        pooltype: i32,
-    ) -> Result<Box<dyn PoolRpc>, tonic::Status> {
-        Ok(match PoolBackend::try_from(pooltype)? {
-            PoolBackend::Lvs => Box::new(LvsSvc::new()),
-            PoolBackend::Lvm => Box::new(LvmSvc::new()),
-        })
-    }
+
     /// Probe backends for the given name and/or uuid and return the right one.
-    async fn probe_backend(
+    pub(crate) async fn probe<I: Into<ProbeType>>(
         &self,
-        name: &str,
-        uuid: &Option<String>,
-    ) -> Result<Box<dyn PoolRpc>, tonic::Status> {
-        let probe = PoolProbe::NameUuid {
-            name: name.to_owned(),
-            uuid: uuid.to_owned(),
+        probe: I,
+    ) -> Result<Box<dyn PoolSvcRpc>, tonic::Status> {
+        let probe = probe.into();
+        let kind = match probe {
+            ProbeType::ByKind(kind) => kind,
+            ProbeType::ById(probe) => match (
+                LvmSvc::probe(&probe).await,
+                LvsSvc::probe(probe.clone()).await,
+            ) {
+                (Ok(true), _) => Ok(PoolBackend::Lvm),
+                (_, Ok(true)) => Ok(PoolBackend::Lvs),
+                (Err(error), _) | (_, Err(error)) => Err(error),
+                _ => {
+                    Err(Status::not_found(format!("Pool {probe:?} not found")))
+                }
+            }?,
         };
-        Ok(match self.probe_backend_kind(probe).await? {
-            PoolBackend::Lvm => Box::new(LvmSvc::new()),
+        Ok(match kind {
             PoolBackend::Lvs => Box::new(LvsSvc::new()),
+            PoolBackend::Lvm => Box::new(LvmSvc::new()),
         })
     }
-
-    pub(crate) async fn probe_backend_kind(
-        &self,
-        probe: PoolProbe,
-    ) -> Result<PoolBackend, tonic::Status> {
-        match (
-            LvmSvc::probe(&probe).await,
-            LvsSvc::probe(probe.clone()).await,
-        ) {
-            (Ok(true), _) => Ok(PoolBackend::Lvm),
-            (_, Ok(true)) => Ok(PoolBackend::Lvs),
-            (Err(error), _) | (_, Err(error)) => Err(error),
-            _ => Err(Status::not_found(format!("Pool {probe:?} not found"))),
-        }
-    }
 }
 
 #[tonic::async_trait]
@@ -345,13 +362,15 @@ impl PoolRpc for PoolService {
         &self,
         request: Request<CreatePoolRequest>,
     ) -> GrpcResult<Pool> {
-        let backend = self.backend(request.get_ref().pooltype)?;
+        let backend = self
+            .probe(PoolBackend::try_from(request.get_ref().pooltype)?)
+            .await;
         self.locked(
             GrpcClientContext::new(&request, function_name!()),
             async move {
                 info!("{:?}", request.get_ref());
 
-                backend.create_pool(request).await
+                backend?.create_pool(request).await
             },
         )
         .await
@@ -363,14 +382,17 @@ impl PoolRpc for PoolService {
         request: Request<DestroyPoolRequest>,
     ) -> GrpcResult<()> {
         let backend = self
-            .probe_backend(&request.get_ref().name, &request.get_ref().uuid)
-            .await?;
+            .probe(PoolIdProbe::name_uuid(
+                &request.get_ref().name,
+                &request.get_ref().uuid,
+            ))
+            .await;
         self.locked(
             GrpcClientContext::new(&request, function_name!()),
             async move {
                 info!("{:?}", request.get_ref());
 
-                backend.destroy_pool(request).await
+                backend?.destroy_pool(request).await
             },
         )
         .await
@@ -382,14 +404,17 @@ impl PoolRpc for PoolService {
         request: Request<ExportPoolRequest>,
     ) -> GrpcResult<()> {
         let backend = self
-            .probe_backend(&request.get_ref().name, &request.get_ref().uuid)
-            .await?;
+            .probe(PoolIdProbe::name_uuid(
+                &request.get_ref().name,
+                &request.get_ref().uuid,
+            ))
+            .await;
         self.locked(
             GrpcClientContext::new(&request, function_name!()),
             async move {
                 info!("{:?}", request.get_ref());
 
-                backend.export_pool(request).await
+                backend?.export_pool(request).await
             },
         )
         .await
@@ -400,13 +425,15 @@ impl PoolRpc for PoolService {
         &self,
         request: Request<ImportPoolRequest>,
     ) -> GrpcResult<Pool> {
-        let backend = self.backend(request.get_ref().pooltype)?;
+        let backend = self
+            .probe(PoolBackend::try_from(request.get_ref().pooltype)?)
+            .await;
         self.locked(
             GrpcClientContext::new(&request, function_name!()),
             async move {
                 info!("{:?}", request.get_ref());
 
-                backend.import_pool(request).await
+                backend?.import_pool(request).await
             },
         )
         .await