fix: restore node-level DNS after unexpected LocalDNS exit - #9360
fix: restore node-level DNS after unexpected LocalDNS exit#9360Saewon Kwak (saewoni) wants to merge 7 commits into
Conversation
Windows Unit Test Results 3 files 14 suites 52s ⏱️ Results for commit 6eb258a. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Removes unnecessary LocalDNS cgroup delegation to prevent teardown failures after unclean supervisor termination.
Changes:
- Sets
Delegate=noforlocaldns.service. - Adds VHD-content validation for the setting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
parts/linux/cloud-init/artifacts/localdns-delegate.conf |
Disables cgroup delegation. |
vhdbuilder/packer/test/linux-vhd-content-test.sh |
Verifies delegation remains disabled. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The terminal-state E2E can falsely pass when resolver-state retrieval fails.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
E2E status: branch-VHD E2E passed; the red
|
| Check | VHD under test | Result |
|---|---|---|
| AKS Linux VHD Build - PR check-in gate (e2e Run AgentBaker E2E) | VHD built from this branch (has ExecStopPost) |
✅ pass |
| Agentbaker E2E | currently published VHD (no ExecStopPost) |
❌ fail (expected) |
The branch-VHD E2E passed — the LocalDNSHostsPlugin/{Ubuntu2204,Ubuntu2404,AzureLinuxV3} lifecycle validation, including the terminal dead-service SIGKILL case, ran on nodes provisioned from branch-built VHDs and succeeded.
The red Agentbaker E2E ran against the published VHD, which does not yet have ExecStopPost baked in, so the new lifecycle assertion correctly fails there with FAIL: 70-localdns.conf still present after localdns died — i.e. it reproduces the very blackhole this PR fixes on the unpatched image. Its 4th failure, Ubuntu2204_ArtifactStreaming_TrustedLaunch (a waagent iptables ... CHAIN_ZERO failed ExtHandler error), is unrelated to this change.
This matches the live-node A/B validation in the PR description: without ExecStopPost the node stays pointed at the dead 169.254.10.10 listener; with it, ExecStopPost removes 70-localdns.conf and reverts DNS on the untrappable SIGKILL path.
There was a problem hiding this comment.
🟡 Changes recommended
The E2E validation can exit without restoring its service override and does not correctly retry transient resolver-read failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
e2e/scenario_localdns_hosts.go:170
- 🟡 Medium Risk — 🔧 Script Logic: Because this script starts with
set -e, a transient failure from either resolver read terminates the entire validation at the assignment instead of retrying as the comment promises. This can make the eventual-consistency check flaky and also bypass the restoration below. Convert read failures to an empty snapshot; the existing non-empty guard will then retry safely.
if command -v resolvectl >/dev/null 2>&1; then
current_dns=$(resolvectl status 2>/dev/null)
else
current_dns=$(cat /run/systemd/resolve/resolv.conf 2>/dev/null)
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
Failed gate run
Detective summaryTwo tests failed: Likely causeSignatures: Recommended owner/actionNode Lifecycle/E2E owner: continue CSE threshold work under #39622938 and inspect whether bootstrap-token validation is a transient STLS/bootstrap timing issue before treating it as PR-caused. Strongest alternativePR-change-caused LocalDNS regression is the strongest alternative because PR 9360 modifies LocalDNS service/script behavior; it is less likely because neither primary failure is a LocalDNS functional assertion and the CSE timing threshold is a known recurring signature. Evidence
Wiki signature
|
There was a problem hiding this comment.
🟡 Changes recommended
The resolver polling loop exits on transient read errors instead of retrying them.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Ye Wang [msft] (@yewmsft) could you please re-review PR #9360 after the latest updates? The PR has been rebased onto the latest Since your earlier review, we have:
The remaining scope clarification is intentional: #9360 restores the node resolver ( |
There was a problem hiding this comment.
🔵 Needs a closer look
The E2E resolver poll exits on transient read errors instead of retrying as intended.
Review details
Suppressed comments (1)
e2e/scenario_localdns_hosts.go:204
- 🟡 Medium Risk — 🔧 Script Logic: With
set -e, a transient nonzero exit fromresolvectl statusor the fallbackcatterminates the validation at this assignment, so the surrounding 12-attempt poll never retries errored reads as intended. Handle the read failure inside the loop and leavecurrent_dnsempty so the next iteration can retry.
if command -v resolvectl >/dev/null 2>&1; then
current_dns=$(resolvectl status 2>/dev/null)
else
current_dns=$(cat /run/systemd/resolve/resolv.conf 2>/dev/null)
fi
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The new SIGTERM trap can prevent systemd from restarting LocalDNS after an unexpected termination.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
e2e/scenario/scenario_localdns_hosts.go:204
- 🟡 Medium Risk — 🧪 Test Coverage: Because the script starts with
set -e, a failedresolvectlor fallback file read exits immediately at this assignment rather than retrying as the surrounding comments require. Make each read failure assign an empty snapshot so the loop continues and only fails after the bounded polling window.
if command -v resolvectl >/dev/null 2>&1; then
current_dns=$(resolvectl status 2>/dev/null)
else
current_dns=$(cat /run/systemd/resolve/resolv.conf 2>/dev/null)
fi
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
Ye Wang [msft] (yewmsft)
left a comment
There was a problem hiding this comment.
took a closer pass at what this actually guarantees. the ExecStopPost layer is the right call and the shellspec coverage is good. the EXIT-trap restore in the e2e is fixed, thanks.
two things before this can go in — the required-check breakage, and the drop-in path being re-derived at stop-post time. rest are correctness/accuracy nits inline.
one thing on the PR description: "nothing reverts node-level DNS when localdns exits unexpectedly" isn't quite right. localdns.sh line 1043 — pre-existing on main, not part of this change — already calls cleanup_iptables_and_dns at the top of every ExecStart, so crash -> successful restart already reverted node DNS. the hole this PR closes is the terminal case: Restart=on-failure gives up (StartLimit exhausted, or the unit is stopped and never comes back), ExecStart never runs again, and the drop-in outlives the service. that's narrower than what the description claims but it's the real incident, and it's a stronger argument for ExecStopPost than the current wording. please update it.
| fi | ||
| dig +short +time=5 +tries=1 mcr.microsoft.com @169.254.10.10 | grep -q . | ||
|
|
||
| # Terminal dead-service case: this is the incident scenario the PR fixes. |
There was a problem hiding this comment.
blocker. Agentbaker E2E is a required check and it runs main's published VHD, not this branch's build — e2e/config/config.go defaults SIG_VERSION_TAG_NAME=branch / refs/heads/main, and .pipelines/scripts/e2e_run.sh:83 only overrides the selector when VHD_BUILD_ID is set. so this block asserts on an ExecStopPost that isn't in the image under test.
that's exactly what's happening right now: build 180533805 (the required check on this PR) failed all three lifecycle scenarios, while the branch-VHD run passed. merge this and the required check goes red on every PR in the repo until a main VHD carrying ExecStopPost publishes.
gate the block on the unit actually having the hook:
| # Terminal dead-service case: this is the incident scenario the PR fixes. | |
| # This block asserts on ExecStopPost, which is baked into the VHD. The standalone | |
| # Agentbaker E2E runs main's published VHD, so skip until the new image ships. | |
| if ! systemctl show localdns.service -p ExecStopPost --value | grep -q 'localdns.sh cleanup'; then | |
| echo "SKIP: VHD predates the ExecStopPost cleanup hook" | |
| exit 0 | |
| fi | |
| # Terminal dead-service case: this is the incident scenario the PR fixes. |
There was a problem hiding this comment.
Addressed in 5fc8605. The lifecycle validator now checks for the baked ExecStopPost hook before asserting the new behavior and skips with a diagnostic on published VHDs that predate the hook. The branch-VHD E2E remains the authoritative validation for the new lifecycle behavior.
There was a problem hiding this comment.
the gate works, but it sits at the top of the validator (:70-73), so on an older vhd it skips everything, not just the terminal block. the restart-loop coverage at :113-156 passes fine on main's vhd today — that's what standalone e2e was actually exercising, and now it's a no-op there. green-by-skip.
move the check down to guard only the terminal block at :158:
if sudo systemctl show localdns.service -p ExecStopPost --value | grep -q 'localdns.sh cleanup'; then
# terminal dead-service case ... (current :158-229)
else
echo "SKIP: VHD predates the ExecStopPost cleanup hook"
fithe EXIT trap at :101 is already installed above that point, so restore still works on either branch. branch-vhd e2e keeps covering the new behavior; standalone keeps the old coverage instead of skipping the whole scenario.
| # SIGTERM is the normal systemd stop signal and must be reported as a clean stop. | ||
| trap 'echo "Received SIGTERM. Cleaning up..."; cleanup_localdns_configs || true; exit 0' TERM |
There was a problem hiding this comment.
this trap can't fire until the current foreground command returns, and the watchdog loop spends nearly all its time parked in sleep "${HEALTH_CHECK_INTERVAL}" (line 932) — 12s at WatchdogSec=60. on top of that the health-check body has curl --max-time 10 plus three dig at 2s x 2 tries, then cleanup_localdns_configs waits LOCALDNS_SHUTDOWN_DELAY=5 for coredns to drain. TimeoutStopSec=30. systemctl stop was effectively instant before this PR because SIGTERM was untrapped; now it isn't, and it can plausibly hit the timeout and get SIGKILLed.
for the record, SIGTERM was never in the ABRT ERR INT PIPE list, so there was no exit-216-on-stop bug to fix here — but the trap does buy the graceful coredns drain and the dummy-interface removal, so keep it and fix the sleep:
# line 932
sleep "${HEALTH_CHECK_INTERVAL}" & wait $!wait is interruptible, so the trap runs immediately instead of up to 12s later.
There was a problem hiding this comment.
Addressed in 5fc8605. The watchdog sleep now runs as sleep ... & wait $!, allowing SIGTERM to interrupt the wait and enter cleanup promptly instead of potentially consuming the stop timeout. The explicit SIGTERM trap was removed; normal cleanup remains covered by the EXIT trap and ExecStopPost.
There was a problem hiding this comment.
outcome is fine, but two things in the rationale don't hold.
1. with the TERM trap gone, sleep ... & wait $! isn't doing anything — and the comment at :948-949 says it is. measured on bash 5.2.21, SIGTERM at t=0.3s:
no TERM trap, plain `sleep 10` -> exits 0.3s, EXIT trap ran, rc=143
no TERM trap, `sleep 10 & wait $!` -> exits 0.3s, EXIT trap ran, rc=143
TERM trap , plain `sleep 10` -> exits 10.0s <- the original bug
TERM trap , `sleep 10 & wait $!` -> exits 0.3s
an untrapped TERM goes through bash's terminating-signal handler, which runs the EXIT trap and re-raises right away. the deferral only ever existed because a user trap can't run until the foreground command returns. keeping & wait $! is harmless, but fix or drop the comment — it explains a mechanism that's no longer in play.
2. "an unexpected TERM remains a failure eligible for Restart=on-failure" — it isn't. systemd.service(5): a clean exit is "exit code of 0; for types other than Type=oneshot, one of the signals SIGHUP, SIGINT, SIGTERM, or SIGPIPE", and on-failure restarts on signals excluding those four. so death by TERM is clean to systemd — unit goes inactive, no restart, with or without the trap.
net: node dns still gets restored via ExecStopPost, which is what matters. just don't count on a restart after an unexpected TERM.
There was a problem hiding this comment.
🔵 Needs a closer look
The E2E resolver polling exits on transient read failures instead of retrying.
Review details
Suppressed comments (1)
e2e/scenario/scenario_localdns_hosts.go:204
- 🟡 Medium Risk — 🔧 Script Logic:
set -eis active, so ifresolvectl status(or the fallbackcat) transiently fails, the failed assignment terminates the validation immediately instead of retrying as the surrounding comments intend. Suppress the read status inside the substitution; the existing non-empty check still prevents a failed read from being accepted as restored DNS.
if command -v resolvectl >/dev/null 2>&1; then
current_dns=$(resolvectl status 2>/dev/null)
else
current_dns=$(cat /run/systemd/resolve/resolv.conf 2>/dev/null)
fi
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The E2E cleanup can leave LocalDNS dead after an early failure, and resolver-read failures bypass the intended retry loop.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
e2e/scenario/scenario_localdns_hosts.go:110
- 🟡 Medium Risk — 🏗️ Architecture: The recovery block is gated on the temporary override already existing, but the preceding SIGKILL/recovery loop can fail before that file is created. In that path the
EXITtrap skipsreset-failedand restart entirely, potentially leaving LocalDNS dead on the shared scenario node. Run this recovery block unconditionally;rm -fis already safe when the override is absent.
e2e/scenario/scenario_localdns_hosts.go:204
- 🟡 Medium Risk — 🔧 Script Logic: Because the script enables
set -e, a transient failure from either resolver-read assignment exits the validation immediately rather than continuing this 12-attempt poll. Capture read failures as an empty snapshot so the existing non-empty guard retries them as intended.
if command -v resolvectl >/dev/null 2>&1; then
current_dns=$(resolvectl status 2>/dev/null)
else
current_dns=$(cat /run/systemd/resolve/resolv.conf 2>/dev/null)
fi
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The E2E validator has retry and cleanup defects that can cause flaky failures and leave its node unhealthy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
e2e/scenario/scenario_localdns_hosts.go:204
- 🟡 Medium Risk — 🔧 Script Logic:
set -emakes a failedresolvectl/catassignment terminate the script immediately, so the loop never retries the errored snapshots that the comment says are eventually consistent. Mask the read status and let the existing non-empty check decide whether to retry.
if command -v resolvectl >/dev/null 2>&1; then
current_dns=$(resolvectl status 2>/dev/null)
else
current_dns=$(cat /run/systemd/resolve/resolv.conf 2>/dev/null)
fi
e2e/scenario/scenario_localdns_hosts.go:110
- 🟡 Medium Risk — 🏗️ Architecture: Cleanup is gated on the no-restart drop-in, but that file is created only after the repeated SIGKILL recovery checks. If one of those checks fails because the service is dead, the trap skips
reset-failedand restart entirely and leaves the scenario node unhealthy. Run the service restoration unconditionally;rm -falready handles an absent override.
if [ -f "$NORESTART" ]; then
cleanup_status=0
sudo rm -f "$NORESTART" || { echo "ERROR: failed to remove $NORESTART"; cleanup_status=1; }
sudo systemctl daemon-reload || { echo "ERROR: systemd daemon-reload failed during test cleanup"; cleanup_status=1; }
sudo systemctl reset-failed localdns.service || { echo "ERROR: reset-failed localdns.service failed during test cleanup"; cleanup_status=1; }
if ! sudo systemctl is-active --quiet localdns.service; then
sudo systemctl start localdns.service || { echo "ERROR: failed to restart localdns.service during test cleanup"; cleanup_status=1; }
fi
if ! sudo systemctl is-active --quiet localdns.service; then
echo "ERROR: localdns.service is not active after test cleanup"
cleanup_status=1
fi
if [ "$test_status" -eq 0 ] && [ "$cleanup_status" -ne 0 ]; then
test_status=$cleanup_status
fi
fi
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
It modifies production-critical systemd and node DNS failure handling, warranting final human validation despite comprehensive coverage.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
AgentBaker Linux gate detectiveRun: https://msazure.visualstudio.com/CloudNativeCompute/_build/results?buildId=180593480 Detective summaryThe E2E job completed with 122 passed, 70 skipped, and 2 failed scenarios. The failed surface is post-provision VM validation, not VHD build or CSE provisioning: AzureLinuxV3_CustomSysctls failed the bootstrap-token credential validation, and Ubuntu2204_ArtifactStreaming_ImagePull failed because the streaming pod used overlayfs fallback instead of an overlaybd TCMU backstore. Likely cause / signature
ConfidenceMedium: both failures are deterministic single-scenario validation assertions in this run, but they affect unrelated feature areas, which keeps shared environment/test-run flake plausible. Recommended owner/actionPR author and Node Lifecycle E2E owners should inspect whether the LocalDNS lifecycle cleanup changes can perturb bootstrap-token validation state; artifact streaming owners should confirm overlaybd/TEMU backstore detection on Ubuntu2204 before retrying. Strongest alternativeShared E2E environment flake is the strongest alternative because two unrelated validations failed while 122 scenarios passed; it is less likely for each individual scenario because both failures occurred after successful node readiness and reached precise validators. Evidence links |
Rebased onto latest main; the e2e scenario file was renamed from scenario_localdns_hosts_test.go to scenario_localdns_hosts.go by the standalone-CLI e2e refactor (#9321), so the lifecycle validator is re-attached to the new Register-based scenario. When the localdns supervisor exits unexpectedly (SIGKILL), the shell cleanup traps do not run, so the node can retain the network drop-in that points DNS at the dead localdns listener (169.254.10.10), causing a node-level DNS outage. - localdns.service: add ExecStopPost=/opt/azure/containers/localdns/localdns.sh cleanup so DNS is reverted after both graceful and unexpected exits. - localdns.sh: add cleanup mode (localdns_cleanup_mode) that restores node DNS and always exits 0 so a cleanup error cannot wedge systemd recovery; make cleanup_iptables_and_dns aggregate failures instead of returning early so DNS drop-in removal and network reload always run even when iptables rule deletion fails. - localdns_spec.sh: ShellSpec coverage for cleanup_iptables_and_dns and cleanup mode (success, successful rule removal, iptables-failure still restores DNS, reload failure reported, cleanup mode exits 0 on success and failure). - e2e: lifecycle validator covering normal stop/start, kill+recovery with a genuinely-new-MainPID check, and the terminal dead-service case (disable auto-restart via a transient Restart=no drop-in, kill, then assert the 70-localdns.conf drop-in was removed and DNS no longer points at 169.254.10.10, polling for a terminal ActiveState and DNS revert).
The DNS-revert settle loop suppressed resolver-read errors (|| true), so an errored or empty resolvectl/resolv.conf read produced an empty current_dns, which the absence check then treated as 'listener gone' -> success. A failed read would therefore mask the terminal-outage regression the check exists to catch. Drop the error suppression and only accept a successful, non-empty snapshot that omits 169.254.10.10; empty/failed reads keep polling and fail the test if the resolver state never becomes readable.
Register an EXIT cleanup handler before installing the temporary Restart=no systemd drop-in used by the terminal dead-service validation. The handler removes the override, reloads systemd, resets the failed unit, restarts LocalDNS, and verifies the service is active. Preserve the original test status and report cleanup failures instead of masking them, so a failed assertion cannot contaminate the node or subsequent validation.
ExecStopPost can run after network state has already been torn down, so re-deriving the default route and network file can fail before the known 70-localdns.conf drop-in is removed. Sweep the known runtime drop-in path without depending on ip route or networkctl status, continue network reload, and retain aggregate failure reporting. Keep the dummy localdns interface and .10/.11 addresses during this node-level cleanup: an orphaned CoreDNS child may still be serving pods, and deleting the interface would turn a fast failure into default-route DNS blackholing. Service recovery handles the next-start interface lifecycle. Update ShellSpec coverage for cleanup without network-variable discovery.
Co-authored-by: saewoni <23280628+saewoni@users.noreply.github.com>
- Avoid route/interface discovery during ExecStopPost; sweep the known network drop-in directly so cleanup still restores node DNS after network state has been torn down. - Make SIGTERM cleanup interruptible by waiting on sleep in a child process. - Document that cleanup mode intentionally leaves the dummy interface and listener addresses for service recovery/orphaned-process safety, and make cleanup failure logs actionable. - Skip lifecycle assertions on VHDs without the ExecStopPost hook, add a positive node DNS resolution check, retry resolver reads under set -e, and install the E2E restoration trap before the first service mutation. - Clarify that the normal restart loop tests service recovery while the terminal block tests ExecStopPost.
Devinwong
left a comment
There was a problem hiding this comment.
There are some changes since my last Sept 3 approval. But my point still holds: approved from e2e test and integration. LocalDNS domain knowledge change still defers to the team's review.
The cleanup discovery-failure ShellSpec case intentionally exercises successful iptables cleanup while network-variable discovery would fail. Assert its successful rule-removal and network-reload output so ShellSpec reports no unexpected output warnings in CI.
There was a problem hiding this comment.
🔵 Needs a closer look
An unbounded iptables lock wait can prevent the post-stop hook from restoring DNS before systemd’s timeout.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
parts/linux/cloud-init/artifacts/localdns.sh:663
- 🟡 Medium Risk — 🔧 Script Logic: DNS restoration is still sequenced after
iptables -w. If another process holds the xtables lock, the unbounded wait can consumeTimeoutStopSec=30, causing systemd to killExecStopPostbefore it removes the drop-in—the exact node-DNS blackhole this hook is intended to prevent. Restore/reload DNS before touching iptables, or use a bounded iptables wait that leaves enough time for DNS cleanup.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
AgentBaker Linux gate detectiveRun: https://msazure.visualstudio.com/CloudNativeCompute/_build/results?buildId=180727566 Detective summaryThe E2E job had one failed scenario out of 195: ACL_NetworkIsolatedCluster_NonAnonymousACR. Node preparation, SSH connectivity, pod readiness, NIC validation, LocalDNS exporter, and wireserver checks completed; VM validation then failed on the kernel-log validator with Critical kernel issues detected and a call trace. Likely cause / signature�2e-acl-networkisolated-kernel-critical-validation: ACL NetworkIsolated VM validation fails after readiness because the kernel log contains a critical call trace. Failed test records, E2E log, build metadata, and adjacent passing validations corroborate a real post-provision validation failure. This PR changes LocalDNS cleanup/lifecycle files, so direct PR-change causality is weak. ConfidenceMedium-high for recurring ACL/kernel validation flake; low for PR-change-caused. Recommended owner/actionNode Lifecycle E2E/VHD owners should inspect the ACL NetworkIsolated kernel log artifact for the call trace. Treat as infrastructure/VHD validation unless it reproduces only on this LocalDNS PR. Strongest alternativeA PR-side LocalDNS lifecycle interaction is the strongest alternative because the run is PR 9360, but the failure is ACL kernel validation after LocalDNS checks passed and the PR does not change ACL/kernel paths. Evidence links |
Problem
This PR addresses the LocalDNS failure reported by Azure/AKS#5930.
localdns.servicesuperviseslocaldns.sh, which starts CoreDNS as a background process. During an unexpected supervisor exit, such asSIGKILL, the script cannot run its cleanup traps. The node can therefore retain the network drop-in that points DNS at LocalDNS while the resolver is no longer available. Restart attempts can also encounter leftover processes and fail, eventually leaving the service dead and causing a node-level DNS outage.Scope: this PR restores node-level (host) DNS only. It reverts the node host resolver (the
169.254.10.10node-listener drop-in and iptables rules) after an unexpected LocalDNS exit. It does not restore pod DNS. Pods receivenameserver 169.254.10.11(the cluster listener) from kubelet--cluster-dns, baked into each pod's/etc/resolv.confat creation and not repointable from the node for the pod's lifetime, soExecStopPostcannot give already-running pods a working resolver. Restoring pod DNS requires keeping the.11cluster listener answering -- guaranteeing LocalDNS service recovery from the terminal dead state (the cgroup / StartLimit cause) and/or a.11cluster-DNS fallback -- which is a distinct root cause tracked as a separate follow-up PR. This PR therefore does not by itself fully close AKS#5930; it addresses the node-DNS redirect and validates that behavior.Fix
ExecStopPost=/opt/azure/containers/localdns/localdns.sh cleanupso DNS cleanup runs after both normal and unexpected service exits.cleanupmode tolocaldns.shthat restores node DNS configuration and always exits successfully; systemd remains responsible for process cleanup.ExecStopPosthandles node-DNS restoration after unexpected exits.ExecStopPostDNS cleanup) baked into the VHD, which are identical regardless of the bootstrap path, so the scenario runs under the default (aks-node-controller / scriptless) provisioning path.The following are intentionally out of scope and tracked separately: pod-level DNS restoration (cluster-listener
.11recovery / fallback), guaranteed LocalDNS service recovery from the terminal dead state (cgroup delegation / StartLimit), and the unrelated network reconfiguration change.Live reproduction
The original failure was historically reproduced twice on the disposable cluster
sakwa-localdns-repro-0710using the affected Ubuntu 24.04 imageAKSUbuntu-2404gen2containerd-202608.06.1. That cluster is no longer retained. The historical reproduction targeted the single system-pool nodeaks-sysnp-14424852-vmss000000with LocalDNS enabled.Healthy baseline:
Fault injection:
Observed failure:
The first run reached
NRestarts=13; the second reachedNRestarts=6. Each run was recovered withsystemctl reset-failed localdns.servicefollowed bysystemctl restart localdns.service.Validation summary
sakwa-localdns-repro-0710, shipped Ubuntu 24.04 imageSIGKILLand restart-limit exhaustionStandard_D4s_v3node insouthcentralus70-localdns.confand restored node DNSlocaldns.shcleanup suitego test ./... -run '^$' -count=1ine2eThe live A/B test manually replaced the LocalDNS artifacts on a running node. The earlier branch-VHD E2E validated artifacts baked from the branch at that time; it is a separate validation layer and must be rerun after subsequent code/test hardening. The lifecycle validator is attached to the existing LocalDNS hosts-plugin scenarios for Ubuntu 22.04, Ubuntu 24.04, and Azure Linux V3.
Live validation
The core
ExecStopPostbehavior was tested on a fresh LocalDNS-enabled AKS node insouthcentralus(Standard_D4s_v3, Ubuntu 24.04) throughaz vmss run-command. The live A/B used the cleanup implementation available at that time. Subsequent cleanup and E2E hardening changes are covered by ShellSpec, syntax checks, and E2E compilation; a fresh branch-VHD cloud E2E is still required for the current head. The test used an A/B on the same node: shipped artifacts first, then the PR artifacts. Each phase installed a temporary runtimeRestart=nodrop-in, read the serviceMainPID, sentSIGKILL, waited for a terminalActiveState, and checked the node resolver state. The test always restored the original files, removed the temporary drop-in, reloaded systemd, reset the failed state, and restarted LocalDNS before exiting.SIGKILLis intentional: it cannot be caught by Bash, so the normal shell traps do not run. The test therefore exercises the systemdExecStopPostpath directly. The cleanup mode restores node DNS only; it intentionally does not delete the dummy interface or claim to recover the LocalDNS process/cgroup.Phase A — shipped artifacts
This reproduced the node-level DNS blackhole from the reported failure.
Phase B — PR #9360 artifacts
The journal confirmed the cleanup path ran on the live test node:
Normal stop/start was also verified:
Result
ExecStopPostSIGKILL70-localdns.conffailed.10; DNS blackholefailed.10removed;getentsucceedsMeaning for AKS#5930
This live test proves the node-level part of the incident: after an untrappable LocalDNS
SIGKILL, PR #9360 removes the stale node DNS configuration and restores host-level DNS even when the service remainsfailed.It does not prove pod-DNS recovery. Pods use the separate cluster listener
169.254.10.11, which is baked into existing pods'/etc/resolv.conf. Recovery of that listener is handled by the stacked follow-up work in PR #9439. Therefore, PR #9360 should claim node-level DNS restoration, not complete closure of every pod-DNS failure mode in AKS#5930.The node was restored to the shipped artifacts and left healthy after the test. The current branch passes the cleanup-focused ShellSpec coverage and E2E compilation checks. A fresh branch-VHD cloud E2E remains required to validate the latest baked artifacts.