feat(dns): pod-network cluster-DNS probe via overlay-test pods (task 242) - #28
Conversation
…242) 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.<derived> 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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // 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) |
| 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6888c6c260
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| status.AddCondition(types.NewCondition( | ||
| "ClusterDNSDown", |
There was a problem hiding this comment.
When this opt-in monitor reports ClusterDNSDown=True, the chart still also runs dns-health with clusterDomains: []; DNSMonitor.checkDNSDomains treats an empty cluster list as success and updateFailureTracking emits ClusterDNSDown=False every cycle. The Kubernetes condition manager keys updates only by condition type, so the next dns-health status can overwrite this pod-sourced failure and make the new alert depend on monitor ordering instead of the actual pod probe result. Suppress the old cluster condition when cluster DNS is disabled, or have this monitor use a separate condition.
Useful? React with 👍 / 👎.
| checkNameservers: true | ||
| failureCountThreshold: 3 | ||
| enableNameserverChecks: true | ||
| {{- if .Values.clusterDnsPodProbe.enabled }} |
There was a problem hiding this comment.
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 👍 / 👎.
2 participants
Summary
Implements the correct way to monitor cluster DNS on this fleet — the real fix behind the gated re-enable.
Why the in-agent check can't work
node-doctor runs
hostNetwork. Verified live on a1-ops-prd: from the host netns,kubernetes.default.svc.a1-ops-prd.local(the correct domain) → NXDOMAIN via kube-dns ClusterIP, while the same name resolves from a pod and external domains resolve from the host. Cilium doesn't route host-netns→ClusterIP for cluster records. So no domain (derived or explicit) lets node-doctor resolve cluster DNS — the check stays disabled (clusterDomains: []).The fix
Probe()(resolveskubernetes.default.svc.<derived>)./clusterdns— runsProbe()in the pod's pod-network context, where cluster records DO resolve.network-cluster-dns-pod— discovers overlay-test pods, HTTP-probes their/clusterdns(node-doctor can reach pod IPs from host netns — that's how the CNI overlay test works — just not the ClusterIP), and drivesClusterDNSDownfrom pod-sourced truth with DNSMonitor-style consecutive-failure latching. No-peers cycle is a no-op (no false positive).Safety / rollout
clusterDnsPodProbe.enableddefaults false → ships inert./clusterdns(this release). Enabling against an older overlay-test image 404s every probe → false ClusterDNSDown. So the re-enable = roll v1.8.3 (new overlay-test image) then flip the toggle, in a watched window with canary.Tests
pkg/clusterdns: derivation (standard/custom/fallback) + hermeticProbe.Task: #19561 (242). Extends the
hostnetwork-agent-cannot-resolve-clusterip-dns-cilium-probe-from-podfinding.