From 9bc119c04f7accdc19fb77e8bfc0604ce3188caf Mon Sep 17 00:00:00 2001
From: Adam Chen <encodeous7@gmail.com>
Date: Sat, 5 Sep 2026 14:13:34 +0000
Subject: [PATCH] fix(prefix_health): override metric should preserve INF
 metric

---
 e2e/healthcheck_test.go     | 18 ++++++++++++++----
 e2e/recovery_test.go        | 29 ++++++++++++++++++++++-------
 state/prefix_health.go      |  6 +++++-
 state/prefix_health_test.go | 14 ++++++++++++++
 4 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/e2e/healthcheck_test.go b/e2e/healthcheck_test.go
index 0cde5e47..1c8c6ceb 100644
--- a/e2e/healthcheck_test.go
+++ b/e2e/healthcheck_test.go
@@ -190,8 +190,9 @@ health.service.test. 0 IN A %s
 		},
 	}
 
-	// Configure Backup with Static check (Metric 1000)
-	backupMetric := uint32(1000)
+	// Keep the backup more expensive even with the initial one-second link
+	// metric, CI latency variation, and the 10% route-switch deadband.
+	backupMetric := state.DurationToMetric(10 * time.Second)
 	central.Routers[2].Prefixes = []state.PrefixHealthWrapper{
 		{
 			&state.StaticPrefixHealth{
@@ -229,7 +230,7 @@ health.service.test. 0 IN A %s
 
 	// A. Initial state: HTTP server is DOWN on primary.
 	// Primary health check should fail (Metric INF).
-	// Client should route to Backup (Metric 1000).
+	// Client should route to Backup.
 
 	t.Log("Step A: Waiting for routing to fallback (Primary DOWN)")
 	h.WaitForStatus(t, "client", func(status *protocol.StatusResponse) bool {
@@ -246,7 +247,16 @@ health.service.test. 0 IN A %s
 	// C. Wait for Primary to become healthy
 	// Primary should advertise Metric 10.
 	// Client should switch to Primary.
-	t.Log("Step C: Waiting for routing to switch to Primary (Primary UP)")
+	t.Log("Step C: Waiting for Primary to advertise healthy Metric 10")
+	h.WaitForStatus(t, "primary", func(status *protocol.StatusResponse) bool {
+		for _, prefix := range status.GetNode().GetAdvertised() {
+			if prefix.GetPrefix() == servicePrefixStr && prefix.GetMetric() == 10 {
+				return true
+			}
+		}
+		return false
+	})
+	t.Log("Waiting for routing to switch to Primary (Primary UP)")
 	h.WaitForStatus(t, "client", func(status *protocol.StatusResponse) bool {
 		return HasSelectedRoute(status, "10.0.3.1/32", "primary", "primary")
 	})
diff --git a/e2e/recovery_test.go b/e2e/recovery_test.go
index ac08fe2e..fa6261e6 100644
--- a/e2e/recovery_test.go
+++ b/e2e/recovery_test.go
@@ -109,9 +109,18 @@ func TestRecoveryExample(t *testing.T) {
 	}
 
 	// 6. Wait for recovery
+	defer func() {
+		if t.Failed() {
+			for _, name := range nodeNames {
+				stdout, stderr, err := h.Exec(name, []string{"nylon", "status", "-i", "nylon0", "--json"})
+				t.Logf("Recovery status for %s (error: %v):\n%s\n%s", name, err, stdout, stderr)
+			}
+		}
+	}()
 	t.Log("Waiting for recovery (rerouting)...")
 	// Start a background pinger to trigger routing
 	stopPinger := make(chan struct{})
+	defer close(stopPinger)
 	go func() {
 		ticker := time.NewTicker(time.Second)
 		defer ticker.Stop()
@@ -125,13 +134,19 @@ func TestRecoveryExample(t *testing.T) {
 		}
 	}()
 	h.WaitForTrace(alice, fmt.Sprintf("Fwd packet: %s -> %s, via %s", nylonIPs[alice], nylonIPs[bob], vps))
-	close(stopPinger)
-
-	t.Log("Recovery successful! Traffic rerouted via VPS.")
 
-	// Final connectivity check
-	stdout, stderr, err = h.Exec(alice, []string{"ping", "-c", "3", nylonIPs[bob]})
-	if err != nil {
-		t.Fatalf("Post-recovery ping failed: %v\nStdout: %s\nStderr: %s", err, stdout, stderr)
+	// A forwarding trace only establishes Alice's next hop. Wait for the
+	// remaining hops and the return path to recover before asserting delivery.
+	deadline := time.Now().Add(WaitTimeout)
+	for {
+		stdout, stderr, err = h.Exec(alice, []string{"ping", "-c", "3", "-W", "1", "-w", "4", nylonIPs[bob]})
+		if err == nil {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("Timed out waiting for post-recovery connectivity: %v\nStdout: %s\nStderr: %s", err, stdout, stderr)
+		}
+		time.Sleep(time.Second)
 	}
+	t.Log("Recovery successful! Traffic rerouted via VPS and connectivity restored.")
 }
diff --git a/state/prefix_health.go b/state/prefix_health.go
index 4ba53d75..3e1e0290 100644
--- a/state/prefix_health.go
+++ b/state/prefix_health.go
@@ -282,10 +282,14 @@ type httpPrefixHealthMonitor struct {
 }
 
 func (h *httpPrefixHealthMonitor) GetMetric() uint32 {
+	metric := h.lastMetric.Load()
+	if metric == INF {
+		return INF
+	}
 	if h.hasMetricOverride {
 		return h.metricOverride
 	}
-	return h.lastMetric.Load()
+	return metric
 }
 
 func (h *httpPrefixHealthMonitor) Stop() {
diff --git a/state/prefix_health_test.go b/state/prefix_health_test.go
index c5dde6ae..e412d349 100644
--- a/state/prefix_health_test.go
+++ b/state/prefix_health_test.go
@@ -115,6 +115,20 @@ delay: 5s
 	}
 }
 
+func TestHTTPPrefixHealthMetricOverridePreservesFailures(t *testing.T) {
+	monitor := &httpPrefixHealthMonitor{hasMetricOverride: true, metricOverride: 10}
+	for _, metric := range []uint32{INF, 250, INF} {
+		monitor.lastMetric.Store(metric)
+		want := uint32(10)
+		if metric == INF {
+			want = INF
+		}
+		if got := monitor.GetMetric(); got != want {
+			t.Fatalf("health metric %d: got %d, want %d", metric, got, want)
+		}
+	}
+}
+
 type roundTripFunc func(*http.Request) (*http.Response, error)
 
 func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {