Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion cmd/overlay-test-server/main.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -10,6 +13,8 @@
"log"
"net/http"
"os"

"github.com/supporttools/node-doctor/pkg/clusterdns"
)

func main() {
Expand All @@ -30,6 +35,13 @@
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

Check warning

Code scanning / gosec

Errors unhandled Warning

Errors unhandled
})

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
Expand Down
23 changes: 23 additions & 0 deletions helm/node-doctor/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,29 @@ data:
checkNameservers: true
failureCountThreshold: 3
enableNameserverChecks: true
{{- if .Values.clusterDnsPodProbe.enabled }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard the new Helm value before dereferencing it

The checked-in chart values file (helm/node-doctor/values.yaml) still has no clusterDnsPodProbe block, so rendering/installing helm/node-doctor directly from the repo evaluates .Values.clusterDnsPodProbe as nil here and fails before the intended disabled default can apply. The defaults were added only to values.yaml.template, which helps the release-generation path but not local chart users or CI that renders the committed chart; add the block to values.yaml as well or use a nil-safe guard/default.

Useful? React with 👍 / 👎.


# 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:
Expand Down
20 changes: 20 additions & 0 deletions helm/node-doctor/values.yaml.template
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
116 changes: 116 additions & 0 deletions pkg/clusterdns/clusterdns.go
Original file line number Diff line number Diff line change
@@ -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.<domain>", 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)

Check failure

Code scanning / gosec

Potential file inclusion via variable Error

Potential file inclusion via variable
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:
//
// <namespace>.svc.<clusterDomain> svc.<clusterDomain> <clusterDomain>
//
// so the cluster domain is the suffix after the "svc." label. It prefers the canonical
// "svc.<clusterDomain>" entry, then any "<ns>.svc.<clusterDomain>" 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.<clusterDomain>".
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 "<namespace>.svc.<clusterDomain>".
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
}
137 changes: 137 additions & 0 deletions pkg/clusterdns/clusterdns_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}
Loading
Loading