fix(site-replication): send the reverse-reachability probe as POST (#6790)

This commit is contained in:
唐小鸭
2026-08-28 19:50:09 +08:00
committed by GitHub
parent eb6b617ca2
commit 3b87d61cbf
3 changed files with 66 additions and 8 deletions
+12 -3
View File
@@ -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);
}
@@ -789,6 +789,19 @@ fn expected_admin_route_matrix() -> Vec<RouteMatrixEntry> {
// 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() {
+41 -5
View File
@@ -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<reqwest::Client> {
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() {