diff --git a/control-plane/agents/src/bin/core/nexus/operations_helper.rs b/control-plane/agents/src/bin/core/nexus/operations_helper.rs index 6bc5e5964..57ffe7b7d 100644 --- a/control-plane/agents/src/bin/core/nexus/operations_helper.rs +++ b/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 { /// 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 { 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", ); } } diff --git a/control-plane/agents/src/bin/core/node/wrapper.rs b/control-plane/agents/src/bin/core/node/wrapper.rs index 8d3c02ddb..d033887ef 100644 --- a/control-plane/agents/src/bin/core/node/wrapper.rs +++ b/control-plane/agents/src/bin/core/node/wrapper.rs @@ -1283,22 +1283,15 @@ impl ReplicaApi for Arc> { 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> { Err(SvcError::ReplicaSetPropertyFailed { attribute: "entity_id".to_string(), replica: request.uuid().to_string(), + source, }) } Err(error) => Err(error), diff --git a/control-plane/agents/src/common/errors.rs b/control-plane/agents/src/common/errors.rs index 41cf63e56..e0a7557e7 100644 --- a/control-plane/agents/src/common/errors.rs +++ b/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 { diff --git a/utils/platform/src/k8s/mod.rs b/utils/platform/src/k8s/mod.rs index 658c2becd..279afd8a2 100644 --- a/utils/platform/src/k8s/mod.rs +++ b/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 { 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 { + 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 { let namespaces: Api = Api::all(client); let ns = namespaces