From 6888c6c2606b121bf7ef16cc3156a746aa3d985b Mon Sep 17 00:00:00 2001 From: Matthew Mattox Date: Sat, 4 Jul 2026 11:47:32 -0500 Subject: [PATCH] feat(dns): pod-network cluster-DNS probe via overlay-test pods (task 242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit node-doctor runs hostNetwork; from the host netns the kube-dns ClusterIP does NOT resolve CLUSTER records (Cilium host-netns->ClusterIP NXDOMAINs any cluster domain, even the correct one — verified on a1-ops-prd), so the in-agent cluster-DNS check cannot work and stays disabled (clusterDomains: []). This adds the correct path: - pkg/clusterdns: shared cluster-domain derivation (moved out of dns.go) + Probe() that resolves kubernetes.default.svc. via net.Resolver. - overlay-test-server: new /clusterdns endpoint runs Probe() in the pod's own (pod-network) context, where cluster records DO resolve, returning JSON. - new monitor network-cluster-dns-pod: discovers overlay-test pods, HTTP-probes their /clusterdns (node-doctor CAN reach pod IPs from host netns, just not the ClusterIP), and drives ClusterDNSDown from pod-sourced truth with DNSMonitor-style consecutive- failure latching. No-peers cycle leaves the counter untouched (no false positive). - chart: gated behind clusterDnsPodProbe.enabled (default FALSE) so it ships inert. Requires an overlay-test image serving /clusterdns (this release); enabling against an older overlay-test image would 404 every probe -> false ClusterDNSDown. Tests: clusterdns derivation/probe (hermetic), monitor all-resolved/all-fail-threshold/ partial-min-success/no-peers via fake clientset + httptest. Task #19561 (242). Re-enable (flip clusterDnsPodProbe.enabled + roll new overlay-test image) is a watched- window decision. --- cmd/overlay-test-server/main.go | 14 +- helm/node-doctor/templates/configmap.yaml | 23 + helm/node-doctor/values.yaml.template | 20 + pkg/clusterdns/clusterdns.go | 116 +++++ pkg/clusterdns/clusterdns_test.go | 137 ++++++ pkg/monitors/network/clusterdnspod.go | 446 ++++++++++++++++++ pkg/monitors/network/clusterdnspod_test.go | 209 ++++++++ pkg/monitors/network/dns.go | 71 +-- .../network/dns_clusterdomain_test.go | 72 --- 9 files changed, 969 insertions(+), 139 deletions(-) create mode 100644 pkg/clusterdns/clusterdns.go create mode 100644 pkg/clusterdns/clusterdns_test.go create mode 100644 pkg/monitors/network/clusterdnspod.go create mode 100644 pkg/monitors/network/clusterdnspod_test.go diff --git a/cmd/overlay-test-server/main.go b/cmd/overlay-test-server/main.go index 8489cb1..cdd2bad 100644 --- a/cmd/overlay-test-server/main.go +++ b/cmd/overlay-test-server/main.go @@ -1,6 +1,9 @@ // Package main implements a minimal HTTP health server for overlay-test pods. -// It serves /healthz and returns JSON with pod metadata, enabling HTTP-based +// It serves /healthz, which returns JSON with pod metadata, enabling HTTP-based // connectivity probing that works on Cilium clusters where ICMP is silently dropped. +// It also serves /clusterdns, which resolves cluster DNS from pod-network context, +// where node-doctor's host-network context cannot (Cilium doesn't route +// host-netns->ClusterIP for cluster records). package main import ( @@ -10,6 +13,8 @@ import ( "log" "net/http" "os" + + "github.com/supporttools/node-doctor/pkg/clusterdns" ) func main() { @@ -30,6 +35,13 @@ func main() { json.NewEncoder(w).Encode(resp) //nolint:errcheck // best-effort response }) + http.HandleFunc("/clusterdns", func(w http.ResponseWriter, r *http.Request) { + result := clusterdns.Probe(r.Context(), "/etc/resolv.conf") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(result) //nolint:errcheck // best-effort response + }) + addr := fmt.Sprintf(":%d", *port) log.Printf("overlay-test-server listening on %s (node=%s, podIP=%s)", addr, nodeName, podIP) if err := http.ListenAndServe(addr, nil); err != nil { //nolint:gosec // intentionally binds all interfaces for intra-cluster probing diff --git a/helm/node-doctor/templates/configmap.yaml b/helm/node-doctor/templates/configmap.yaml index 8e569c4..f7b7dca 100644 --- a/helm/node-doctor/templates/configmap.yaml +++ b/helm/node-doctor/templates/configmap.yaml @@ -200,6 +200,29 @@ data: checkNameservers: true failureCountThreshold: 3 enableNameserverChecks: true + {{- if .Values.clusterDnsPodProbe.enabled }} + + # Cluster DNS via pod-network (overlay-test pods). node-doctor runs hostNetwork, + # from which the kube-dns ClusterIP does NOT resolve CLUSTER records (Cilium + # host-netns->ClusterIP), so the in-agent cluster-DNS check above stays disabled + # (clusterDomains: []). This monitor instead queries the overlay-test pods' + # /clusterdns endpoint — they run in pod-network and DO resolve cluster records — + # and drives ClusterDNSDown from that pod-sourced truth. Requires an overlay-test + # image that serves /clusterdns (v1.8.3+); enabling it against an older overlay-test + # image makes every probe 404 and falsely reports ClusterDNSDown. + - name: cluster-dns-pod + type: network-cluster-dns-pod + enabled: true + interval: {{ .Values.clusterDnsPodProbe.interval }} + timeout: {{ .Values.clusterDnsPodProbe.timeout }} + config: + labelSelector: {{ .Values.clusterDnsPodProbe.labelSelector | quote }} + namespace: {{ .Release.Namespace }} + probePort: {{ .Values.clusterDnsPodProbe.probePort }} + probePath: {{ .Values.clusterDnsPodProbe.probePath | quote }} + minSuccessPods: {{ .Values.clusterDnsPodProbe.minSuccessPods }} + failureCountThreshold: {{ .Values.clusterDnsPodProbe.failureCountThreshold }} + {{- end }} exporters: kubernetes: diff --git a/helm/node-doctor/values.yaml.template b/helm/node-doctor/values.yaml.template index 8e80b85..22aee9f 100644 --- a/helm/node-doctor/values.yaml.template +++ b/helm/node-doctor/values.yaml.template @@ -215,6 +215,26 @@ prometheusRule: noMetrics: for: 10m +# Cluster DNS via pod-network probe (task 242). +# DISABLED by default. node-doctor runs hostNetwork and cannot resolve cluster DNS +# ClusterIP records (Cilium host-netns->ClusterIP), so the in-agent cluster-DNS check +# is off (dns-health clusterDomains: []). When enabled, this adds a monitor that queries +# the overlay-test pods' /clusterdns endpoint (pod-network, which DOES resolve cluster +# records) and drives ClusterDNSDown from that. +# PREREQUISITE: the overlay-test image must serve /clusterdns (v1.8.3+). Enabling this +# against an older overlay-test image makes every probe 404 -> false ClusterDNSDown. +clusterDnsPodProbe: + enabled: false + interval: 30s + timeout: 5s + labelSelector: "app=node-doctor-overlay-test" + probePort: 8023 + probePath: "/clusterdns" + # Number of overlay-test pods that must resolve cluster DNS for it to be healthy. + minSuccessPods: 1 + # Consecutive failed cycles before ClusterDNSDown latches True. + failureCountThreshold: 3 + # Health probe configuration. # Probes default to `exec` running the binary's built-in health check, which talks # to a per-pod unix socket (/var/run/node-doctor/health.sock) instead of TCP :8080. diff --git a/pkg/clusterdns/clusterdns.go b/pkg/clusterdns/clusterdns.go new file mode 100644 index 0000000..f4f5be8 --- /dev/null +++ b/pkg/clusterdns/clusterdns.go @@ -0,0 +1,116 @@ +// Package clusterdns derives the Kubernetes cluster-DNS probe target from resolver +// configuration and performs cluster-DNS resolution probes. It is shared by the DNS +// health monitor (pkg/monitors/network) and the overlay-test-server, which resolves +// cluster DNS from pod-network context where node-doctor's host-network context cannot. +package clusterdns + +import ( + "bufio" + "context" + "net" + "os" + "strings" + "time" +) + +// ClusterProbeName returns the default in-cluster DNS probe target. 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 ClusterProbeName(resolverPath string) string { + if domain, ok := DeriveClusterDomainFromResolver(resolverPath); ok { + return "kubernetes.default.svc." + domain + } + return "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 +} + +// ProbeResult is the outcome of a single cluster-DNS resolution probe. +type ProbeResult struct { + Target string `json:"target"` + Resolved bool `json:"resolved"` + Addresses []string `json:"addresses,omitempty"` + LatencyMs float64 `json:"latencyMs"` + Error string `json:"error,omitempty"` +} + +// Probe resolves the cluster-DNS probe target (derived from resolverPath via +// ClusterProbeName) using the system resolver and reports the outcome, including +// resolution latency. It is used both by the DNS health monitor and by the +// overlay-test-server's /clusterdns endpoint, which runs in pod-network context where +// cluster DNS resolution actually works. +func Probe(ctx context.Context, resolverPath string) ProbeResult { + target := ClusterProbeName(resolverPath) + result := ProbeResult{Target: target} + + start := time.Now() + addrs, err := (&net.Resolver{}).LookupHost(ctx, target) + result.LatencyMs = float64(time.Since(start).Microseconds()) / 1000.0 + + if err != nil { + result.Resolved = false + result.Error = err.Error() + return result + } + + result.Resolved = true + result.Addresses = addrs + return result +} diff --git a/pkg/clusterdns/clusterdns_test.go b/pkg/clusterdns/clusterdns_test.go new file mode 100644 index 0000000..dbaaf9d --- /dev/null +++ b/pkg/clusterdns/clusterdns_test.go @@ -0,0 +1,137 @@ +package clusterdns + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" +) + +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 TestClusterProbeName(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 := ClusterProbeName(custom) + want := "kubernetes.default.svc.mesh.internal" + if got != want { + t.Errorf("ClusterProbeName(custom) = %q, want %q", got, want) + } + + // Non-derivable resolver: fall back to the well-known cluster.local target. + fallback := ClusterProbeName(filepath.Join(t.TempDir(), "missing")) + if fallback != "kubernetes.default.svc.cluster.local" { + t.Errorf("ClusterProbeName(fallback) = %q, want kubernetes.default.svc.cluster.local", fallback) + } +} + +// TestProbe is hermetic: it does not depend on real network access succeeding, since +// the test sandbox likely has no DNS/network access at all. It only asserts on the +// deterministic field-wiring, branching on whether resolution happened to succeed. +func TestProbe(t *testing.T) { + resolv := writeResolv(t, "search default.svc.probe.test svc.probe.test probe.test\nnameserver 10.96.0.10\n") + wantTarget := "kubernetes.default.svc.probe.test" + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + result := Probe(ctx, resolv) + + if result.Target != wantTarget { + t.Errorf("Probe().Target = %q, want %q", result.Target, wantTarget) + } + if result.LatencyMs < 0 { + t.Errorf("Probe().LatencyMs = %v, want >= 0", result.LatencyMs) + } + + if result.Resolved { + if len(result.Addresses) == 0 { + t.Errorf("Probe() Resolved=true but Addresses is empty") + } + if result.Error != "" { + t.Errorf("Probe() Resolved=true but Error = %q, want empty", result.Error) + } + } else if result.Error == "" { + t.Errorf("Probe() Resolved=false but Error is empty") + } +} diff --git a/pkg/monitors/network/clusterdnspod.go b/pkg/monitors/network/clusterdnspod.go new file mode 100644 index 0000000..c10e7d4 --- /dev/null +++ b/pkg/monitors/network/clusterdnspod.go @@ -0,0 +1,446 @@ +// Package network provides network health monitoring capabilities. +package network + +import ( + "context" + "encoding/json" + "fmt" + "net" + "net/http" + "sort" + "strconv" + "sync" + "time" + + "github.com/supporttools/node-doctor/pkg/clusterdns" + "github.com/supporttools/node-doctor/pkg/monitors" + "github.com/supporttools/node-doctor/pkg/types" +) + +const ( + // clusterDNSPodMonitorType is the registered monitor type string. + clusterDNSPodMonitorType = "network-cluster-dns-pod" + + defaultClusterDNSPodLabelSelector = "app=node-doctor-overlay-test" + defaultClusterDNSPodProbePort = 8023 + defaultClusterDNSPodProbePath = "/clusterdns" + defaultClusterDNSPodTimeout = 5 * time.Second + defaultClusterDNSPodFailureCountThreshold = 3 + defaultClusterDNSPodMinSuccessPods = 1 + + // maxClusterDNSProbePeers caps how many overlay-test pods are probed per cycle, + // bounding load on overlay-test pods when there are many nodes. + maxClusterDNSProbePeers = 5 +) + +// ClusterDNSPodConfig holds the configuration for the cluster-DNS-via-pods monitor. +type ClusterDNSPodConfig struct { + Enabled bool + + // LabelSelector selects the overlay-test pods to probe (default "app=node-doctor-overlay-test"). + LabelSelector string + + // Namespace to search for overlay-test pods (default from getNamespaceFromEnvOrDefault()). + Namespace string + + // ProbePort is the overlay-test-server's HTTP port (default 8023). + ProbePort int + + // ProbePath is the overlay-test-server's cluster-DNS endpoint (default "/clusterdns"). + ProbePath string + + // Timeout bounds each individual per-pod HTTP probe (default 5s). This is + // intentionally separate from the BaseMonitor-level check timeout: it bounds a + // single pod's probe so that one slow/hanging pod doesn't consume the entire + // check-cycle timeout budget. + Timeout time.Duration + + // FailureCountThreshold is the number of consecutive failed cycles before + // reporting ClusterDNSDown (default 3). + FailureCountThreshold int + + // MinSuccessPods is the number of overlay-test pods that must report + // resolved=true in a cycle for cluster DNS to be considered healthy (default 1). + MinSuccessPods int +} + +// applyDefaults fills in unset ClusterDNSPodConfig fields with their default values. +func (c *ClusterDNSPodConfig) applyDefaults() { + if c.LabelSelector == "" { + c.LabelSelector = defaultClusterDNSPodLabelSelector + } + if c.Namespace == "" { + c.Namespace = getNamespaceFromEnvOrDefault() + } + if c.ProbePort == 0 { + c.ProbePort = defaultClusterDNSPodProbePort + } + if c.ProbePath == "" { + c.ProbePath = defaultClusterDNSPodProbePath + } + if c.Timeout == 0 { + c.Timeout = defaultClusterDNSPodTimeout + } + if c.FailureCountThreshold == 0 { + c.FailureCountThreshold = defaultClusterDNSPodFailureCountThreshold + } + if c.MinSuccessPods == 0 { + c.MinSuccessPods = defaultClusterDNSPodMinSuccessPods + } +} + +// parseClusterDNSPodConfig parses the cluster-DNS-pod monitor configuration from a map. +func parseClusterDNSPodConfig(configMap map[string]interface{}) (*ClusterDNSPodConfig, error) { + config := &ClusterDNSPodConfig{} + + if configMap == nil { + return config, nil + } + + if v, ok := configMap["enabled"]; ok { + b, ok := v.(bool) + if !ok { + return nil, fmt.Errorf("enabled must be a boolean") + } + config.Enabled = b + } + + if v, ok := configMap["labelSelector"]; ok { + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("labelSelector must be a string") + } + config.LabelSelector = s + } + + if v, ok := configMap["namespace"]; ok { + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("namespace must be a string") + } + config.Namespace = s + } + + if v, ok := configMap["probePort"]; ok { + switch n := v.(type) { + case float64: + config.ProbePort = int(n) + case int: + config.ProbePort = n + default: + return nil, fmt.Errorf("probePort must be an integer") + } + } + + if v, ok := configMap["probePath"]; ok { + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("probePath must be a string") + } + config.ProbePath = s + } + + if v, ok := configMap["timeout"]; ok { + d, err := parseDuration(v) + if err != nil { + return nil, fmt.Errorf("invalid timeout: %w", err) + } + config.Timeout = d + } + + if v, ok := configMap["failureCountThreshold"]; ok { + switch n := v.(type) { + case float64: + config.FailureCountThreshold = int(n) + case int: + config.FailureCountThreshold = n + default: + return nil, fmt.Errorf("failureCountThreshold must be an integer") + } + } + + if v, ok := configMap["minSuccessPods"]; ok { + switch n := v.(type) { + case float64: + config.MinSuccessPods = int(n) + case int: + config.MinSuccessPods = n + default: + return nil, fmt.Errorf("minSuccessPods must be an integer") + } + } + + return config, nil +} + +// ValidateClusterDNSPodConfig validates the cluster-DNS-pod monitor configuration. +func ValidateClusterDNSPodConfig(config types.MonitorConfig) error { + if config.Name == "" { + return fmt.Errorf("monitor name is required") + } + + if config.Type != clusterDNSPodMonitorType { + return fmt.Errorf("invalid monitor type: expected %s, got %s", clusterDNSPodMonitorType, config.Type) + } + + cfg, err := parseClusterDNSPodConfig(config.Config) + if err != nil { + return fmt.Errorf("failed to parse cluster-dns-pod config: %w", err) + } + cfg.applyDefaults() + + if cfg.FailureCountThreshold < 1 { + return fmt.Errorf("failureCountThreshold must be at least 1, got %d", cfg.FailureCountThreshold) + } + + if cfg.MinSuccessPods < 1 { + return fmt.Errorf("minSuccessPods must be at least 1, got %d", cfg.MinSuccessPods) + } + + if cfg.ProbePort < 1 || cfg.ProbePort > 65535 { + return fmt.Errorf("probePort must be between 1 and 65535, got %d", cfg.ProbePort) + } + + return nil +} + +// clusterDNSPodHTTPClient is the minimal HTTP client interface used by +// ClusterDNSPodMonitor, allowing tests to inject a fake client. +type clusterDNSPodHTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + +// ClusterDNSPodMonitor verifies cluster DNS resolution via overlay-test pods running in +// pod-network context, working around the host-network Cilium ClusterIP routing gap. +// +// PeerDiscovery intentionally does NOT skip same-node pods for this monitor's purposes +// — overlay-test pods run on all nodes; other nodes' overlay-test pods are sufficient +// even though the underlying PeerDiscovery implementation excludes same-node pods via +// SelfNodeName. This is fine because we only need >= MinSuccessPods responses from ANY +// subset of overlay-test pods, not the node-doctor's own node's pod specifically. +type ClusterDNSPodMonitor struct { + name string + config *ClusterDNSPodConfig + discovery PeerDiscovery + httpClient clusterDNSPodHTTPClient + + mu sync.Mutex + consecutiveFailures int + + *monitors.BaseMonitor +} + +// init registers the cluster-DNS-pod monitor with the monitor registry. DefaultConfig +// is intentionally left unset so ApplyDefaultMonitors never auto-enables this monitor, +// keeping it inert/dormant/code-only until explicitly configured. +func init() { + monitors.MustRegister(monitors.MonitorInfo{ + Type: clusterDNSPodMonitorType, + Factory: NewClusterDNSPodMonitor, + Validator: ValidateClusterDNSPodConfig, + Description: "Verifies cluster DNS resolution via overlay-test pods running in pod-network context (works around host-network Cilium ClusterIP routing gaps)", + }) +} + +// NewClusterDNSPodMonitorWithDiscovery creates a cluster-DNS-pod monitor using the +// supplied PeerDiscovery and HTTP client. This is a test-injection constructor mirroring +// NewKubernetesPeerDiscoveryWithClient's pattern. +func NewClusterDNSPodMonitorWithDiscovery(ctx context.Context, config types.MonitorConfig, discovery PeerDiscovery, httpClient clusterDNSPodHTTPClient) (types.Monitor, error) { + cfg, err := parseClusterDNSPodConfig(config.Config) + if err != nil { + return nil, fmt.Errorf("failed to parse cluster-dns-pod config: %w", err) + } + cfg.applyDefaults() + + baseMonitor, err := monitors.NewBaseMonitor(config.Name, config.Interval, config.Timeout) + if err != nil { + return nil, fmt.Errorf("failed to create base monitor: %w", err) + } + + monitor := &ClusterDNSPodMonitor{ + name: config.Name, + config: cfg, + discovery: discovery, + httpClient: httpClient, + BaseMonitor: baseMonitor, + } + + if err := baseMonitor.SetCheckFunc(monitor.checkClusterDNSPod); err != nil { + return nil, fmt.Errorf("failed to set check function: %w", err) + } + + return monitor, nil +} + +// NewClusterDNSPodMonitor is the standard registered factory for the cluster-DNS-pod +// monitor. It builds a real Kubernetes-backed PeerDiscovery and a real *http.Client. +func NewClusterDNSPodMonitor(ctx context.Context, config types.MonitorConfig) (types.Monitor, error) { + cfg, err := parseClusterDNSPodConfig(config.Config) + if err != nil { + return nil, fmt.Errorf("failed to parse cluster-dns-pod config: %w", err) + } + cfg.applyDefaults() + + discovery, err := NewKubernetesPeerDiscovery(&PeerDiscoveryConfig{ + Namespace: cfg.Namespace, + LabelSelector: cfg.LabelSelector, + }) + if err != nil { + return nil, fmt.Errorf("failed to create kubernetes peer discovery: %w", err) + } + + httpClient := &http.Client{ + Transport: &http.Transport{ + DisableKeepAlives: true, // Each probe should be independent. + DialContext: (&net.Dialer{ + Timeout: 5 * time.Second, + }).DialContext, + }, + } + + return NewClusterDNSPodMonitorWithDiscovery(ctx, config, discovery, httpClient) +} + +// Start starts the cluster-DNS-pod monitor, beginning peer discovery before the base +// monitor's check loop. +func (m *ClusterDNSPodMonitor) Start() (<-chan *types.Status, error) { + if err := m.discovery.Start(context.Background()); err != nil { + return nil, fmt.Errorf("failed to start peer discovery: %w", err) + } + + return m.BaseMonitor.Start() +} + +// Stop stops the cluster-DNS-pod monitor and its peer discovery. +func (m *ClusterDNSPodMonitor) Stop() { + m.discovery.Stop() + m.BaseMonitor.Stop() +} + +// clusterDNSPodProbeOutcome is the per-peer result of a single cluster-DNS pod probe. +type clusterDNSPodProbeOutcome struct { + resolved bool +} + +// checkClusterDNSPod polls discovered overlay-test pods' /clusterdns endpoint and +// derives a ClusterDNSDown condition from the pod-sourced cluster-DNS resolution truth. +func (m *ClusterDNSPodMonitor) checkClusterDNSPod(ctx context.Context) (*types.Status, error) { + status := types.NewStatus(m.name) + + peers := m.discovery.GetPeers() + if len(peers) == 0 { + status.AddEvent(types.NewEvent( + types.EventWarning, + "NoOverlayTestPodsFound", + "No overlay-test pods found for cluster DNS probing", + )) + // Intentionally leave the consecutive-failure counter untouched: a transient + // peer-discovery gap (e.g. right after monitor startup before the first + // Refresh completes) is a discovery-layer problem, not evidence of an actual + // cluster-DNS failure, so it must not be conflated into a false ClusterDNSDown. + return status, nil + } + + // Cap to at most maxClusterDNSProbePeers peers, sorted by Name for a deterministic + // selection. This bounds load on overlay-test pods when there are many nodes. + sort.Slice(peers, func(i, j int) bool { return peers[i].Name < peers[j].Name }) + selected := peers + if len(selected) > maxClusterDNSProbePeers { + selected = selected[:maxClusterDNSProbePeers] + } + + outcomes := make(chan clusterDNSPodProbeOutcome, len(selected)) + var wg sync.WaitGroup + + for _, peer := range selected { + wg.Add(1) + go func(p Peer) { + defer wg.Done() + outcomes <- clusterDNSPodProbeOutcome{resolved: m.probePeer(ctx, p)} + }(peer) + } + + go func() { + wg.Wait() + close(outcomes) + }() + + resolvedCount := 0 + totalProbed := len(selected) + for outcome := range outcomes { + if outcome.resolved { + resolvedCount++ + } + } + + m.mu.Lock() + if resolvedCount >= m.config.MinSuccessPods { + m.consecutiveFailures = 0 + } else { + m.consecutiveFailures++ + } + consecutiveFailures := m.consecutiveFailures + m.mu.Unlock() + + // Toggle ClusterDNSDown True/False based on the consecutive-failure count, using + // the same sticky-latched semantics as DNSMonitor.updateFailureTracking: True once + // the threshold is reached, False only once the counter is back to exactly 0, and + // left unset in between so a previously-latched condition stays as-is. + switch { + case consecutiveFailures >= m.config.FailureCountThreshold: + status.AddCondition(types.NewCondition( + "ClusterDNSDown", + types.ConditionTrue, + "RepeatedClusterDNSPodProbeFailures", + fmt.Sprintf("Cluster DNS via overlay-test pods has failed %d consecutive times (threshold: %d); last cycle resolved %d/%d pods", + consecutiveFailures, m.config.FailureCountThreshold, resolvedCount, totalProbed), + )) + case consecutiveFailures == 0: + status.AddCondition(types.NewCondition( + "ClusterDNSDown", + types.ConditionFalse, + "ClusterDNSResolvedViaPods", + fmt.Sprintf("Cluster DNS resolved via %d/%d overlay-test pod(s)", resolvedCount, totalProbed), + )) + } + + status.AddEvent(types.NewEvent( + types.EventInfo, + "ClusterDNSPodProbeSummary", + fmt.Sprintf("Cluster DNS pod probe: %d/%d overlay-test pod(s) resolved cluster DNS", resolvedCount, totalProbed), + )) + + return status, nil +} + +// probePeer issues a single HTTP probe to a peer's overlay-test-server /clusterdns +// endpoint and reports whether that pod resolved cluster DNS. Any error (timeout, +// connection failure, non-2xx status, malformed body) is treated as unresolved. +func (m *ClusterDNSPodMonitor) probePeer(ctx context.Context, peer Peer) bool { + reqCtx, cancel := context.WithTimeout(ctx, m.config.Timeout) + defer cancel() + + url := "http://" + net.JoinHostPort(peer.PodIP, strconv.Itoa(m.config.ProbePort)) + m.config.ProbePath + + req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, url, nil) + if err != nil { + return false + } + + resp, err := m.httpClient.Do(req) + if err != nil { + return false + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return false + } + + var result clusterdns.ProbeResult + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return false + } + + return result.Resolved +} diff --git a/pkg/monitors/network/clusterdnspod_test.go b/pkg/monitors/network/clusterdnspod_test.go new file mode 100644 index 0000000..2218635 --- /dev/null +++ b/pkg/monitors/network/clusterdnspod_test.go @@ -0,0 +1,209 @@ +package network + +import ( + "context" + "encoding/json" + "net" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/supporttools/node-doctor/pkg/clusterdns" + "github.com/supporttools/node-doctor/pkg/types" +) + +// startFakeOverlayServers starts a single HTTP server that binds on all local +// addresses (":0") so that distinct loopback IPs (127.0.0.2, 127.0.0.3, ...) can each +// act as a distinct simulated overlay-test pod while sharing one ProbePort, mirroring +// how ClusterDNSPodMonitor addresses peers via peer.PodIP + a single configured +// ProbePort. The handler dispatches on the request Host header (which carries the +// dialed peer.PodIP:port) to return a canned clusterdns.ProbeResult per "pod". Hosts +// with no registered result respond 404, simulating an unreachable/errored pod. +func startFakeOverlayServers(t *testing.T, hostToResult map[string]clusterdns.ProbeResult) (port int, cleanup func()) { + t.Helper() + + mux := http.NewServeMux() + mux.HandleFunc("/clusterdns", func(w http.ResponseWriter, r *http.Request) { + host := r.Host + if h, _, err := net.SplitHostPort(r.Host); err == nil { + host = h + } + result, ok := hostToResult[host] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(result) + }) + + ts := httptest.NewUnstartedServer(mux) + listener, err := net.Listen("tcp", ":0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + _ = ts.Listener.Close() + ts.Listener = listener + ts.Start() + + tcpAddr, ok := listener.Addr().(*net.TCPAddr) + if !ok { + t.Fatalf("unexpected listener address type: %T", listener.Addr()) + } + + return tcpAddr.Port, ts.Close +} + +func newTestClusterDNSPodMonitor(port int, peers []Peer, opts func(*ClusterDNSPodConfig)) *ClusterDNSPodMonitor { + cfg := &ClusterDNSPodConfig{ + ProbePort: port, + ProbePath: "/clusterdns", + Timeout: 2 * time.Second, + FailureCountThreshold: 3, + MinSuccessPods: 1, + } + if opts != nil { + opts(cfg) + } + + return &ClusterDNSPodMonitor{ + name: "test-cluster-dns-pod", + config: cfg, + discovery: NewStaticPeerDiscovery(peers), + httpClient: &http.Client{ + Timeout: 5 * time.Second, + }, + } +} + +func hasCondition(status *types.Status, condType string, condStatus types.ConditionStatus, reason string) bool { + for _, c := range status.Conditions { + if c.Type == condType && c.Status == condStatus && c.Reason == reason { + return true + } + } + return false +} + +func hasAnyCondition(status *types.Status, condType string) bool { + for _, c := range status.Conditions { + if c.Type == condType { + return true + } + } + return false +} + +func hasEvent(status *types.Status, reason string) bool { + for _, e := range status.Events { + if e.Reason == reason { + return true + } + } + return false +} + +func TestClusterDNSPodMonitor_AllPodsResolved(t *testing.T) { + port, cleanup := startFakeOverlayServers(t, map[string]clusterdns.ProbeResult{ + "127.0.0.2": {Target: "kubernetes.default.svc.cluster.local", Resolved: true, Addresses: []string{"10.96.0.1"}}, + "127.0.0.3": {Target: "kubernetes.default.svc.cluster.local", Resolved: true, Addresses: []string{"10.96.0.1"}}, + }) + defer cleanup() + + peers := []Peer{ + {Name: "overlay-a", NodeName: "node-a", PodIP: "127.0.0.2"}, + {Name: "overlay-b", NodeName: "node-b", PodIP: "127.0.0.3"}, + } + monitor := newTestClusterDNSPodMonitor(port, peers, nil) + + status, err := monitor.checkClusterDNSPod(context.Background()) + if err != nil { + t.Fatalf("checkClusterDNSPod() unexpected error: %v", err) + } + + if !hasCondition(status, "ClusterDNSDown", types.ConditionFalse, "ClusterDNSResolvedViaPods") { + t.Errorf("expected ClusterDNSDown=False/ClusterDNSResolvedViaPods, got conditions: %+v", status.Conditions) + } + if !hasEvent(status, "ClusterDNSPodProbeSummary") { + t.Errorf("expected ClusterDNSPodProbeSummary event, got events: %+v", status.Events) + } +} + +func TestClusterDNSPodMonitor_AllPodsFailRepeatedly(t *testing.T) { + // Register no hosts, so every probe gets a 404 -> unresolved. + port, cleanup := startFakeOverlayServers(t, map[string]clusterdns.ProbeResult{}) + defer cleanup() + + peers := []Peer{ + {Name: "overlay-a", NodeName: "node-a", PodIP: "127.0.0.2"}, + {Name: "overlay-b", NodeName: "node-b", PodIP: "127.0.0.3"}, + } + monitor := newTestClusterDNSPodMonitor(port, peers, func(c *ClusterDNSPodConfig) { + c.FailureCountThreshold = 3 + }) + + var status *types.Status + var err error + for i := 0; i < monitor.config.FailureCountThreshold; i++ { + status, err = monitor.checkClusterDNSPod(context.Background()) + if err != nil { + t.Fatalf("checkClusterDNSPod() unexpected error on cycle %d: %v", i, err) + } + } + + if !hasCondition(status, "ClusterDNSDown", types.ConditionTrue, "RepeatedClusterDNSPodProbeFailures") { + t.Errorf("expected ClusterDNSDown=True/RepeatedClusterDNSPodProbeFailures after %d cycles, got conditions: %+v", + monitor.config.FailureCountThreshold, status.Conditions) + } +} + +func TestClusterDNSPodMonitor_PartialSuccessMeetsMinSuccessPods(t *testing.T) { + port, cleanup := startFakeOverlayServers(t, map[string]clusterdns.ProbeResult{ + "127.0.0.2": {Target: "kubernetes.default.svc.cluster.local", Resolved: true, Addresses: []string{"10.96.0.1"}}, + // 127.0.0.3 intentionally unregistered -> 404 -> unresolved + }) + defer cleanup() + + peers := []Peer{ + {Name: "overlay-a", NodeName: "node-a", PodIP: "127.0.0.2"}, + {Name: "overlay-b", NodeName: "node-b", PodIP: "127.0.0.3"}, + } + monitor := newTestClusterDNSPodMonitor(port, peers, func(c *ClusterDNSPodConfig) { + c.MinSuccessPods = 1 + }) + + status, err := monitor.checkClusterDNSPod(context.Background()) + if err != nil { + t.Fatalf("checkClusterDNSPod() unexpected error: %v", err) + } + + if !hasCondition(status, "ClusterDNSDown", types.ConditionFalse, "ClusterDNSResolvedViaPods") { + t.Errorf("expected ClusterDNSDown=False/ClusterDNSResolvedViaPods with partial success, got conditions: %+v", status.Conditions) + } + if !hasEvent(status, "ClusterDNSPodProbeSummary") { + t.Errorf("expected ClusterDNSPodProbeSummary event, got events: %+v", status.Events) + } +} + +func TestClusterDNSPodMonitor_NoPeersFound(t *testing.T) { + monitor := newTestClusterDNSPodMonitor(8023, nil, nil) + // Pre-seed a nonzero failure count to prove the no-peers path leaves it untouched. + monitor.consecutiveFailures = 5 + + status, err := monitor.checkClusterDNSPod(context.Background()) + if err != nil { + t.Fatalf("checkClusterDNSPod() unexpected error: %v", err) + } + + if !hasEvent(status, "NoOverlayTestPodsFound") { + t.Errorf("expected NoOverlayTestPodsFound event, got events: %+v", status.Events) + } + if hasAnyCondition(status, "ClusterDNSDown") { + t.Errorf("expected no ClusterDNSDown condition on no-peers cycle, got conditions: %+v", status.Conditions) + } + if monitor.consecutiveFailures != 5 { + t.Errorf("expected consecutiveFailures to remain unchanged at 5 on no-peers cycle, got %d", monitor.consecutiveFailures) + } +} diff --git a/pkg/monitors/network/dns.go b/pkg/monitors/network/dns.go index 71cf409..1ce3349 100644 --- a/pkg/monitors/network/dns.go +++ b/pkg/monitors/network/dns.go @@ -16,6 +16,7 @@ import ( "sync" "time" + "github.com/supporttools/node-doctor/pkg/clusterdns" "github.com/supporttools/node-doctor/pkg/monitors" "github.com/supporttools/node-doctor/pkg/types" ) @@ -2347,73 +2348,11 @@ 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. +// defaultClusterDomains returns the default in-cluster DNS probe target(s), derived +// from the resolver's search domains via pkg/clusterdns. See clusterdns.ClusterProbeName +// for the derivation and fallback behavior. 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 + return []string{clusterdns.ClusterProbeName(resolverPath)} } // exclusiveCond describes one member of a mutually-exclusive condition group. diff --git a/pkg/monitors/network/dns_clusterdomain_test.go b/pkg/monitors/network/dns_clusterdomain_test.go index 7110946..236efa6 100644 --- a/pkg/monitors/network/dns_clusterdomain_test.go +++ b/pkg/monitors/network/dns_clusterdomain_test.go @@ -6,60 +6,6 @@ import ( "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") @@ -69,24 +15,6 @@ func writeResolv(t *testing.T, content string) string { 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")