From d14472d400e63b9b8a30ad7308f366b4adef52c1 Mon Sep 17 00:00:00 2001 From: Matthew Mattox Date: Sat, 4 Jul 2026 10:23:48 -0500 Subject: [PATCH] feat(dns,cni): derive cluster domain from resolv.conf (241/242) + topology-aware peer latency (245) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 241/242 — DNS monitor now derives the in-cluster probe target from /etc/resolv.conf 'search' domains (kubernetes.default.svc.) instead of hardcoding cluster.local. A hardcoded cluster.local produces a fleet-wide false ClusterDNSResolutionFailed on clusters with a CUSTOM cluster domain (the target does not resolve) — the root cause of the original incident. Derivation is the DEFAULT only when clusterDomains is nil; an explicit empty slice (cluster DNS disabled, the current live mitigation) is left untouched, so this change does NOT re-enable the check on the fleet — re-enabling stays a config decision. 242's pod-network intent is satisfied by probing the derived domain through the cluster resolver (node-doctor uses ClusterFirstWithHostNet). 245 — peer latency is now topology-aware. Discovery tags each peer with its node's topology.kubernetes.io/zone label and computes SameZone relative to this node; cross-zone peers use CrossZoneWarningLatency/CrossZoneCriticalLatency when configured, so normal inter-site WAN latency does not raise a false NetworkDegraded. Inert by default: with no zone labels OR no cross-zone thresholds set, behaviour is unchanged. Tests: cluster-domain derivation (standard/custom/fallback/disabled-stays-disabled), resolv.conf parsing, topology threshold selection + config parsing, discovery zone population (labeled cross/same-zone and unlabeled inert path). Chart RBAC already grants nodes list. Tasks: #19560/#19561 (241/242), #19564 (245). NOTE: DNS cluster-DNS re-enable is a GATED deploy decision (re-enables the incident check) — code shipped, roll deferred. --- pkg/monitors/network/cni.go | 44 +++++- pkg/monitors/network/cni_topology_test.go | 57 ++++++++ pkg/monitors/network/dns.go | 92 ++++++++++++- .../network/dns_clusterdomain_test.go | 130 ++++++++++++++++++ pkg/monitors/network/peer_discovery.go | 45 ++++++ .../network/peer_discovery_zone_test.go | 88 ++++++++++++ 6 files changed, 443 insertions(+), 13 deletions(-) create mode 100644 pkg/monitors/network/cni_topology_test.go create mode 100644 pkg/monitors/network/dns_clusterdomain_test.go create mode 100644 pkg/monitors/network/peer_discovery_zone_test.go diff --git a/pkg/monitors/network/cni.go b/pkg/monitors/network/cni.go index d9a0c8c..d4e03c2 100644 --- a/pkg/monitors/network/cni.go +++ b/pkg/monitors/network/cni.go @@ -67,10 +67,17 @@ type ConnectivityConfig struct { PingCount int // PingTimeout is the timeout for each ping. PingTimeout time.Duration - // WarningLatency is the latency threshold for warnings. + // WarningLatency is the latency threshold for warnings (same-zone peers). WarningLatency time.Duration - // CriticalLatency is the latency threshold for critical conditions. + // CriticalLatency is the latency threshold for critical conditions (same-zone peers). CriticalLatency time.Duration + // CrossZoneWarningLatency/CrossZoneCriticalLatency are the latency thresholds applied + // to peers in a DIFFERENT topology zone (e.g. cross-site nodes). When 0 (unset), the + // same-zone thresholds above are used for all peers — so topology awareness is inert + // until explicitly configured. Set these looser than the same-zone thresholds so normal + // inter-site WAN latency does not raise a false NetworkDegraded. + CrossZoneWarningLatency time.Duration + CrossZoneCriticalLatency time.Duration // FailureThreshold is consecutive failures before marking peer unreachable. FailureThreshold int // MinReachablePeers is the percentage of peers that must be reachable. @@ -363,6 +370,20 @@ func parseCNIConfig(configMap map[string]interface{}) (*CNIMonitorConfig, error) } config.Connectivity.CriticalLatency = duration } + if v, ok := connMap["crossZoneWarningLatency"]; ok { + duration, err := parseDuration(v) + if err != nil { + return nil, fmt.Errorf("invalid connectivity.crossZoneWarningLatency: %w", err) + } + config.Connectivity.CrossZoneWarningLatency = duration + } + if v, ok := connMap["crossZoneCriticalLatency"]; ok { + duration, err := parseDuration(v) + if err != nil { + return nil, fmt.Errorf("invalid connectivity.crossZoneCriticalLatency: %w", err) + } + config.Connectivity.CrossZoneCriticalLatency = duration + } if failureThreshold, ok := connMap["failureThreshold"]; ok { switch v := failureThreshold.(type) { case int: @@ -425,6 +446,17 @@ func ValidateCNIConfig(config types.MonitorConfig) error { } // checkCNI performs the CNI connectivity check. +// warningLatencyFor returns the warning-latency threshold to apply to a peer. A peer +// in a different topology zone uses CrossZoneWarningLatency when it is configured +// (non-zero); otherwise the same-zone WarningLatency is used. This keeps topology +// awareness inert until both zone labels exist AND a cross-zone threshold is set. +func (m *CNIMonitor) warningLatencyFor(peer Peer) time.Duration { + if !peer.SameZone && m.config.Connectivity.CrossZoneWarningLatency > 0 { + return m.config.Connectivity.CrossZoneWarningLatency + } + return m.config.Connectivity.WarningLatency +} + func (m *CNIMonitor) checkCNI(ctx context.Context) (*types.Status, error) { status := types.NewStatus(m.name) @@ -490,10 +522,10 @@ func (m *CNIMonitor) checkCNI(ctx context.Context) (*types.Status, error) { reachableCount++ totalLatency += peerStatus.AvgLatency - // Check for high latency (collect but don't emit individual events) - if peerStatus.AvgLatency > m.config.Connectivity.CriticalLatency { - highLatencyPeers = append(highLatencyPeers, fmt.Sprintf("%s (%.2fms)", result.peer.NodeName, float64(peerStatus.AvgLatency)/float64(time.Millisecond))) - } else if peerStatus.AvgLatency > m.config.Connectivity.WarningLatency { + // Check for high latency (collect but don't emit individual events). + // Cross-zone peers use the looser cross-zone warning threshold when configured, + // so normal inter-site WAN latency does not raise a false NetworkDegraded. + if peerStatus.AvgLatency > m.warningLatencyFor(result.peer) { highLatencyPeers = append(highLatencyPeers, fmt.Sprintf("%s (%.2fms)", result.peer.NodeName, float64(peerStatus.AvgLatency)/float64(time.Millisecond))) } } else { diff --git a/pkg/monitors/network/cni_topology_test.go b/pkg/monitors/network/cni_topology_test.go new file mode 100644 index 0000000..481cf64 --- /dev/null +++ b/pkg/monitors/network/cni_topology_test.go @@ -0,0 +1,57 @@ +package network + +import ( + "testing" + "time" +) + +func TestWarningLatencyFor(t *testing.T) { + mk := func(warn, crossWarn time.Duration) *CNIMonitor { + return &CNIMonitor{config: &CNIMonitorConfig{ + Connectivity: ConnectivityConfig{ + WarningLatency: warn, + CrossZoneWarningLatency: crossWarn, + }, + }} + } + sameZone := Peer{NodeName: "a", SameZone: true} + crossZone := Peer{NodeName: "b", SameZone: false} + + // Cross-zone threshold configured: same-zone peer uses tight, cross-zone uses loose. + m := mk(200*time.Millisecond, 800*time.Millisecond) + if got := m.warningLatencyFor(sameZone); got != 200*time.Millisecond { + t.Errorf("same-zone threshold = %v, want 200ms", got) + } + if got := m.warningLatencyFor(crossZone); got != 800*time.Millisecond { + t.Errorf("cross-zone threshold = %v, want 800ms", got) + } + + // Cross-zone threshold unset (0): topology awareness is inert — both use WarningLatency. + inert := mk(200*time.Millisecond, 0) + if got := inert.warningLatencyFor(crossZone); got != 200*time.Millisecond { + t.Errorf("inert cross-zone threshold = %v, want 200ms (same as warning)", got) + } + if got := inert.warningLatencyFor(sameZone); got != 200*time.Millisecond { + t.Errorf("inert same-zone threshold = %v, want 200ms", got) + } +} + +func TestConnectivityCrossZoneLatencyParsing(t *testing.T) { + cfg, err := parseCNIConfig(map[string]interface{}{ + "connectivity": map[string]interface{}{ + "warningLatency": "200ms", + "criticalLatency": "500ms", + "crossZoneWarningLatency": "1s", + "crossZoneCriticalLatency": "2s", + }, + }) + if err != nil { + t.Fatalf("parseCNIConfig: %v", err) + } + if cfg.Connectivity.CrossZoneWarningLatency != time.Second { + t.Errorf("crossZoneWarningLatency = %v, want 1s", cfg.Connectivity.CrossZoneWarningLatency) + } + if cfg.Connectivity.CrossZoneCriticalLatency != 2*time.Second { + t.Errorf("crossZoneCriticalLatency = %v, want 2s", cfg.Connectivity.CrossZoneCriticalLatency) + } +} diff --git a/pkg/monitors/network/dns.go b/pkg/monitors/network/dns.go index 8234024..71cf409 100644 --- a/pkg/monitors/network/dns.go +++ b/pkg/monitors/network/dns.go @@ -1232,9 +1232,23 @@ func parseDNSConfig(configMap map[string]interface{}) (*DNSMonitorConfig, error) // //nolint:gocyclo // config defaults are inherently branchy across many optional fields func (c *DNSMonitorConfig) applyDefaults() error { - // Default cluster domains - only apply if not explicitly set (nil vs empty slice) + // Default resolver path first — cluster-domain derivation reads it below. + if c.ResolverPath == "" { + c.ResolverPath = "/etc/resolv.conf" + } + + // Default cluster domains - only apply if not explicitly set (nil vs empty slice). + // An explicit empty slice ([]) means "cluster DNS check disabled" and is left as-is; + // only a nil (unset) value gets a default. + // + // The default is DERIVED from the resolver's search domains rather than hardcoded to + // "kubernetes.default.svc.cluster.local". A hardcoded cluster.local produces a + // fleet-wide false ClusterDNSResolutionFailed on clusters that use a CUSTOM cluster + // domain (the target simply does not resolve). Derivation reads the real domain from + // /etc/resolv.conf `search` and builds the correct kubernetes.default.svc., + // falling back to cluster.local only when derivation is not possible. if c.ClusterDomains == nil { - c.ClusterDomains = []string{"kubernetes.default.svc.cluster.local"} + c.ClusterDomains = defaultClusterDomains(c.ResolverPath) } // Default external domains - only apply if not explicitly set (nil vs empty slice) @@ -1247,11 +1261,6 @@ func (c *DNSMonitorConfig) applyDefaults() error { c.LatencyThreshold = 1 * time.Second } - // Default nameserver check enabled - if c.ResolverPath == "" { - c.ResolverPath = "/etc/resolv.conf" - } - // Default failure count threshold if c.FailureCountThreshold == 0 { c.FailureCountThreshold = 3 @@ -2338,6 +2347,75 @@ func (m *DNSMonitor) parseResolverConfig() ([]string, error) { return nameservers, nil } +// defaultClusterDomains returns the default in-cluster DNS probe target(s). It derives +// the cluster domain from the resolver's search domains and builds +// "kubernetes.default.svc.", falling back to the well-known +// "kubernetes.default.svc.cluster.local" when derivation is not possible. This prevents +// the false ClusterDNSResolutionFailed that a hardcoded cluster.local causes on clusters +// with a custom cluster domain. +func defaultClusterDomains(resolverPath string) []string { + if domain, ok := deriveClusterDomainFromResolver(resolverPath); ok { + return []string{"kubernetes.default.svc." + domain} + } + return []string{"kubernetes.default.svc.cluster.local"} +} + +// deriveClusterDomainFromResolver reads resolverPath and extracts the Kubernetes +// cluster domain from its `search` line. Returns ("", false) if the file can't be read +// or no cluster domain can be identified. +func deriveClusterDomainFromResolver(resolverPath string) (string, bool) { + file, err := os.Open(resolverPath) + if err != nil { + return "", false + } + defer file.Close() + + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + if strings.HasPrefix(line, "search") { + fields := strings.Fields(line) + if len(fields) >= 2 { + return deriveClusterDomain(fields[1:]) + } + } + } + if err := scanner.Err(); err != nil { + return "", false + } + return "", false +} + +// deriveClusterDomain extracts the cluster domain from a list of resolver search +// domains. A Kubernetes pod's search list looks like: +// +// .svc. svc. +// +// so the cluster domain is the suffix after the "svc." label. It prefers the canonical +// "svc." entry, then any ".svc." entry. Returns +// ("", false) when no svc-scoped search domain is present (e.g. a non-Kubernetes +// resolver), so the caller can fall back rather than probe a bogus target. +func deriveClusterDomain(searchDomains []string) (string, bool) { + // Prefer the canonical middle entry: "svc.". + for _, d := range searchDomains { + d = strings.TrimSuffix(strings.TrimSpace(d), ".") + if strings.HasPrefix(d, "svc.") && len(d) > len("svc.") { + return d[len("svc."):], true + } + } + // Fall back to ".svc.". + for _, d := range searchDomains { + d = strings.TrimSuffix(strings.TrimSpace(d), ".") + if _, domain, found := strings.Cut(d, ".svc."); found && domain != "" { + return domain, true + } + } + return "", false +} + // exclusiveCond describes one member of a mutually-exclusive condition group. type exclusiveCond struct{ Type, Reason, Message string } diff --git a/pkg/monitors/network/dns_clusterdomain_test.go b/pkg/monitors/network/dns_clusterdomain_test.go new file mode 100644 index 0000000..7110946 --- /dev/null +++ b/pkg/monitors/network/dns_clusterdomain_test.go @@ -0,0 +1,130 @@ +package network + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDeriveClusterDomain(t *testing.T) { + tests := []struct { + name string + search []string + want string + wantOK bool + }{ + { + name: "standard cluster.local search list", + search: []string{"default.svc.cluster.local", "svc.cluster.local", "cluster.local"}, + want: "cluster.local", + wantOK: true, + }, + { + name: "custom cluster domain (the incident case)", + search: []string{"default.svc.k8s.example.com", "svc.k8s.example.com", "k8s.example.com"}, + want: "k8s.example.com", + wantOK: true, + }, + { + name: "only the ns-scoped entry present (no bare svc.)", + search: []string{"kube-system.svc.cluster.local"}, + want: "cluster.local", + wantOK: true, + }, + { + name: "trailing dots tolerated", + search: []string{"svc.cluster.local."}, + want: "cluster.local", + wantOK: true, + }, + { + name: "non-kubernetes resolver -> no derivation", + search: []string{"corp.example.com", "example.com"}, + want: "", + wantOK: false, + }, + { + name: "empty search", + search: nil, + want: "", + wantOK: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := deriveClusterDomain(tt.search) + if got != tt.want || ok != tt.wantOK { + t.Errorf("deriveClusterDomain(%v) = (%q, %v), want (%q, %v)", tt.search, got, ok, tt.want, tt.wantOK) + } + }) + } +} + +func writeResolv(t *testing.T, content string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "resolv.conf") + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatalf("write resolv.conf: %v", err) + } + return p +} + +func TestDeriveClusterDomainFromResolver(t *testing.T) { + custom := writeResolv(t, "search default.svc.k8s.example.com svc.k8s.example.com k8s.example.com\nnameserver 10.43.0.10\noptions ndots:5\n") + if d, ok := deriveClusterDomainFromResolver(custom); !ok || d != "k8s.example.com" { + t.Errorf("custom domain: got (%q,%v), want (k8s.example.com,true)", d, ok) + } + + // Missing file -> no derivation, no panic. + if d, ok := deriveClusterDomainFromResolver(filepath.Join(t.TempDir(), "nope")); ok || d != "" { + t.Errorf("missing file: got (%q,%v), want ('',false)", d, ok) + } + + // No search line -> no derivation. + noSearch := writeResolv(t, "nameserver 1.1.1.1\n") + if d, ok := deriveClusterDomainFromResolver(noSearch); ok || d != "" { + t.Errorf("no search line: got (%q,%v), want ('',false)", d, ok) + } +} + +func TestDefaultClusterDomains(t *testing.T) { + // Custom-domain cluster: derived target must use the real domain, NOT cluster.local. + custom := writeResolv(t, "search default.svc.mesh.internal svc.mesh.internal mesh.internal\nnameserver 10.96.0.10\n") + got := defaultClusterDomains(custom) + want := "kubernetes.default.svc.mesh.internal" + if len(got) != 1 || got[0] != want { + t.Errorf("defaultClusterDomains(custom) = %v, want [%q]", got, want) + } + + // Non-derivable resolver: fall back to the well-known cluster.local target. + fallback := defaultClusterDomains(filepath.Join(t.TempDir(), "missing")) + if len(fallback) != 1 || fallback[0] != "kubernetes.default.svc.cluster.local" { + t.Errorf("defaultClusterDomains(fallback) = %v, want [kubernetes.default.svc.cluster.local]", fallback) + } +} + +// TestApplyDefaultsDerivesClusterDomain verifies the config wiring: a nil ClusterDomains +// gets the derived default, while an explicit empty slice (cluster DNS disabled) is left +// untouched — the property that keeps this change from re-enabling the check on a fleet +// that intentionally set clusterDomains: []. +func TestApplyDefaultsDerivesClusterDomain(t *testing.T) { + resolv := writeResolv(t, "search default.svc.custom.zone svc.custom.zone custom.zone\nnameserver 10.96.0.10\n") + + // nil -> derived + c := &DNSMonitorConfig{ResolverPath: resolv} + if err := c.applyDefaults(); err != nil { + t.Fatalf("applyDefaults: %v", err) + } + if len(c.ClusterDomains) != 1 || c.ClusterDomains[0] != "kubernetes.default.svc.custom.zone" { + t.Errorf("nil ClusterDomains derived = %v, want [kubernetes.default.svc.custom.zone]", c.ClusterDomains) + } + + // explicit empty slice -> left disabled (NOT re-enabled) + disabled := &DNSMonitorConfig{ResolverPath: resolv, ClusterDomains: []string{}} + if err := disabled.applyDefaults(); err != nil { + t.Fatalf("applyDefaults: %v", err) + } + if len(disabled.ClusterDomains) != 0 { + t.Errorf("explicit empty ClusterDomains should stay empty, got %v", disabled.ClusterDomains) + } +} diff --git a/pkg/monitors/network/peer_discovery.go b/pkg/monitors/network/peer_discovery.go index 3688619..a3c327c 100644 --- a/pkg/monitors/network/peer_discovery.go +++ b/pkg/monitors/network/peer_discovery.go @@ -25,6 +25,13 @@ type Peer struct { NodeIP string // PodIP is the pod IP (same as NodeIP when using hostNetwork). PodIP string + // Zone is the peer node's topology.kubernetes.io/zone label (empty if unlabeled). + Zone string + // SameZone is true when the peer is in the same topology zone as this node (or when + // zone labels are absent on either side). It drives topology-aware latency thresholds: + // a peer in a different zone (e.g. a cross-site node) can be held to a looser latency + // threshold so normal WAN latency does not raise a false NetworkDegraded. + SameZone bool // LastSeen is when this peer was last seen in discovery. LastSeen time.Time } @@ -230,6 +237,13 @@ func (d *kubernetesPeerDiscovery) discoverPeers(ctx context.Context) ([]Peer, er return nil, fmt.Errorf("failed to list pods: %w", err) } + // Build a nodeName -> zone map for topology-aware latency thresholds. This is + // best-effort: if the node list fails (e.g. missing RBAC) or nodes are unlabeled, + // zones stay empty and every peer is treated as same-zone (tight threshold), which + // preserves the pre-topology behaviour. + zoneByNode := d.nodeZones(ctx) + selfZone := zoneByNode[d.config.SelfNodeName] + peers := make([]Peer, 0, len(pods.Items)) now := time.Now() @@ -250,11 +264,19 @@ func (d *kubernetesPeerDiscovery) discoverPeers(ctx context.Context) ([]Peer, er continue // Skip pods without a valid node IP } + peerZone := zoneByNode[pod.Spec.NodeName] peer := Peer{ Name: pod.Name, NodeName: pod.Spec.NodeName, NodeIP: nodeIP, PodIP: pod.Status.PodIP, + Zone: peerZone, + // Same zone when the labels match. When either side is unlabeled the zones + // compare equal only if BOTH are empty, so an all-unlabeled cluster keeps + // SameZone=true everywhere (unchanged behaviour); a partially-labeled cluster + // treats unknown-zone peers as cross-zone (looser threshold), which is the safe + // direction (fewer false NetworkDegraded). + SameZone: peerZone == selfZone, LastSeen: now, } @@ -264,6 +286,29 @@ func (d *kubernetesPeerDiscovery) discoverPeers(ctx context.Context) ([]Peer, er return peers, nil } +// nodeZones returns a map of node name -> topology.kubernetes.io/zone label. It is +// best-effort: on any error it returns an empty (non-nil) map so callers treat all +// peers as same-zone. The legacy failure-domain.beta.kubernetes.io/zone label is used +// as a fallback for older clusters. +func (d *kubernetesPeerDiscovery) nodeZones(ctx context.Context) map[string]string { + out := make(map[string]string) + nodes, err := d.client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + return out + } + for i := range nodes.Items { + n := &nodes.Items[i] + zone := n.Labels["topology.kubernetes.io/zone"] + if zone == "" { + zone = n.Labels["failure-domain.beta.kubernetes.io/zone"] + } + if zone != "" { + out[n.Name] = zone + } + } + return out +} + // getNodeIP extracts the node IP from a pod. // For hostNetwork pods, this is the host IP. func getNodeIP(pod *corev1.Pod) string { diff --git a/pkg/monitors/network/peer_discovery_zone_test.go b/pkg/monitors/network/peer_discovery_zone_test.go new file mode 100644 index 0000000..4a56dab --- /dev/null +++ b/pkg/monitors/network/peer_discovery_zone_test.go @@ -0,0 +1,88 @@ +package network + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func zoneNode(name, zone string) *corev1.Node { + return &corev1.Node{ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{"topology.kubernetes.io/zone": zone}, + }} +} + +func runningPodOn(name, node, hostIP string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "node-doctor", Labels: map[string]string{"app": "node-doctor"}}, + Spec: corev1.PodSpec{NodeName: node}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, HostIP: hostIP, PodIP: hostIP}, + } +} + +// TestDiscoverPeersPopulatesZone verifies peers are tagged with their node zone and +// SameZone is computed relative to the local node, driving topology-aware thresholds. +func TestDiscoverPeersPopulatesZone(t *testing.T) { + client := fake.NewSimpleClientset( + zoneNode("self", "site-a"), + zoneNode("peer-same", "site-a"), + zoneNode("peer-cross", "site-b"), + runningPodOn("nd-self", "self", "10.0.0.1"), + runningPodOn("nd-same", "peer-same", "10.0.0.2"), + runningPodOn("nd-cross", "peer-cross", "10.1.0.3"), + ) + pd, err := NewKubernetesPeerDiscoveryWithClient(&PeerDiscoveryConfig{ + Namespace: "node-doctor", + LabelSelector: "app=node-doctor", + SelfNodeName: "self", + }, client) + if err != nil { + t.Fatalf("NewKubernetesPeerDiscoveryWithClient: %v", err) + } + if err := pd.Refresh(context.Background()); err != nil { + t.Fatalf("Refresh: %v", err) + } + peers := pd.GetPeers() + byNode := map[string]Peer{} + for _, p := range peers { + byNode[p.NodeName] = p + } + if len(peers) != 2 { // self excluded + t.Fatalf("got %d peers, want 2: %+v", len(peers), peers) + } + if p := byNode["peer-same"]; p.Zone != "site-a" || !p.SameZone { + t.Errorf("peer-same = zone %q sameZone %v, want site-a/true", p.Zone, p.SameZone) + } + if p := byNode["peer-cross"]; p.Zone != "site-b" || p.SameZone { + t.Errorf("peer-cross = zone %q sameZone %v, want site-b/false", p.Zone, p.SameZone) + } +} + +// TestDiscoverPeersUnlabeledIsSameZone verifies the inert path: with no zone labels, +// every peer is same-zone (unchanged pre-topology behaviour). +func TestDiscoverPeersUnlabeledIsSameZone(t *testing.T) { + client := fake.NewSimpleClientset( + runningPodOn("nd-self", "self", "10.0.0.1"), + runningPodOn("nd-a", "node-a", "10.0.0.2"), + ) + pd, err := NewKubernetesPeerDiscoveryWithClient(&PeerDiscoveryConfig{ + Namespace: "node-doctor", + LabelSelector: "app=node-doctor", + SelfNodeName: "self", + }, client) + if err != nil { + t.Fatalf("NewKubernetesPeerDiscoveryWithClient: %v", err) + } + if err := pd.Refresh(context.Background()); err != nil { + t.Fatalf("Refresh: %v", err) + } + for _, p := range pd.GetPeers() { + if p.Zone != "" || !p.SameZone { + t.Errorf("unlabeled peer %s = zone %q sameZone %v, want ''/true", p.NodeName, p.Zone, p.SameZone) + } + } +}