relax ping failures

src/ping.rs

@@ -6,31 +6,37 @@ use tokio::time::timeout;
 
 type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>;
 
-/// Pings an IP address using the system ping command and returns the round-trip time.
+/// Pings an IP address with multiple attempts for reliability.
 ///
-/// This implementation uses the system `ping` binary which is typically setuid root,
-/// allowing ICMP operations without requiring CAP_NET_RAW or elevated privileges
-/// for the application itself.
+/// This function sends multiple ping packets and tolerates some packet loss,
+/// making it more resilient to temporary network issues. It only fails if
+/// all packets are lost.
 ///
 /// # Arguments
 ///
 /// * `ip` - The IP address to ping
-/// * `timeout_duration` - Maximum time to wait for a response
+/// * `timeout_duration` - Maximum time to wait for responses
+/// * `count` - Number of ping packets to send (default: 3)
 ///
 /// # Returns
 ///
-/// * `Ok(Duration)` - The round-trip time if successful
-/// * `Err(anyhow::Error)` - If the ping fails or times out
-pub async fn ping(ip: IpAddr, timeout_duration: Duration) -> Result<Duration> {
+/// * `Ok(Duration)` - The average round-trip time if at least one packet succeeds
+/// * `Err(anyhow::Error)` - If all pings fail or 100% packet loss
+pub async fn ping_with_retries(
+    ip: IpAddr,
+    timeout_duration: Duration,
+    count: u32,
+) -> Result<Duration> {
     let ip_str = ip.to_string();
     let timeout_secs = timeout_duration.as_secs().max(1);
+    let packet_count = count.max(1).to_string();
 
     // Build ping command arguments based on OS
     let args = if cfg!(target_os = "macos") {
         // macOS: -W is timeout in ms
         vec![
             "-c".to_string(),
-            "1".to_string(),
+            packet_count,
             "-W".to_string(),
             (timeout_secs * 1000).to_string(),
             ip_str.clone(),
@@ -39,15 +45,16 @@ pub async fn ping(ip: IpAddr, timeout_duration: Duration) -> Result<Duration> {
         // Linux: -W is timeout in seconds
         vec![
             "-c".to_string(),
-            "1".to_string(),
+            packet_count,
             "-W".to_string(),
             timeout_secs.to_string(),
             ip_str.clone(),
         ]
     };
 
-    // Execute ping command with timeout
-    let result = timeout(timeout_duration + Duration::from_secs(1), async {
+    // Execute ping command with timeout (extra time for multiple packets)
+    let total_timeout = timeout_duration * count + Duration::from_secs(2);
+    let result = timeout(total_timeout, async {
         let output = Command::new("ping")
             .args(&args)
             .stdout(Stdio::piped())
@@ -55,12 +62,18 @@ pub async fn ping(ip: IpAddr, timeout_duration: Duration) -> Result<Duration> {
             .output()
             .await?;
 
-        if output.status.success() {
-            let stdout = String::from_utf8_lossy(&output.stdout);
+        let stdout = String::from_utf8_lossy(&output.stdout);
+
+        // Check for 100% packet loss - this is a hard failure
+        if stdout.contains("100% packet loss") || stdout.contains("100.0% packet loss") {
+            return Err(format!("All {} ping packets lost to {}", count, ip_str).into());
+        }
+
+        // If we got any successful packets, parse the average RTT
+        if output.status.success() || !stdout.is_empty() {
             parse_ping_output(&stdout)
         } else {
             let stderr = String::from_utf8_lossy(&output.stderr);
-            let stdout = String::from_utf8_lossy(&output.stdout);
             Err(format!(
                 "Ping failed for {}: {}{}",
                 ip_str,
@@ -74,7 +87,7 @@ pub async fn ping(ip: IpAddr, timeout_duration: Duration) -> Result<Duration> {
 
     match result {
         Ok(inner_result) => inner_result,
-        Err(_) => Err(format!("Ping timeout for {}", ip_str).into()),
+        Err(_) => Err(format!("Ping timeout for {} after {} attempts", ip_str, count).into()),
     }
 }
 
@@ -201,27 +214,46 @@ rtt min/avg/max/mdev = 0.543/0.543/0.543/0.000 ms"#;
     }
 
     #[tokio::test]
-    async fn test_ping_localhost() {
-        // This test actually pings localhost - skip if no network
+    async fn test_ping_with_retries_localhost() {
+        // Test multi-packet ping to localhost
         let ip: IpAddr = "127.0.0.1".parse().unwrap();
-        let result = ping(ip, Duration::from_secs(5)).await;
+        let result = ping_with_retries(ip, Duration::from_secs(5), 3).await;
 
         // Localhost ping should succeed on most systems
         if result.is_ok() {
             let duration = result.unwrap();
-            // Localhost should respond in < 100ms
+            // Localhost should respond in < 100ms on average
             assert!(duration.as_millis() < 100);
         }
         // If it fails, that's okay too - some systems don't allow ping to localhost
     }
 
     #[tokio::test]
-    async fn test_ping_invalid_ip() {
+    async fn test_ping_with_retries_invalid_ip() {
         // This should fail - non-routable IP
         let ip: IpAddr = "192.0.2.1".parse().unwrap(); // TEST-NET-1, not routable
-        let result = ping(ip, Duration::from_secs(2)).await;
+        let result = ping_with_retries(ip, Duration::from_secs(2), 3).await;
 
-        // Should either timeout or fail
+        // Should fail with 100% packet loss
         assert!(result.is_err());
     }
+
+    #[test]
+    fn test_parse_ping_output_with_packet_loss() {
+        // Test output with some packet loss but not 100%
+        let output = r#"PING 192.168.1.1 (192.168.1.1): 56 data bytes
+64 bytes from 192.168.1.1: icmp_seq=0 ttl=64 time=1.234 ms
+Request timeout for icmp_seq 1
+
+--- 192.168.1.1 ping statistics ---
+3 packets transmitted, 1 packets received, 66.7% packet loss
+round-trip min/avg/max/stddev = 1.234/1.234/1.234/0.000 ms"#;
+
+        let result = parse_ping_output(output);
+        assert!(result.is_ok());
+        let duration = result.unwrap();
+        // Should still parse the average from the one successful packet
+        assert!(duration.as_secs_f64() > 0.001);
+        assert!(duration.as_secs_f64() < 0.002);
+    }
 }

src/websocket_client.rs

@@ -21,7 +21,7 @@ const CONNECTION_TIMEOUT: Duration = Duration::from_secs(30);
 
 type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>;
 
-use crate::ping::ping;
+use crate::ping::ping_with_retries;
 use crate::proto::agent::{
     AgentHeartbeat, AgentJob, AgentJobList, JobType, MonitoringCheck, QueryType, SnmpResult,
 };
@@ -666,7 +666,8 @@ async fn run_monitoring_task(
 
                 let timeout = Duration::from_secs(5);
 
-                match ping(ip_addr, timeout).await {
+                // Send 3 pings for reliability - only fail if all 3 fail
+                match ping_with_retries(ip_addr, timeout, 3).await {
                     Ok(rtt) => {
                         let check = MonitoringCheck {
                             device_id: device_id.clone(),