diff --git a/rustfs/src/admin/handlers/site_replication.rs b/rustfs/src/admin/handlers/site_replication.rs index 067f6a5ab..796e3804e 100644 --- a/rustfs/src/admin/handlers/site_replication.rs +++ b/rustfs/src/admin/handlers/site_replication.rs @@ -4283,7 +4283,7 @@ async fn probe_reverse_peer_reachability(state: &SiteReplicationState, local_pee continue; } }; - if let Err(err) = PeerAdminRequest::put(&connection, SITE_REPLICATION_DEVNULL_PATH, &state.service_account_access_key) + if let Err(err) = devnull_probe_request(&connection, &state.service_account_access_key) .send(&secret_key, &serde_json::json!({})) .await { @@ -4294,6 +4294,13 @@ async fn probe_reverse_peer_reachability(state: &SiteReplicationState, local_pee errors } +/// The probe must target devnull with a method the admin router actually +/// registers for it — a mismatched method makes every probe fail and turns +/// the "not reachable" warning into permanent noise. +pub(crate) fn devnull_probe_request<'a>(connection: &'a PeerConnection, access_key: &'a str) -> PeerAdminRequest<'a> { + PeerAdminRequest::post(connection, SITE_REPLICATION_DEVNULL_PATH, access_key) +} + async fn backfill_existing_buckets_after_add( state: &SiteReplicationState, local_peer: &PeerInfo, @@ -11098,13 +11105,15 @@ mod tests { deployment_id: "remote".to_string(), ..peer("remote", "https://remote.example.com") }; - let detail = "x".repeat(SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT + 32); + let detail = format!("HEAD{}TAIL", "x".repeat(SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT + 32)); let error = status_peer_error(&remote, detail); assert_eq!(error.name, "remote"); assert_eq!(error.endpoint, "https://remote.example.com"); - assert!(error.error.ends_with("(truncated)")); + assert!(error.error.contains("(truncated)")); + assert!(error.error.starts_with("HEAD")); + assert!(error.error.ends_with("TAIL")); assert!(error.error.chars().count() <= SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT); } diff --git a/rustfs/src/admin/route_registration_test.rs b/rustfs/src/admin/route_registration_test.rs index 8508e06ce..25b84a165 100644 --- a/rustfs/src/admin/route_registration_test.rs +++ b/rustfs/src/admin/route_registration_test.rs @@ -789,6 +789,19 @@ fn expected_admin_route_matrix() -> Vec { // registered_admin_router pins ENV_HEALTH_ENDPOINT_ENABLE because the // production registration helper intentionally honors that environment switch. +#[test] +#[serial] +fn test_reverse_probe_targets_a_registered_devnull_route() { + // The reverse-reachability probe used to send PUT while only POST is + // routed for devnull, so every probe failed with 501 and every add/join + // reported a false "not reachable" warning. + let router = registered_admin_router(); + let connection = crate::site_replication::transport::PeerConnection::new("http://peer.example.com:9000", false, "") + .expect("peer connection"); + let request = crate::admin::handlers::site_replication::devnull_probe_request(&connection, "site-replicator-0"); + assert_route(&router, request.method().clone(), request.path()); +} + #[test] #[serial] fn test_admin_route_matrix_matches_registered_routes() { diff --git a/rustfs/src/site_replication/transport.rs b/rustfs/src/site_replication/transport.rs index bbdb31c77..30ad28f95 100644 --- a/rustfs/src/site_replication/transport.rs +++ b/rustfs/src/site_replication/transport.rs @@ -612,11 +612,28 @@ impl<'a> PeerAdminRequest<'a> { } } + pub(crate) fn post(connection: &'a PeerConnection, path: &'a str, access_key: &'a str) -> Self { + Self { + method: Method::POST, + ..Self::put(connection, path, access_key) + } + } + pub(crate) fn with_client(mut self, client: &'a reqwest::Client) -> Self { self.client = Some(client); self } + #[cfg(test)] + pub(crate) fn method(&self) -> &Method { + &self.method + } + + #[cfg(test)] + pub(crate) fn path(&self) -> &str { + self.path + } + async fn resolved_client(&self) -> S3Result { match self.client { Some(client) => Ok(client.clone()), @@ -916,11 +933,15 @@ pub(crate) fn summarize_peer_error_detail(detail: &str) -> String { return detail.to_string(); } - let suffix = "... (truncated)"; - let take_chars = SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT.saturating_sub(suffix.chars().count()); - let mut summary: String = detail.chars().take(take_chars).collect(); - summary.push_str(suffix); - summary + // Keep both ends: nested peer errors append the decisive status/code at + // the tail, so a prefix-only cut drops exactly the part an operator needs. + let ellipsis = " ... (truncated) ... "; + let budget = SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT.saturating_sub(ellipsis.chars().count()); + let head_chars = budget / 2; + let tail_chars = budget - head_chars; + let head: String = detail.chars().take(head_chars).collect(); + let tail: String = detail.chars().skip(detail_chars.saturating_sub(tail_chars)).collect(); + format!("{head}{ellipsis}{tail}") } #[cfg(test)] @@ -928,6 +949,21 @@ mod tests { use super::*; use serial_test::serial; + #[test] + fn test_summarize_peer_error_detail_keeps_head_and_tail() { + let detail = format!("HEAD{}TAIL", "x".repeat(SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT * 2)); + let summary = summarize_peer_error_detail(&detail); + assert!(summary.starts_with("HEAD")); + assert!(summary.ends_with("TAIL")); + assert!(summary.contains("(truncated)")); + assert!(summary.chars().count() <= SITE_REPLICATION_PEER_ERROR_DETAIL_LIMIT); + } + + #[test] + fn test_summarize_peer_error_detail_passes_short_details_through() { + assert_eq!(summarize_peer_error_detail(" short error "), "short error"); + } + #[tokio::test] #[serial] async fn test_site_replication_peer_client_rebuilds_when_generation_changes() {