chore(bors): merge pull request #782

782: Tracing and Platform fixes r=tiagolobocastro a=tiagolobocastro

    feat(platform): support custom namespace
    
    When we run outside of the cluster, we can't just use the env to determine
    the correct namespace..
    For now we expose an additional method which allows overriding it.
    A better solution is perhaps to simply ensure we create clients with the
    correct namespace as default?
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

---

    fix(agents/core): use tracing api
    
    Incorrect log api was used for error logging.
    Also include error source in the service error.
    
    Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>

Co-authored-by: Tiago Castro <tiagolobocastro@gmail.com>

control-plane/agents/src/bin/core/nexus/operations_helper.rs

@@ -20,8 +20,6 @@ use stor_port::types::v0::{
     },
 };
 
-use tracing::log::error;
-
 impl OperationGuardArc<NexusSpec> {
     /// Attach the specified replica to the volume nexus
     /// The replica might need to be shared/unshared so it can be opened by the nexus
@@ -143,9 +141,10 @@ impl OperationGuardArc<NexusSpec> {
                     replica_state.node.clone(),
                 );
                 if let Err(error) = node.set_replica_entity_id(&set_entity_id).await {
-                    error!(
-                        "Failed to set entity_id for the replica {}",
-                        error.to_string()
+                    tracing::error!(
+                        replica.uuid = %replica_state.uuid,
+                        %error,
+                        "Failed to set entity_id",
                     );
                 }
             }

control-plane/agents/src/bin/core/node/wrapper.rs

@@ -1283,22 +1283,15 @@ impl ReplicaApi for Arc<tokio::sync::RwLock<NodeWrapper>> {
 
                 Ok(replica)
             }
-            Err(SvcError::GrpcRequestError { source, .. })
-                if source.code() == tonic::Code::DataLoss =>
-            {
-                Err(SvcError::ReplicaSetPropertyFailed {
-                    attribute: "entity_id".to_string(),
-                    replica: request.uuid.to_string(),
-                })
-            }
             Err(error) => {
                 let mut ctx = dataplane.reconnect(GETS_TIMEOUT).await?;
                 self.update_pool_replica_states(ctx.deref_mut()).await?;
 
                 match self.read().await.replicas().into_iter().find(|replica| {
-                    replica.uuid == request.uuid
+                    (replica.uuid == request.uuid
                         || replica.name
-                            == ReplicaName::from_opt_uuid(request.name.as_ref(), &request.uuid)
+                            == ReplicaName::from_opt_uuid(request.name.as_ref(), &request.uuid))
+                        && replica.entity_id == request.entity_id
                 }) {
                     Some(replica) => Ok(replica),
                     None => Err(error),
@@ -1420,6 +1413,7 @@ impl ReplicaApi for Arc<tokio::sync::RwLock<NodeWrapper>> {
                 Err(SvcError::ReplicaSetPropertyFailed {
                     attribute: "entity_id".to_string(),
                     replica: request.uuid().to_string(),
+                    source,
                 })
             }
             Err(error) => Err(error),

control-plane/agents/src/common/errors.rs

@@ -402,8 +402,12 @@ pub enum SvcError {
         replicas: u8,
         id: String,
     },
-    #[snafu(display("Cannot set '{attribute}' on Replica '{replica}'"))]
-    ReplicaSetPropertyFailed { attribute: String, replica: String },
+    #[snafu(display("Cannot set '{attribute}' on Replica '{replica}: {source}'"))]
+    ReplicaSetPropertyFailed {
+        attribute: String,
+        replica: String,
+        source: tonic::Status,
+    },
 }
 
 impl SvcError {

utils/platform/src/k8s/mod.rs

@@ -19,12 +19,24 @@ impl K8s {
     /// Create new `Self` from a given `Client`.
     pub async fn from(client: Client) -> Result<Self, PlatformError> {
         let platform_uuid = Self::fetch_platform_uuid(client).await?;
+        // todo: should we use the default namespace from the client?
         let platform_ns = std::env::var("MY_POD_NAMESPACE").unwrap_or_else(|_| "default".into());
         Ok(Self {
             platform_uuid,
             platform_ns,
         })
     }
+    /// Create new `Self` from a given `Client` and namespace.
+    /// This may be necessary if we're running outside of the cluster itself, which
+    /// means that MY_POD_NAMESPACE itself is not available... Perhaps using a client
+    /// configured with the namespace as default would be better.
+    pub async fn from_custom(client: Client, namespace: &str) -> Result<Self, PlatformError> {
+        let platform_uuid = Self::fetch_platform_uuid(client).await?;
+        Ok(Self {
+            platform_uuid,
+            platform_ns: namespace.to_string(),
+        })
+    }
     async fn fetch_platform_uuid(client: Client) -> Result<PlatformUid, PlatformError> {
         let namespaces: Api<Namespace> = Api::all(client);
         let ns = namespaces