diff --git a/pkg/goldpinger/stats_test.go b/pkg/goldpinger/stats_test.go index e6a5a7c..2bf359f 100644 --- a/pkg/goldpinger/stats_test.go +++ b/pkg/goldpinger/stats_test.go @@ -12,67 +12,108 @@ import ( // a destroyed peer. This prevents stale pod IPs from lingering in // /metrics after rolling updates (see #167). func TestDeletePeerMetrics_CleansResponseTimeHistogram(t *testing.T) { - origHostname := GoldpingerConfig.Hostname - GoldpingerConfig.Hostname = "test-instance" - defer func() { GoldpingerConfig.Hostname = origHostname }() - - hostIP := "10.0.0.1" - podIP := "10.0.0.2" - - // Simulate a ping observation (call_type="ping") - goldpingerResponseTimePeersHistogram.WithLabelValues( - GoldpingerConfig.Hostname, "ping", hostIP, podIP, - ).Observe(0.005) - - if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { - t.Fatal("response time histogram has no label values before cleanup — test setup is broken") + tests := []struct { + name string + hostIP string + podIP string + }{ + {"IPv4", "10.0.0.1", "10.0.0.2"}, + {"IPv6", "2001:db8::1", "2001:db8:1::2"}, } - DeletePeerMetrics(hostIP, podIP) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + GoldpingerConfig.Hostname = "test-instance" + defer func() { GoldpingerConfig.Hostname = origHostname }() - if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 0 { - t.Errorf("response time histogram still has %d label set(s) after DeletePeerMetrics", n) + // Simulate a ping observation (call_type="ping") + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", tt.hostIP, tt.podIP, + ).Observe(0.005) + + if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { + t.Fatal("response time histogram has no label values before cleanup — test setup is broken") + } + + DeletePeerMetrics(tt.hostIP, tt.podIP) + + if n := countMetrics(goldpingerResponseTimePeersHistogram); n != 0 { + t.Errorf("response time histogram still has %d label set(s) after DeletePeerMetrics", n) + } + }) } } // TestDeletePeerMetrics_LeavesOtherPeersIntact verifies that pruning // metrics for one peer does not affect a different peer's label set. func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { - origHostname := GoldpingerConfig.Hostname - GoldpingerConfig.Hostname = "test-instance" - defer func() { GoldpingerConfig.Hostname = origHostname }() - - // Peer A - goldpingerResponseTimePeersHistogram.WithLabelValues( - GoldpingerConfig.Hostname, "ping", "10.0.0.1", "10.0.0.2", - ).Observe(0.005) - SetPeerLossPct("10.0.0.1", "10.0.0.2", 0) - - // Peer B - goldpingerResponseTimePeersHistogram.WithLabelValues( - GoldpingerConfig.Hostname, "ping", "10.0.0.3", "10.0.0.4", - ).Observe(0.010) - SetPeerLossPct("10.0.0.3", "10.0.0.4", 1.5) - - // Delete peer A only - DeletePeerMetrics("10.0.0.1", "10.0.0.2") - DeletePeerUDPMetrics("10.0.0.1", "10.0.0.2") - - // Peer B should survive in both metrics - if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { - t.Error("response time histogram lost all label sets — peer B should still exist") - } - if countMetrics(goldpingerPeersLossPct) == 0 { - t.Error("loss pct gauge lost all label sets — peer B should still exist") + tests := []struct { + name string + peerA [2]string // {hostIP, podIP} + peerB [2]string + }{ + { + "IPv4", + [2]string{"10.0.0.1", "10.0.0.2"}, + [2]string{"10.0.0.3", "10.0.0.4"}, + }, + { + "IPv6", + [2]string{"2001:db8::1", "2001:db8:1::2"}, + [2]string{"2001:db8::3", "2001:db8:2::4"}, + }, + { + "MixedV4DeleteV6Survives", + [2]string{"10.0.0.1", "10.0.0.2"}, + [2]string{"2001:db8::3", "2001:db8:2::4"}, + }, + { + "MixedV6DeleteV4Survives", + [2]string{"2001:db8::1", "2001:db8:1::2"}, + [2]string{"10.0.0.3", "10.0.0.4"}, + }, } - // Clean up peer B so it doesn't leak into other tests - goldpingerResponseTimePeersHistogram.DeleteLabelValues( - GoldpingerConfig.Hostname, "ping", "10.0.0.3", "10.0.0.4", - ) - goldpingerPeersLossPct.DeleteLabelValues( - GoldpingerConfig.Hostname, "10.0.0.3", "10.0.0.4", - ) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + GoldpingerConfig.Hostname = "test-instance" + defer func() { GoldpingerConfig.Hostname = origHostname }() + + // Peer A + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", tt.peerA[0], tt.peerA[1], + ).Observe(0.005) + SetPeerLossPct(tt.peerA[0], tt.peerA[1], 0) + + // Peer B + goldpingerResponseTimePeersHistogram.WithLabelValues( + GoldpingerConfig.Hostname, "ping", tt.peerB[0], tt.peerB[1], + ).Observe(0.010) + SetPeerLossPct(tt.peerB[0], tt.peerB[1], 1.5) + + // Delete peer A only + DeletePeerMetrics(tt.peerA[0], tt.peerA[1]) + DeletePeerUDPMetrics(tt.peerA[0], tt.peerA[1]) + + // Peer B should survive in both metrics + if countMetrics(goldpingerResponseTimePeersHistogram) == 0 { + t.Error("response time histogram lost all label sets — peer B should still exist") + } + if countMetrics(goldpingerPeersLossPct) == 0 { + t.Error("loss pct gauge lost all label sets — peer B should still exist") + } + + // Clean up peer B so it doesn't leak into other tests + goldpingerResponseTimePeersHistogram.DeleteLabelValues( + GoldpingerConfig.Hostname, "ping", tt.peerB[0], tt.peerB[1], + ) + goldpingerPeersLossPct.DeleteLabelValues( + GoldpingerConfig.Hostname, tt.peerB[0], tt.peerB[1], + ) + }) + } } // TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics verifies that @@ -80,51 +121,60 @@ func TestDeletePeerMetrics_LeavesOtherPeersIntact(t *testing.T) { // If a new per-peer metric is added but not cleaned up in // DeletePeerUDPMetrics, this test will fail. func TestDeletePeerUDPMetrics_CleansAllPerPeerMetrics(t *testing.T) { - // Save and restore hostname since we set it for the test - origHostname := GoldpingerConfig.Hostname - origUseHostIP := GoldpingerConfig.UseHostIP - GoldpingerConfig.Hostname = "test-instance" - GoldpingerConfig.UseHostIP = false - defer func() { - GoldpingerConfig.Hostname = origHostname - GoldpingerConfig.UseHostIP = origUseHostIP - }() - - hostIP := "10.0.0.1" - podIP := "10.0.0.2" - - // Populate all per-peer UDP metrics so they have label values - SetPeerLossPct(hostIP, podIP, 5.0) - SetPeerHopCount(hostIP, podIP, 2) - ObservePeerUDPRtt(hostIP, podIP, 0.001) - CountUDPDuplicates(hostIP, podIP, 1) - CountUDPOutOfOrder(hostIP, podIP, 1) - CountUDPError(podIP) // UseHostIP=false so target=podIP - - // Verify they exist before cleanup - perPeerCollectors := map[string]prometheus.Collector{ - "goldpinger_peers_loss_pct": goldpingerPeersLossPct, - "goldpinger_peers_hop_count": goldpingerPeersHopCount, - "goldpinger_peers_udp_rtt_s": goldpingerPeersUDPRtt, - "goldpinger_udp_duplicates_total": goldpingerUDPDuplicatesCounter, - "goldpinger_udp_out_of_order_total": goldpingerUDPOutOfOrderCounter, - "goldpinger_udp_errors_total": goldpingerUDPErrorsCounter, + tests := []struct { + name string + hostIP string + podIP string + }{ + {"IPv4", "10.0.0.1", "10.0.0.2"}, + {"IPv6", "2001:db8::1", "2001:db8:1::2"}, } - for name, collector := range perPeerCollectors { - if countMetrics(collector) == 0 { - t.Fatalf("metric %s has no label values before cleanup — test setup is broken", name) - } - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + origHostname := GoldpingerConfig.Hostname + origUseHostIP := GoldpingerConfig.UseHostIP + GoldpingerConfig.Hostname = "test-instance" + GoldpingerConfig.UseHostIP = false + defer func() { + GoldpingerConfig.Hostname = origHostname + GoldpingerConfig.UseHostIP = origUseHostIP + }() - // Run cleanup - DeletePeerUDPMetrics(hostIP, podIP) + // Populate all per-peer UDP metrics so they have label values + SetPeerLossPct(tt.hostIP, tt.podIP, 5.0) + SetPeerHopCount(tt.hostIP, tt.podIP, 2) + ObservePeerUDPRtt(tt.hostIP, tt.podIP, 0.001) + CountUDPDuplicates(tt.hostIP, tt.podIP, 1) + CountUDPOutOfOrder(tt.hostIP, tt.podIP, 1) + CountUDPError(tt.podIP) // UseHostIP=false so target=podIP - // Verify all per-peer metrics are cleaned up - for name, collector := range perPeerCollectors { - if n := countMetrics(collector); n != 0 { - t.Errorf("metric %s still has %d label set(s) after DeletePeerUDPMetrics — add it to the cleanup function", name, n) - } + // Verify they exist before cleanup + perPeerCollectors := map[string]prometheus.Collector{ + "goldpinger_peers_loss_pct": goldpingerPeersLossPct, + "goldpinger_peers_hop_count": goldpingerPeersHopCount, + "goldpinger_peers_udp_rtt_s": goldpingerPeersUDPRtt, + "goldpinger_udp_duplicates_total": goldpingerUDPDuplicatesCounter, + "goldpinger_udp_out_of_order_total": goldpingerUDPOutOfOrderCounter, + "goldpinger_udp_errors_total": goldpingerUDPErrorsCounter, + } + + for name, collector := range perPeerCollectors { + if countMetrics(collector) == 0 { + t.Fatalf("metric %s has no label values before cleanup — test setup is broken", name) + } + } + + // Run cleanup + DeletePeerUDPMetrics(tt.hostIP, tt.podIP) + + // Verify all per-peer metrics are cleaned up + for name, collector := range perPeerCollectors { + if n := countMetrics(collector); n != 0 { + t.Errorf("metric %s still has %d label set(s) after DeletePeerUDPMetrics — add it to the cleanup function", name, n) + } + } + }) } }