diff --git a/CODEOWNERS b/CODEOWNERS index d5642451cca..87de69e36b9 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -31,6 +31,7 @@ spec/parts/linux/cloud-init/artifacts/mariner-package-update_spec.sh @Azure/agen parts/linux/cloud-init/artifacts/localdns.sh @Azure/agentbakeradmin @saewoni @yewmsft parts/linux/cloud-init/artifacts/localdns.service @Azure/agentbakeradmin @saewoni @yewmsft parts/linux/cloud-init/artifacts/localdns-delegate.conf @Azure/agentbakeradmin @saewoni @yewmsft +parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf @Azure/agentbakeradmin @saewoni @yewmsft spec/parts/linux/cloud-init/artifacts/localdns_spec.sh @Azure/agentbakeradmin @saewoni @yewmsft # Set components.json & windows_settings.json to no code-owner so that it can be approved and merged by component owner diff --git a/e2e/scenario/scenario_localdns_hosts.go b/e2e/scenario/scenario_localdns_hosts.go index f78f7eacebe..d21680bc62a 100644 --- a/e2e/scenario/scenario_localdns_hosts.go +++ b/e2e/scenario/scenario_localdns_hosts.go @@ -50,42 +50,162 @@ func init() { // unexpected-exit DNS teardown this PR fixes) on the target // distros. The hosts-plugin functionality itself is covered by // the scenario's default provisioning validation. - if tt.name == "Ubuntu2204" || tt.name == "Ubuntu2404" || tt.name == "AzureLinuxV3" { - return validateLocalDNSLifecycle(ctx, s) + if tt.name != "Ubuntu2204" && tt.name != "Ubuntu2404" && tt.name != "AzureLinuxV3" { + return nil } - return nil + if err := validateLocalDNSLifecycle(ctx, s); err != nil { + return err + } + // Then assert the restart budget actually bounds failures. + // + // The full failure-mode matrix runs on Ubuntu2404 only: it is + // systemd 255, where daemon-reload does not clear the start + // limiter and where the provisioning regression was found. On + // shortened clocks a healthy run of all seven modes costs ~7min, + // and the sizing against TestTimeoutVMSS is asserted at build + // time by TestLocalDNSFaultMatrixFitsVMSSBudget. The other + // distros run the single discriminating mode instead (~40s) -- + // enough to catch the directives being dropped on those images. + faults := localdnsDiscriminatingFault() + if tt.name == "Ubuntu2404" { + faults = localdnsFaultMatrix + } + return validateLocalDNSRestartBudget(ctx, s, faults) }, }, }) } } +// validateLocalDNSLifecycle exercises localdns.service end to end on the node: a normal +// stop/start, three supervisor kills recovered by Restart=on-failure, and the terminal +// dead-service case that #9360's ExecStopPost hook exists to handle. +// +// # Why the shell below is sparsely commented +// +// The script is SCP'd to the node, and the e2e's Bastion tunnel (bastionssh.go) forwards +// each SSH packet as a single websocket message with no chunking, against an 8,192-byte +// service cap. go-scp emits the body as a 4,096-byte chunk then the remainder, so +// max_write = script_size - 4096 + 45, and any script over 12,242 bytes kills the tunnel +// mid-run with StatusMessageTooBig. Build 181818040 lost three lanes to exactly that, after +// this script grew 512 bytes past a margin of 380. TestLocalDNSScriptsFitBastionLimit now +// guards every script this package sends. +// +// Comments cost the same as code on that wire and buy nothing at runtime, so the reasoning +// lives here instead. Keep the shell terse; put the "why" in this comment. +// +// # Lane gating (EXPECT_EXECSTOPPOST) +// +// Set from the lane's own image selection rather than probed from the node. Asking +// 'systemctl show -p ExecStopPost' whether to test ExecStopPost means the assertion only +// runs where it is already guaranteed to pass, and deleting the hook would silently turn +// the block off everywhere instead of failing it. See laneResolvedMainBuiltImage. +// +// # The EXIT trap +// +// Installed before any service mutation so 'set -e' cannot leave the node carrying the +// temporary Restart=no override or a failed unit. +// +// The override is removed unconditionally rather than behind '[ -f "$NORESTART" ]'. That +// test runs unprivileged, but sudo creates the drop-in inside a directory that root's umask +// makes 0750 root:root, so the guard could not stat the file, returned false, and skipped +// its own removal -- leaving Restart=no in effect for everything that ran afterwards on the +// node. 'rm -f' is already a no-op when the file is absent, so the guard bought nothing. +// The post-removal check uses 'sudo test -f' for the same reason. +// +// The restore retries rather than firing a single start. The kill block above can leave an +// orphaned CoreDNS holding 169.254.10.10:53 for a moment (the "Failed to kill control +// group" warning this test tolerates), and an immediate start then fails to bind. That is +// the exact transient RestartSec=2 exists to wait out in production, so wait for it here +// rather than failing the scenario on a cleanup race. +// +// # Kill/recovery cycles +// +// reset-failed runs before every kill. The budget (StartLimitBurst=5 within +// StartLimitIntervalSec=720) belongs to the unit and is shared by every actor that starts +// it -- CSE at provisioning, the validations above, and systemd's own Restart=on-failure. +// Without the reset, too few slots remain and the third kill's restart is refused with +// "Start request repeated too quickly", failing this loop for the wrong reason. What is +// under test here is Restart=on-failure, not the rate limiter. +// +// Recovery requires a genuinely new MainPID: immediately after kill -9 systemd may still +// report the killed invocation as active/running until it processes SIGCHLD, so checking +// active/running alone can observe the old process and falsely declare recovery. +// +// NRestarts is asserted >=1, not 3, because reset-failed zeroes it too -- it therefore +// reports the final cycle only. The three cycles are already proven individually by the +// new-MainPID requirement; this is a last check that the final kill was recovered by +// Restart=on-failure and not by something else. +// +// The StartLimit journal check is scoped with --since to these cycles: deliberately +// exhausting the budget is the expected outcome of validateLocalDNSRestartBudget, which +// runs after this returns. The cgroup teardown warning is surfaced but not failed on -- +// fixing it is out of scope for this PR. +// +// # Terminal dead-service case +// +// The dead state is reached with a transient Restart=no drop-in and one kill, rather than +// by tripping StartLimit with rapid kills, which is timing-dependent and flaky. +// ExecStopPost runs on the SIGKILL path regardless of Restart=. +// +// Termination is awaited on ActiveState, not SubState: SubState passes through transitional +// values such as stop-post while ExecStopPost is still running the cleanup under test, so +// asserting on drop-in removal then would race it. ActiveState only becomes failed/inactive +// after ExecStopPost completes. +// +// The drop-in absence check uses 'sudo ls' because it concludes "absent" from a failed +// glob. If that directory were ever created root-only -- as this test's own drop-in +// directory is -- an unprivileged ls would fail, the check would read that as success, and +// the regression under test would pass silently. It is 0755 today; the assertion should not +// depend on that. +// +// The resolver check polls because networkctl reload propagates to systemd-resolved +// asynchronously, and it rejects empty or unreadable snapshots rather than treating them as +// "restored" -- a failed read would otherwise mask the regression. Removing the address is +// not sufficient on its own, so a working resolver is verified with getent afterwards. func validateLocalDNSLifecycle(ctx context.Context, s *Scenario) error { - _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, ` + expectExecStopPost := "true" + if laneResolvedMainBuiltImage() { + expectExecStopPost = "false" + } + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, + localdnsLifecycleScript(expectExecStopPost), 0, "LocalDNS lifecycle validation failed") + return err +} + +// localdnsLifecycleScript renders the script validateLocalDNSLifecycle runs on the node. +// Split out from the caller so TestLocalDNSScriptsFitBastionLimit can measure the assembled +// result -- see that test and this file's doc comment for why the size matters. +func localdnsLifecycleScript(expectExecStopPost string) string { + return ` set -eu +EXPECT_EXECSTOPPOST=` + expectExecStopPost + ` + NORESTART=/run/systemd/system/localdns.service.d/99-e2e-no-restart.conf -# Install cleanup before any service mutation so set -e cannot leave the node -# with the temporary Restart=no override or a failed LocalDNS unit. restore_localdns_test_state() { test_status=$? trap - EXIT set +e cleanup_status=0 - if [ -f "$NORESTART" ]; then - 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; } - fi - # The restart loop can hit systemd's start limit without creating NORESTART. - # Clear any failed state before trying to start LocalDNS; this is best-effort - # so a reset failure does not prevent the rest of cleanup. - sudo systemctl reset-failed localdns.service || true - 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; } + # Unconditional: an unprivileged [ -f ] cannot stat inside the 0750 root-owned dir. + 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; } + if sudo test -f "$NORESTART"; then + echo "ERROR: $NORESTART still present after cleanup" + cleanup_status=1 fi + # Retry: an orphaned CoreDNS can still hold 169.254.10.10:53 for a moment. + for cleanup_attempt in 1 2 3 4 5 6; do + sudo systemctl is-active --quiet localdns.service && break + sudo systemctl reset-failed localdns.service || true + sudo systemctl start localdns.service && break + sleep 3 + done if ! sudo systemctl is-active --quiet localdns.service; then echo "ERROR: localdns.service is not active after test cleanup" + sudo systemctl status localdns.service --no-pager -l || true cleanup_status=1 fi if [ "$test_status" -eq 0 ] && [ "$cleanup_status" -ne 0 ]; then @@ -102,7 +222,7 @@ test "$control_group" = "/localdns.slice/localdns.service" || { exit 1 } -# Normal systemd stop must complete cleanup and return success. +# A normal stop must complete cleanup and return success. sudo systemctl restart localdns.service sudo systemctl is-active --quiet localdns.service sudo systemctl stop localdns.service @@ -110,18 +230,11 @@ test "$(sudo systemctl show localdns.service -p ActiveState --value)" = inactive sudo systemctl start localdns.service sudo systemctl is-active --quiet localdns.service -# Repeatedly kill the supervisor and wait for Restart=on-failure recovery. -# This verifies that systemd can restart LocalDNS; startup cleanup may restore -# DNS on this path, so it does not by itself validate the ExecStopPost fix. -# The terminal dead-service block below validates that fix directly. -# Require a genuinely new MainPID after each kill: immediately after kill -9, -# systemd may still report the killed invocation as active/running until it -# processes SIGCHLD, so checking active/running alone can observe the old -# process and falsely declare recovery. Save the killed PID and require the -# new MainPID to be nonzero and different from it. +# Kill the supervisor three times; each must recover with a new MainPID. test_start=$(date +%s) for i in 1 2 3; do + sudo systemctl reset-failed localdns.service || true killed=$(sudo systemctl show -p MainPID --value localdns.service) test "$killed" -gt 0 sudo kill -9 "$killed" @@ -139,11 +252,10 @@ for i in 1 2 3; do test "$recovered" = true done +# reset-failed zeroes NRestarts too, so this reports the final cycle only. restarts_after=$(sudo systemctl show localdns.service -p NRestarts --value) -# The loop above performs three kill/restart cycles. The manual start before -# the loop resets NRestarts to zero, so assert the absolute restart count. -test "$restarts_after" -ge 3 || { - echo "FAIL: expected >=3 systemd restarts, got $restarts_after" +test "$restarts_after" -ge 1 || { + echo "FAIL: expected >=1 systemd restart after the final kill, got $restarts_after" exit 1 } @@ -152,26 +264,25 @@ printf '%s\n' "$state" printf '%s\n' "$state" | grep -q '^ActiveState=active$' printf '%s\n' "$state" | grep -q '^SubState=running$' printf '%s\n' "$state" | grep -q '^Result=success$' -# The cgroup teardown warning is diagnostic only: fixing it is out of scope for -# this PR (which is about restoring node DNS after an unexpected exit), so we -# surface it but do not fail on it. +# Diagnostic only; not failed on. if sudo journalctl -u localdns.service --since "@$test_start" --no-pager | grep -q 'Failed to kill control group'; then echo "WARNING: LocalDNS cgroup teardown warning observed" fi +# Scoped by --since: budget exhaustion belongs to the restart-budget validation, not here. if sudo journalctl -u localdns.service --since "@$test_start" --no-pager | grep -q 'Start request repeated too quickly'; then - echo "LocalDNS reached systemd StartLimit" + echo "LocalDNS reached systemd StartLimit during the kill/recovery cycles" exit 1 fi dig +short +time=5 +tries=1 mcr.microsoft.com @169.254.10.10 | grep -q . -if sudo systemctl show localdns.service -p ExecStopPost --value | grep -q 'localdns.sh cleanup'; then -# Terminal dead-service case: this is the incident scenario the PR fixes. -# When localdns ends up dead (systemd exhausts restart attempts), ExecStopPost -# must still revert node DNS so the node does not keep pointing at the dead -# localdns listener (169.254.10.10). We reach the dead state deterministically -# by disabling auto-restart with a transient drop-in, then killing the -# supervisor -- tripping StartLimit via rapid kills is timing dependent and -# flaky. ExecStopPost runs on the SIGKILL path regardless of Restart=. +if [ "$EXPECT_EXECSTOPPOST" = true ]; then +# Assert the hook; do not use its presence as permission to look. +if ! sudo systemctl show localdns.service -p ExecStopPost --value | grep -q 'localdns.sh cleanup'; then + echo "FAIL: ExecStopPost=localdns.sh cleanup is missing from localdns.service" + sudo systemctl show localdns.service -p ExecStopPost || true + exit 1 +fi +# Terminal dead-service case: Restart=no drop-in, then one kill. sudo mkdir -p "$(dirname "$NORESTART")" printf '[Service]\nRestart=no\n' | sudo tee "$NORESTART" >/dev/null sudo systemctl daemon-reload @@ -180,11 +291,7 @@ dead_main=$(sudo systemctl show -p MainPID --value localdns.service) test "$dead_main" -gt 0 sudo kill -9 "$dead_main" -# Wait for the service to reach a terminal ActiveState (failed or inactive). -# A non-running SubState is not sufficient: SubState passes through transitional -# values such as stop-post while ExecStopPost is still running the cleanup under -# test, so asserting on drop-in removal then could race the cleanup. ActiveState -# only becomes failed/inactive after ExecStopPost has completed. +# ActiveState, not SubState: SubState passes through stop-post mid-cleanup. dead=false for attempt in 1 2 3 4 5 6 7 8 9 10 11 12; do active_state=$(sudo systemctl show localdns.service -p ActiveState --value) @@ -196,22 +303,13 @@ for attempt in 1 2 3 4 5 6 7 8 9 10 11 12; do done test "$dead" = true -# The localdns network drop-in must have been removed by ExecStopPost. This is -# the authoritative signal that DNS was reverted: the drop-in is what points the -# link's DNS at the localdns listener. -if ls /run/systemd/network/*.d/70-localdns.conf >/dev/null 2>&1; then +# sudo ls: "absent" concluded from a failed glob must not come from a permission error. +if sudo ls /run/systemd/network/*.d/70-localdns.conf >/dev/null 2>&1; then echo "FAIL: 70-localdns.conf still present after localdns died" exit 1 fi -# The live link DNS must no longer include the localdns node listener. This is -# eventually consistent: networkctl reload propagates to systemd-resolved -# asynchronously, so poll (like wait_for_localdns_removed_from_resolv_conf does) -# until the listener IP is gone rather than checking once. Prefer resolvectl -# (the per-link view the drop-in configures); fall back to the resolved stub. -# Only accept a successful, non-empty resolver snapshot: an errored or empty -# read must not be treated as "restored", or a failed read would mask the very -# regression under test. Retry those instead. +# Poll: networkctl reload reaches systemd-resolved asynchronously. Empty reads are retried. dns_reverted=false resolver_state_readable=false localdns_listener_present=false @@ -246,18 +344,13 @@ if [ "$dns_reverted" != true ]; then exit 1 fi -# Removing the LocalDNS address is not sufficient: verify the node has a -# working resolver after cleanup. +# Address removal alone is not sufficient; the node must actually resolve. if ! getent hosts mcr.microsoft.com >/dev/null 2>&1; then echo "FAIL: node cannot resolve DNS after localdns died" exit 1 fi else - echo "SKIP: VHD predates the ExecStopPost cleanup hook" + echo "SKIP: this lane resolved a main-built image, which predates the ExecStopPost cleanup hook" fi - -# The EXIT trap removes the temporary override and restores LocalDNS even if -# an assertion above exits the validation early. -`, 0, "LocalDNS lifecycle validation failed") - return err +` } diff --git a/e2e/scenario/scenario_localdns_restart_budget.go b/e2e/scenario/scenario_localdns_restart_budget.go new file mode 100644 index 00000000000..3ddb59eadcb --- /dev/null +++ b/e2e/scenario/scenario_localdns_restart_budget.go @@ -0,0 +1,775 @@ +package scenario + +import ( + "context" + "fmt" + "time" + + "github.com/Azure/agentbaker/e2e/config" + "github.com/Azure/agentbaker/e2e/logging" +) + +// LocalDNS restart-budget validation. +// +// localdns.service pins StartLimitIntervalSec=720 / StartLimitBurst=5 / RestartSec=2 so +// that unrecoverable failures terminate in 'failed' rather than restarting forever. Only +// the terminal state lets an OnFailure= handoff fire and gives NPD a stable state to +// observe, so the directives are load-bearing -- but nothing guarded them: the lifecycle +// scenario does three kill/recovery cycles and would still pass if they were deleted. +// +// These checks reproduce, in CI, a failure-mode matrix that was measured by hand on a live +// mode=Required nodepool (Ubuntu 24.04.4, systemd 255.4). Each fault drives a real code +// path in localdns.sh or a real systemd timeout rather than replacing the service with a +// stub, and each asserts that the unit reaches 'failed'. +// +// Scope note: the slow modes run on shortened unit clocks so the matrix fits the +// VMSS-scoped validation budget. That means this validates the mechanism -- each mode +// reaches 'failed' because the limiter refused a start after the burst -- not the shipped +// wall-clock durations, which are measured separately and recorded on the PR. +// +// Measured note that shapes the assertions: systemd does NOT report +// Result=start-limit-hit here. It reports the underlying cause -- exit-code for most +// modes, watchdog for the watchdog kill, timeout for the hung start -- and the journal +// shows "Start request repeated too quickly." followed by "Failed with result 'timeout'.". +// So the terminal state is asserted via ActiveState plus the journal refusal line plus the +// ExecStart count, and Result is printed as diagnostic only. + +// localdnsFault is one row of the failure-mode matrix. +type localdnsFault struct { + // token written to the fault file and matched by the injected hooks + name string + // human-readable description used in failure messages + label string + // seconds to wait for the unit to reach 'failed', from the measured time plus margin + deadlineSeconds int + // what this mode actually took when last measured on the gate, in seconds. Data rather + // than a comment so TestLocalDNSFaultMatrixFitsVMSSBudget can size the matrix against + // TestTimeoutVMSS: the healthy run costs the sum of these, and only a regressing mode + // costs its deadline. + measuredSeconds int +} + +// localdnsFaultMatrix is the full set of modes. +// +// Every fault is driven by a hook inside the patched localdns.sh rather than by a systemd +// drop-in. Inducing pre-flight and hung-start with ExecStartPre= would be simpler, but +// ExecStartPre short-circuits before localdns.sh runs, so the ExecStart counter would +// never increment and the burst assertion could not be made. Driving them from inside the +// script also keeps each fault on the real code path. +// +// The three slow modes run on shortened clocks (see installLocalDNSFaultHarness): at their +// shipped values they need ~19.6min between them, and the whole validation phase is bounded +// by TestTimeoutVMSS, which it shares with VM creation. The "shipped" column is what those +// modes take with the real clocks, measured by hand and recorded on the PR; the deadline is +// sized for the shortened run with roughly 2x headroom. +// +// mode shipped shortened by +// readytimeout 318s wait_for_localdns_ready args 60/60 -> 8/8 +// watchdog 370s WatchdogSec 60 -> 10 +// hungstart 625s TimeoutStartSec 90 -> 15, TimeoutStopSec 30 -> 5 +var localdnsFaultMatrix = []localdnsFault{ + {name: "preflight", label: "pre-flight abort", deadlineSeconds: 45, measuredSeconds: 11}, + {name: "resolvdrain", label: "resolv.conf never drains", deadlineSeconds: 80, measuredSeconds: 37}, + {name: "postready", label: "dies right after READY", deadlineSeconds: 120, measuredSeconds: 64}, + {name: "nopidfile", label: "PID file never appears", deadlineSeconds: 90, measuredSeconds: 30}, + {name: "readytimeout", label: "ready-check timeout", deadlineSeconds: 120, measuredSeconds: 75}, + {name: "watchdog", label: "watchdog pings cease", deadlineSeconds: 120, measuredSeconds: 90}, + {name: "hungstart", label: "hung start", deadlineSeconds: 180, measuredSeconds: 110}, +} + +// localdnsDiscriminatingFault is the single mode used on distros that do not carry the +// full matrix. +// +// It must be a mode whose restart cycle sits between the two thresholds: above the shipped +// default's interval/burst = 10/5 = 2s, and below this unit's 720/5 = 144s. resolv.conf +// drain (~5.5s cycle) is the cheapest such mode -- measured 55 restarts over 300s without +// ever reaching 'failed' under the default budget, versus 'failed' in 37s under 720/5. +// +// Pre-flight is deliberately NOT used here despite being faster: its ~0.4s cycle is below +// the default threshold too, so it reaches 'failed' with or without these directives and +// would guard nothing. +func localdnsDiscriminatingFault() []localdnsFault { + for _, f := range localdnsFaultMatrix { + if f.name == "resolvdrain" { + return []localdnsFault{f} + } + } + // unreachable unless the matrix above is edited; fail loudly rather than silently + // running an empty matrix. + return nil +} + +const ( + localdnsFaultScript = "/run/localdns-e2e.sh" + localdnsFaultFile = "/run/localdns-e2e-fault" + localdnsFaultCounter = "/run/localdns-e2e-starts" + localdnsFaultDropIn = "/run/systemd/system/localdns.service.d/99-e2e-fault.conf" + // Kept separate from localdnsFaultDropIn so the real-clock measurement can run with the + // shipped timeouts before the matrix speeds them up. + localdnsFastClockDropIn = "/run/systemd/system/localdns.service.d/99-e2e-fastclock.conf" +) + +// localdnsWorstCycleCeilingSeconds bounds measureLocalDNSWorstCycle's poll. Exported as a +// constant so TestLocalDNSFaultMatrixFitsVMSSBudget can size the whole validation against +// TestTimeoutVMSS at build time. +const localdnsWorstCycleCeilingSeconds = 180 + +// laneResolvedMainBuiltImage reports whether this lane asked for an image built from main, +// rather than one built from the branch under test. +// +// This is the gate for the restart-budget validation, and it deliberately reads the lane's +// own image selection rather than probing the node for the directives. Probing the node +// would key the skip on a value the validation itself asserts, so the assertion could only +// run on images where it was already guaranteed to pass -- and a later retune of the budget +// would silently turn the whole scenario off on every lane instead of failing it. +// +// The gate lane sets SIG_VERSION_TAG_NAME=buildId / SIG_VERSION_TAG_VALUE= +// from VHD_BUILD_ID (.pipelines/scripts/e2e_run.sh:83-88) and therefore tests this PR's own +// VHDs. Everything else falls back to the default branch=refs/heads/main selector, which +// resolves a main-built image that legitimately still ships the systemd default 10s/5 +// budget: localdns.service is baked into the VHD (vhdbuilder/packer/packer_source.sh), not +// delivered through CustomData. .pipelines/e2e.yaml triggers on any PR touching e2e/** and +// runs in exactly that configuration. +// +// A lane pointed at some other branch's image asserts rather than skips: if you asked for a +// specific branch's VHD, a missing budget there is a real failure worth hearing about. +func laneResolvedMainBuiltImage() bool { + return config.Config.SIGVersionTagName == "branch" && + config.Config.SIGVersionTagValue == "refs/heads/main" +} + +// validateLocalDNSRestartBudget asserts that the unit's restart budget terminates each +// supplied failure mode in 'failed', and leaves LocalDNS healthy afterwards. +func validateLocalDNSRestartBudget(ctx context.Context, s *Scenario, faults []localdnsFault) error { + if len(faults) == 0 { + return fmt.Errorf("no LocalDNS faults selected: the fault matrix is misconfigured") + } + + if laneResolvedMainBuiltImage() { + logging.Logf(ctx, "SKIP: this lane resolved a main-built image (%s=%s), which predates the "+ + "LocalDNS restart budget baked into the VHD; run against the PR's VHD build to exercise it", + config.Config.SIGVersionTagName, config.Config.SIGVersionTagValue) + return nil + } + + if err := assertLocalDNSBudgetDirectives(ctx, s); err != nil { + return err + } + if err := assertLocalDNSSurvivesProvisioningRestarts(ctx, s); err != nil { + return err + } + + if err := installLocalDNSFaultHarness(ctx, s); err != nil { + return fmt.Errorf("install LocalDNS fault harness: %w", err) + } + // Always tear the harness down, including on early return, so the node is not left + // running the patched script or a temporary drop-in. This runs even when ctx is already + // cancelled -- a scenario deadline or a dropped exec is exactly the path where cleanup + // matters most, and reusing ctx would make this return immediately, leaving the fault + // file, the patched script and the drop-in behind for whatever runs next. WithoutCancel + // keeps the exec's credentials and values; the timeout stops teardown hanging forever. + defer func() { + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 3*time.Minute) + defer cancel() + _, _ = execScriptOnVMForScenario(cleanupCtx, s, localdnsFaultTeardownScript) + }() + + // Ground truth before anything is sped up: measure one real failure cycle against the + // shipped timeouts and check it fits under the threshold. The margin assertion above is + // arithmetic on a model of the unit; this is the machine's own answer. The matrix below + // runs on shortened clocks and cannot check this. + if err := measureLocalDNSWorstCycle(ctx, s); err != nil { + return err + } + + if err := installLocalDNSFastClocks(ctx, s); err != nil { + return fmt.Errorf("install shortened fault clocks: %w", err) + } + + for _, fault := range faults { + if err := runLocalDNSFault(ctx, s, fault); err != nil { + return err + } + } + return nil +} + +// installLocalDNSFastClocks shortens WatchdogSec, TimeoutStartSec and TimeoutStopSec for the +// matrix. +// +// At their shipped values (60s, 90s and 30s) the watchdog and hung-start modes need ~16 +// minutes between them to exhaust the burst, which does not fit the VMSS-scoped validation +// budget that TestTimeoutVMSS shares with VM creation. Shortened, the same code paths still +// run -- a real watchdog timeout, a real start timeout, SIGTERM, restart, limiter -- on a +// faster clock. +// +// TimeoutStopSec is shortened for a non-obvious reason: hung-start spends its stop phase in +// full. The fault holds a foreground 'sleep infinity', and localdns.sh now traps TERM, so +// bash queues the handler until the foreground child returns -- which it never does. Nothing +// answers the SIGTERM and systemd waits out TimeoutStopSec before SIGKILLing the cgroup on +// every cycle. Before the TERM trap existed an untrapped SIGTERM killed bash immediately and +// the stop phase cost ~6s, which is why the by-hand measurement recorded on the PR (487s) +// is lower than what this now costs shipped (~625s). At 30s the shortened cycle is 47s and +// five of them do not fit any sane deadline; at 5s it is 22s, which is what the matrix's +// hungstart deadline is sized against. +// +// StartLimitIntervalSec, StartLimitBurst and RestartSec are never touched: they are what is +// under test. The sizing this shortening stops exercising is covered by the margin +// assertion and by measureLocalDNSWorstCycle, both of which run before this. +func installLocalDNSFastClocks(ctx context.Context, s *Scenario) error { + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, ` +set -eu +printf '[Service]\nWatchdogSec=10\nTimeoutStartSec=15\nTimeoutStopSec=5\n' | sudo tee `+localdnsFastClockDropIn+` >/dev/null +sudo systemctl daemon-reload +echo "shortened fault clocks installed" +`, 0, "failed to install the shortened fault clocks") + return err +} + +// measureLocalDNSWorstCycle times one real restart cycle of the slowest failure mode and +// asserts it fits under the budget's threshold. +// +// This is the design claim checked against reality rather than against the formula. Waiting +// for the full burst would cost ~8 minutes; two consecutive ExecStarts give the cycle +// length, which is the quantity the budget is sized against, in about 100 seconds. +func measureLocalDNSWorstCycle(ctx context.Context, s *Scenario) error { + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, localdnsWorstCycleScript, 0, + "LocalDNS worst restart cycle does not fit under the start-limit threshold") + return err +} + +var localdnsWorstCycleScript = ` +set -eu + +usec() { systemd-analyze timespan "$1" 2>/dev/null | sed -n '2s/.*: *//p'; } +interval_us=$(usec "$(systemctl show localdns.service -p StartLimitIntervalUSec --value)") +burst_n=$(systemctl show localdns.service -p StartLimitBurst --value) +threshold_s=$(( interval_us / burst_n / 1000000 )) + +echo "=== measuring the real worst restart cycle (hung start, shipped clocks) ===" +echo "threshold is ${threshold_s}s" + +sudo systemctl stop localdns.service 2>/dev/null || true +sudo systemctl reset-failed localdns.service 2>/dev/null || true +echo hungstart | sudo tee ` + localdnsFaultFile + ` >/dev/null +sudo rm -f ` + localdnsFaultCounter + ` +sudo systemctl start --no-block localdns.service || true + +# Two ExecStarts is all that is needed. Worst case here is TimeoutStartSec 90 + +# TimeoutStopSec 30 + RestartSec 2 = ~122s. The ceiling is deliberately close to that +# rather than generous: this poll shares TestTimeoutVMSS with VM creation and the whole +# fault matrix, and if the scenario runs out of budget the VMSS timeout pre-empts the +# per-fault deadlines and you lose the specific diagnostic they exist to produce. On the +# success path it breaks early anyway. Keep it in step with localdnsWorstCycleCeilingSeconds. +observed=0 +for i in $(seq 1 ` + fmt.Sprint(localdnsWorstCycleCeilingSeconds) + `); do + observed=$(sudo grep -c EXECSTART ` + localdnsFaultCounter + ` 2>/dev/null || echo 0) + [ "$observed" -ge 2 ] && break + sleep 1 +done + +# Abort the run: the remaining starts would only repeat the same cycle. +sudo rm -f ` + localdnsFaultFile + ` +sudo systemctl stop localdns.service 2>/dev/null || true +sudo systemctl reset-failed localdns.service 2>/dev/null || true + +if [ "$observed" -lt 2 ]; then + echo "FAIL: only observed $observed ExecStart(s) in 240s; cannot measure a restart cycle" + sudo systemctl status localdns.service --no-pager -l || true + exit 1 +fi + +t1=$(sudo sed -n '1p' ` + localdnsFaultCounter + ` | awk '{print $1}') +t2=$(sudo sed -n '2p' ` + localdnsFaultCounter + ` | awk '{print $1}') +cycle_s=$(( t2 - t1 )) +echo "measured worst restart cycle: ${cycle_s}s (threshold ${threshold_s}s)" + +if [ "$cycle_s" -ge "$threshold_s" ]; then + echo "FAIL: the slowest failure cycle (${cycle_s}s) is not under the threshold (${threshold_s}s)." + echo " A failure on this cycle never accumulates StartLimitBurst starts inside the" + echo " window, so it would restart forever instead of terminating in 'failed'." + exit 1 +fi + +# Leave the service healthy for the matrix that follows. +sudo systemctl start localdns.service +for attempt in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15; do + sudo systemctl is-active --quiet localdns.service && break + sleep 1 +done +sudo systemctl is-active --quiet localdns.service || { + echo "FAIL: localdns did not recover after the worst-cycle measurement" + exit 1 +} +` + +// assertLocalDNSBudgetDirectives checks the effective directives rather than the file, so +// a drop-in that quietly overrides them is caught too. +func assertLocalDNSBudgetDirectives(ctx context.Context, s *Scenario) error { + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, localdnsDirectiveAssertScript, 0, + "LocalDNS restart-budget directives are not as expected") + return err +} + +const localdnsDirectiveAssertScript = ` +set -eu + +fail=0 +check() { + actual=$(systemctl show localdns.service -p "$1" --value) + if [ "$actual" != "$2" ]; then + echo "FAIL: expected $1=$2, got $1=$actual" + fail=1 + fi +} + +# The budget under test. +check StartLimitIntervalUSec "12min" +check StartLimitBurst "5" +check RestartUSec "2s" + +# Restart= is not part of this PR, but the whole matrix depends on it: with Restart=no the +# unit goes terminal after a single start and every fault would report "reached 'failed' +# without the start limiter refusing a start". An earlier e2e validation leaving a +# Restart=no drop-in behind caused exactly that, so assert it here where the message is +# unambiguous rather than letting each fault fail confusingly. +check Restart "on-failure" + +# The threshold is StartLimitIntervalSec/StartLimitBurst = 720/5 = 144s, which only clears +# the slowest restart cycle at TimeoutStartSec=90s. That is pinned in the unit rather than +# inherited, because the manager default is a systemd build-time constant that differs by +# image -- Ubuntu builds it at 90s, Azure Linux at 45s -- so an inherited value made the +# margin, and this assertion, distro-dependent. Assert the pin so dropping it fails here. +check TimeoutStartUSec "1min 30s" + +# The other half of the same problem, and the one that actually bit. Azure Linux ships a +# global service.d drop-in setting TimeoutStopFailureMode=abort, which makes a stop timeout +# send SIGABRT and then wait a SECOND TimeoutStopSec before SIGKILL. The stop side of the +# cycle is doubled, and the arithmetic below -- which models it as one TimeoutStopSec -- has +# no way to see that. Measured on AzureLinuxV3: 153s against a 144s threshold. The unit pins +# 'terminate' to take the distro's drop-in out of the cycle; assert the pin, because if it is +# dropped the margin check below still reports a comfortable 122s while the machine is at 153s. +check TimeoutStopFailureMode "terminate" + +# The design rule itself, as arithmetic on the live values rather than on the numbers this +# comment happens to quote. +# +# threshold = StartLimitIntervalSec / StartLimitBurst +# worst cycle = TimeoutStartSec + TimeoutStopSec + RestartSec +# +# A failure repeating more slowly than the threshold never accumulates StartLimitBurst +# starts inside the window, so it restarts forever instead of reaching 'failed' -- exactly +# the behaviour this PR exists to prevent. Checking every input this way means any change +# to the interval, the burst, either timeout, or RestartSec that closes the margin fails +# here, instead of silently shipping a budget that cannot catch the slow modes. +usec() { systemd-analyze timespan "$1" 2>/dev/null | sed -n '2s/.*: *//p'; } + +interval_us=$(usec "$(systemctl show localdns.service -p StartLimitIntervalUSec --value)") +tstart_us=$(usec "$(systemctl show localdns.service -p TimeoutStartUSec --value)") +tstop_us=$(usec "$(systemctl show localdns.service -p TimeoutStopUSec --value)") +restart_us=$(usec "$(systemctl show localdns.service -p RestartUSec --value)") +burst_n=$(systemctl show localdns.service -p StartLimitBurst --value) + +if [ -z "$interval_us" ] || [ -z "$tstart_us" ] || [ -z "$tstop_us" ] || [ -z "$restart_us" ] \ + || [ -z "$burst_n" ] || [ "$burst_n" -le 0 ] 2>/dev/null; then + echo "FAIL: could not read the restart-budget inputs needed for the margin check" + echo " interval=[$interval_us] burst=[$burst_n] tstart=[$tstart_us] tstop=[$tstop_us] restart=[$restart_us]" + fail=1 +else + threshold_us=$(( interval_us / burst_n )) + worst_us=$(( tstart_us + tstop_us + restart_us )) + echo "margin: threshold $((threshold_us/1000000))s vs worst cycle $((worst_us/1000000))s" + if [ "$threshold_us" -le "$worst_us" ]; then + echo "FAIL: threshold $((threshold_us/1000000))s does not clear the worst restart cycle $((worst_us/1000000))s" + echo " (TimeoutStartSec $((tstart_us/1000000))s + TimeoutStopSec $((tstop_us/1000000))s + RestartSec $((restart_us/1000000))s)" + echo " A failure on that cycle would restart forever rather than terminate in 'failed'." + fail=1 + fi +fi + +if [ "$fail" -ne 0 ]; then + echo + echo "localdns.service is baked into the VHD (vhdbuilder/packer/packer_source.sh), not" + echo "delivered through CustomData, so these directives only exist on an image built" + echo "from a branch that carries them." + echo + echo "This validation is gated on the lane, not on the node: laneResolvedMainBuiltImage()" + echo "skips it when the lane asked for a main-built image. Reaching this message therefore" + echo "means the lane asked for THIS branch's VHD and the image it got does not have the" + echo "budget -- either it was removed, a drop-in overrode it, or it is inherited on this" + echo "distro when the unit should be pinning it." + echo + echo "TimeoutStartUSec and TimeoutStopFailureMode are both pinned by the unit precisely" + echo "because their defaults are systemd build-time constants that differ by image:" + echo "Ubuntu builds the start timeout at 90s and Azure Linux at 45s, and Azure Linux also" + echo "ships a global TimeoutStopFailureMode=abort drop-in that doubles the stop side." +fi + +exit "$fail" +` + +// assertLocalDNSSurvivesProvisioningRestarts models what CSE does at provisioning time. +// +// enableLocalDNS() runs 'systemctl reset-failed localdns' before each start attempt +// because manual restarts draw on the same StartLimitBurst as automatic ones: without the +// reset, one CSE start plus Restart=on-failure drains all five slots in ~11s and every +// later attempt is refused for the rest of the 720s window. This asserts a CSE-shaped loop +// keeps the unit startable. +func assertLocalDNSSurvivesProvisioningRestarts(ctx context.Context, s *Scenario) error { + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, localdnsProvisioningRestartScript, 0, + "LocalDNS could not survive a CSE-style provisioning restart loop") + return err +} + +const localdnsProvisioningRestartScript = ` +set -eu + +for i in 1 2 3 4 5 6; do + sudo systemctl reset-failed localdns.service 2>/dev/null || true + # timeout 30 mirrors the production path exactly (cse_config_localdns.sh). A longer + # bound here would let a restart that takes 30-60s pass this test while still failing + # node provisioning, which is the regression this is meant to catch. + if ! sudo timeout 30 systemctl restart localdns.service; then + echo "FAIL: provisioning-style restart $i was refused" + sudo systemctl status localdns.service --no-pager -l || true + exit 1 + fi +done + +state=$(sudo systemctl show localdns.service -p ActiveState --value) +if [ "$state" != active ]; then + echo "FAIL: localdns is $state after six reset-failed+restart cycles" + exit 1 +fi +echo "provisioning-style restart loop OK" +` + +// installLocalDNSFaultHarness builds a patched copy of the shipped localdns.sh with the +// fault hooks inserted, and points a transient drop-in at it. +// +// The drop-in deliberately overrides only ExecStart: StartLimitIntervalSec, +// StartLimitBurst, RestartSec and TimeoutStartSec continue to come from the shipped unit, +// because those are the thing under test. +func installLocalDNSFaultHarness(ctx context.Context, s *Scenario) error { + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, localdnsFaultHarnessInstallScript, 0, + "failed to install the LocalDNS fault harness") + return err +} + +const localdnsFaultHarnessInstallScript = ` +set -eu + +SRC=/opt/azure/containers/localdns/localdns.sh +DST=` + localdnsFaultScript + ` +FAULT=` + localdnsFaultFile + ` +COUNTER=` + localdnsFaultCounter + ` +DROPIN=` + localdnsFaultDropIn + ` +WORK=$(mktemp -d) + +sudo test -f "$SRC" +# 'sudo cat > file' rather than 'sudo cp': cp creates the copy root-owned and, under +# root's umask, mode 0750. This script runs unprivileged (hence sudo everywhere), so the +# greps and awks below could not read a root-only copy -- grep would fail with permission +# denied, print nothing to stdout, and the anchor count would come back empty rather than +# a number. Redirecting makes the copy owned by the calling user. +sudo cat "$SRC" > "$WORK/script" + +# insert_hook +# +# Matches the anchor as a whole line and requires exactly one occurrence. If localdns.sh +# moves, this fails loudly rather than silently inserting nothing and leaving a test that +# always passes. +insert_hook() { + anchor=$1; pos=$2; blockfile=$3 + count=$(grep -c -x -F "$anchor" "$WORK/script" 2>/dev/null || true) + # Distinguish "could not read the file" from "did not match": an unreadable file makes + # grep print nothing, leaving count empty, which otherwise surfaces as a confusing + # "matched lines" and looks like a missing anchor. + case "$count" in + ''|*[!0-9]*) + echo "ANCHOR-FAIL: could not count '$anchor' in $WORK/script (got [$count])" + ls -la "$WORK/script" || true + exit 1 + ;; + esac + if [ "$count" != "1" ]; then + echo "ANCHOR-FAIL: '$anchor' matched $count lines in localdns.sh, expected exactly 1" + echo "localdns.sh has changed; refresh the e2e fault anchors." + exit 1 + fi + awk -v anchor="$anchor" -v pos="$pos" -v bf="$blockfile" ' + BEGIN { while ((getline line < bf) > 0) block = block line "\n" } + $0 == anchor { + if (pos == "before") printf "%s", block + print + if (pos == "after") printf "%s", block + next + } + { print } + ' "$WORK/script" > "$WORK/next" + mv "$WORK/next" "$WORK/script" +} + +# (1) Authoritative ExecStart counter, plus the pre-flight fault. +# +# The counter is written by the script itself on every invocation. Counting starts by +# grepping the journal for "Starting ..." is unreliable here and produced wrong numbers +# during the manual investigation, so it is deliberately not used. +cat > "$WORK/b1" <<'HOOK' +# --- e2e fault harness --- +LDNS_FAULT="$(cat /run/localdns-e2e-fault 2>/dev/null || echo none)" +echo "$(date +%s) EXECSTART fault=${LDNS_FAULT}" >> /run/localdns-e2e-starts +if [ "${LDNS_FAULT}" = "preflight" ]; then + echo "E2EFAULT preflight: pre-flight check failed" + exit 1 +fi +# --- end e2e fault harness --- +HOOK +insert_hook 'regenerate_localdns_corefile || exit $ERR_LOCALDNS_COREFILE_NOTFOUND' before "$WORK/b1" + +# (2) resolv.conf never drains: the real 5s wait times out. +cat > "$WORK/b2" <<'HOOK' + if [ "$(cat /run/localdns-e2e-fault 2>/dev/null)" = "resolvdrain" ]; then + echo "E2EFAULT resolvdrain: resolv.conf never drains"; sleep 5; return 1 + fi +HOOK +insert_hook 'wait_for_localdns_removed_from_resolv_conf() {' after "$WORK/b2" + +# (3) Watchdog pings cease: the real WatchdogSec=60 fires. +cat > "$WORK/b3" <<'HOOK' + if [ "$(cat /run/localdns-e2e-fault 2>/dev/null)" = "watchdog" ]; then + echo "E2EFAULT watchdog: ceasing watchdog pings" + while true; do sleep 5; done + fi +HOOK +insert_hook 'start_localdns_watchdog() {' after "$WORK/b3" + +# (3b) Shorten the readiness wait for the readytimeout mode only. The real polling loop +# still runs and still times out on its own; it just does not need a full 60s per cycle to +# prove the point. +cat > "$WORK/b3b" <<'HOOK' + if [ "$(cat /run/localdns-e2e-fault 2>/dev/null)" = "readytimeout" ]; then + maxattempts=8; timeout_duration=8 + fi +HOOK +insert_hook ' local timeout_duration=$2' after "$WORK/b3b" + +# (4) PID file never appears: repoint the wait loop at a path CoreDNS will not create so +# the real START_LOCALDNS_TIMEOUT=10 loop times out. COREDNS_COMMAND has already been built +# from the real path, so CoreDNS itself still behaves normally. +cat > "$WORK/b4" <<'HOOK' +if [ "${LDNS_FAULT}" = "nopidfile" ]; then + echo "E2EFAULT nopidfile: pid file will never appear" + LOCALDNS_PID_FILE=/run/localdns-e2e-never-appears.pid + # the real wait loop still runs, on a shorter clock + START_LOCALDNS_TIMEOUT=3 +fi +HOOK +insert_hook 'start_localdns || exit $ERR_LOCALDNS_FAIL' before "$WORK/b4" + +# (5) Readiness never succeeds: kill CoreDNS so the real wait_for_localdns_ready 60 60 +# polls and times out on its own. +cat > "$WORK/b5" <<'HOOK' +if [ "${LDNS_FAULT}" = "readytimeout" ]; then + echo "E2EFAULT readytimeout: killing coredns so readiness never succeeds" + kill -9 "${COREDNS_PID}" 2>/dev/null || true +fi +HOOK +insert_hook 'start_localdns || exit $ERR_LOCALDNS_FAIL' after "$WORK/b5" + +# (6) Hang before READY so the real TimeoutStartSec fires. +cat > "$WORK/b6" <<'HOOK' +if [ "${LDNS_FAULT}" = "hungstart" ]; then + echo "E2EFAULT hungstart: hanging before READY" + sleep infinity +fi +HOOK +insert_hook ' systemd-notify --ready' before "$WORK/b6" + +# (7) Die immediately after READY, before the watchdog loop starts. +cat > "$WORK/b7" <<'HOOK' +if [ "${LDNS_FAULT}" = "postready" ]; then + echo "E2EFAULT postready: exiting immediately after READY" + exit 1 +fi +HOOK +insert_hook 'start_localdns_watchdog' before "$WORK/b7" + +bash -n "$WORK/script" || { echo "FAIL: patched localdns.sh is not valid bash"; exit 1; } + +sudo install -m 0755 "$WORK/script" "$DST" +sudo rm -f "$FAULT" "$COUNTER" +rm -rf "$WORK" + +# Point the unit at the patched copy. Invoking through bash avoids any noexec concern on +# /run. ExecStart= clears the shipped value before setting the replacement. +sudo mkdir -p "$(dirname "$DROPIN")" +# ExecStart only. The clock shortening lives in a separate drop-in installed later, so the +# real-clock worst-cycle measurement can run against the shipped timeouts first. +printf '[Service]\nExecStart=\nExecStart=/bin/bash %s\n' "$DST" | sudo tee "$DROPIN" >/dev/null +sudo systemctl daemon-reload +echo "fault harness installed" +` + +// localdnsFaultTeardownScript removes the harness and restores a healthy LocalDNS. It is +// best-effort by design: it runs from a defer, including on the failure path. +const localdnsFaultTeardownScript = ` +set -u +sudo rm -f ` + localdnsFaultFile + ` ` + localdnsFaultCounter + ` ` + localdnsFaultDropIn + ` ` + localdnsFastClockDropIn + ` ` + localdnsFaultScript + ` +sudo systemctl daemon-reload || true +sudo systemctl reset-failed localdns.service || true +sudo systemctl start localdns.service || true +for attempt in 1 2 3 4 5 6 7 8 9 10 11 12; do + if sudo systemctl is-active --quiet localdns.service; then + break + fi + sleep 1 +done +sudo systemctl is-active --quiet localdns.service || echo "WARNING: localdns is not active after fault teardown" +` + +// runLocalDNSFault arms one fault, waits for the unit to give up, and asserts the terminal +// state before restoring the service for the next mode. +func runLocalDNSFault(ctx context.Context, s *Scenario, fault localdnsFault) error { + _, err := execScriptOnVMForScenarioValidateExitCode(ctx, s, localdnsFaultRunScript(fault), 0, + fmt.Sprintf("LocalDNS restart budget did not bound the %q failure mode", fault.label)) + if err != nil { + return fmt.Errorf("fault %s (%s): %w", fault.name, fault.label, err) + } + return nil +} + +// localdnsFaultRunScript renders the per-fault script. +// +// # The ExecStart count is bounded on both sides, with deliberate slack +// +// Fewer starts than the burst means the unit gave up for some reason other than the +// limiter. Many more means the budget is not being enforced at all — which the journal +// refusal check cannot catch on its own, since a unit that restarted twenty times and then +// tripped the limiter still prints "Start request repeated too quickly". +// +// The upper bound is burst+1 rather than exactly burst. Copilot has twice recommended +// tightening it to equality; before doing so, note that the two measurements disagree: +// +// by hand, live node (PR #9439, 2026-09-17): postready = 6 ExecStarts, others 5 +// CI, gate build 181777373 (Ubuntu2404): postready = 5, every mode 5 +// +// The 6 has not been reproduced and nothing explains it. It is one observation from a real +// node, so tolerate it rather than ship a lane that flakes only when it recurs. The slack +// costs little — a limiter allowing exactly one extra start would pass — against a gate +// cycle lost to a flake. If you can explain or reproduce the 6, tighten this and record why +// here. Do not tighten it on the CI numbers alone; that is precisely what is in dispute. +// +// The hungstart count in that CI run is 6, but it is not evidence either way: +// measureLocalDNSWorstCycle runs the hungstart fault before the matrix, so journal lines for +// that mode conflate both phases. Only $COUNTER, cleared per fault, distinguishes them — +// anyone counting "E2EFAULT hungstart" in a journal will get a wrong answer. +// +// # Self-reference +// +// $burst is read from the live unit, so this compares the SUT against itself. That is only +// sound because assertLocalDNSBudgetDirectives has already pinned StartLimitBurst to 5 +// earlier in the same validation. If that assertion is removed, or reordered to after this +// point, this check silently stops meaning anything. +func localdnsFaultRunScript(fault localdnsFault) string { + return ` +set -eu + +FAULT=` + fault.name + ` +LABEL="` + fault.label + `" +DEADLINE=` + fmt.Sprint(fault.deadlineSeconds) + ` +COUNTER=` + localdnsFaultCounter + ` + +echo "=== fault: $FAULT ($LABEL), deadline ${DEADLINE}s ===" + +sudo systemctl stop localdns.service 2>/dev/null || true +sudo systemctl daemon-reload +# Start each mode from a fresh budget so the previous mode's spent slots cannot make this +# one terminate early and pass for the wrong reason. +sudo systemctl reset-failed localdns.service 2>/dev/null || true + +burst=$(systemctl show localdns.service -p StartLimitBurst --value) + +echo "$FAULT" | sudo tee ` + localdnsFaultFile + ` >/dev/null +sudo rm -f "$COUNTER" +since=$(date '+%Y-%m-%d %H:%M:%S') + +# --no-block: a hung start would otherwise hold this call for TimeoutStartSec. +sudo systemctl start --no-block localdns.service || true + +elapsed=0 +state="" +while [ "$elapsed" -lt "$DEADLINE" ]; do + state=$(sudo systemctl show localdns.service -p ActiveState --value) + if [ "$state" = failed ]; then + break + fi + sleep 1 + elapsed=$((elapsed + 1)) +done + +starts=$(sudo grep -c EXECSTART "$COUNTER" 2>/dev/null || echo 0) +result=$(sudo systemctl show localdns.service -p Result --value) +substate=$(sudo systemctl show localdns.service -p SubState --value) +echo "fault=$FAULT elapsed=${elapsed}s state=$state sub=$substate result=$result execstarts=$starts burst=$burst" + +if [ "$state" != failed ]; then + echo "FAIL: $LABEL did not terminate in 'failed' within ${DEADLINE}s (state=$state)." + echo " The restart budget is not bounding this failure mode; it would restart forever." + sudo journalctl -u localdns.service --since "$since" --no-pager | tail -40 || true + exit 1 +fi + +# The refusal line is the authoritative signal that the limiter -- not some unrelated +# failure -- is what ended the restart loop. Result= is NOT checked: systemd reports the +# underlying cause here (exit-code / watchdog / timeout), never start-limit-hit. +if ! sudo journalctl -u localdns.service --since "$since" --no-pager | grep -q 'Start request repeated too quickly'; then + echo "FAIL: $LABEL reached 'failed' without the start limiter refusing a start." + echo " Something other than the restart budget produced the terminal state." + echo " ExecStarts observed: $starts (expected the burst of $burst)." + echo "--- unit properties ---" + sudo systemctl show localdns.service -p Restart -p RestartUSec -p StartLimitIntervalUSec \ + -p StartLimitBurst -p NRestarts -p Result -p ExecMainStatus --no-pager || true + echo "--- journal for this fault ---" + sudo journalctl -u localdns.service --since "$since" --no-pager | tail -40 || true + echo "--- drop-ins in effect ---" + ls -la /run/systemd/system/localdns.service.d/ 2>/dev/null || true + exit 1 +fi + +# Bounded on both sides: [burst, burst+1]. See localdnsFaultRunScript's doc comment for +# why the upper bound has slack and why tightening it to equality needs new evidence. +if [ "$starts" -lt "$burst" ]; then + echo "FAIL: $LABEL ran $starts ExecStarts, expected at least the burst of $burst" + exit 1 +fi +if [ "$starts" -gt $((burst + 1)) ]; then + echo "FAIL: $LABEL ran $starts ExecStarts, expected at most the burst of $burst (+1 slack)." + echo " The limiter let the unit restart past its budget; it is not being enforced." + exit 1 +fi + +echo "OK: $LABEL terminated in 'failed' after $starts ExecStarts in ${elapsed}s" + +# Restore for the next mode: drop the fault, clear the budget, and confirm the service and +# node DNS actually come back. +sudo rm -f ` + localdnsFaultFile + ` +sudo systemctl reset-failed localdns.service || true +sudo systemctl start localdns.service +for attempt in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15; do + if sudo systemctl is-active --quiet localdns.service; then + break + fi + sleep 1 +done +if ! sudo systemctl is-active --quiet localdns.service; then + echo "FAIL: localdns did not recover after fault $FAULT" + sudo systemctl status localdns.service --no-pager -l || true + exit 1 +fi +if ! dig +short +time=5 +tries=1 mcr.microsoft.com @169.254.10.10 | grep -q .; then + echo "FAIL: node listener is not answering after recovery from fault $FAULT" + exit 1 +fi +` +} diff --git a/e2e/scenario/scenario_localdns_restart_budget_test.go b/e2e/scenario/scenario_localdns_restart_budget_test.go new file mode 100644 index 00000000000..ed110eab9a3 --- /dev/null +++ b/e2e/scenario/scenario_localdns_restart_budget_test.go @@ -0,0 +1,94 @@ +package scenario + +import ( + "testing" + "time" + + "github.com/Azure/agentbaker/e2e/config" +) + +// localdnsNonMatrixAllowance is the time the LocalDNS scenarios must leave for everything +// that is not the fault matrix: VMSS create, node bootstrap, the scenario's default +// provisioning validation, validateLocalDNSLifecycle, and the provisioning-restart loop. +// +// Measured on the check-in gate (build 181710671, LocalDNSHostsPlugin/Ubuntu2404): VM +// creation and the default validations finished at 181s, and the lifecycle validation plus +// the provisioning-restart loop added roughly another 100s. 300s is that with margin, +// because creation time is the part we do not control. +const localdnsNonMatrixAllowance = 300 * time.Second + +// TestLocalDNSFaultMatrixFitsVMSSBudget fails the build when the fault matrix can no longer +// finish inside TestTimeoutVMSS. +// +// This exists because going over the budget does not fail like a sizing error. The VMSS +// context deadline pre-empts the per-fault deadline, so instead of the specific diagnostic +// the matrix is built to produce -- "hung start did not terminate in 'failed' within 180s", +// which says the restart budget stopped bounding that mode -- you get a generic scenario +// timeout that says nothing. Every per-fault deadline is carefully sized, and all of that +// work is wasted the moment the sum stops fitting. +// +// The model is "a healthy run plus one regression", not "every deadline fires at once": +// +// allowance + worst-cycle ceiling + sum(measured) + max(deadline - measured) +// +// A mode only costs its deadline when it is broken, and the first broken mode aborts the +// run, so at most one overrun is ever paid. Summing all seven deadlines would instead +// assert a state that can never be reached and would force the matrix to be cut for no +// reason. What this does guarantee is the property that matters: if any single mode +// regresses, its deadline fires and reports itself before the VMSS context expires. +func TestLocalDNSFaultMatrixFitsVMSSBudget(t *testing.T) { + vmssBudget := config.DefaultConfiguration().TestTimeoutVMSS + + var healthy time.Duration + var worstOverrun time.Duration + for _, fault := range localdnsFaultMatrix { + if fault.measuredSeconds <= 0 { + t.Errorf("fault %q has no measuredSeconds; the matrix cannot be sized against "+ + "TestTimeoutVMSS without it. Run the mode on the gate and record what it took.", + fault.name) + continue + } + if fault.deadlineSeconds <= fault.measuredSeconds { + t.Errorf("fault %q has deadlineSeconds=%d at or below measuredSeconds=%d; it will "+ + "fail on a healthy node", fault.name, fault.deadlineSeconds, fault.measuredSeconds) + continue + } + healthy += time.Duration(fault.measuredSeconds) * time.Second + if overrun := time.Duration(fault.deadlineSeconds-fault.measuredSeconds) * time.Second; overrun > worstOverrun { + worstOverrun = overrun + } + } + + // The worst-cycle measurement runs before the matrix and can spend its full poll. + ceiling := localdnsWorstCycleCeilingSeconds * time.Second + total := localdnsNonMatrixAllowance + ceiling + healthy + worstOverrun + + if total > vmssBudget { + t.Errorf( + "LocalDNS restart-budget validation cannot fit TestTimeoutVMSS.\n"+ + " non-matrix allowance: %v (VM create + default validation + lifecycle)\n"+ + " worst-cycle ceiling: %v\n"+ + " healthy matrix run: %v (%d modes)\n"+ + " worst single overrun: %v\n"+ + " total: %v\n"+ + " TestTimeoutVMSS: %v\n"+ + "Over by %v. Reduce a deadline, drop a fault from localdnsFaultMatrix, lower "+ + "localdnsWorstCycleCeilingSeconds, or move the matrix to its own scenario. Do not "+ + "raise the allowance without a measurement to back it.", + localdnsNonMatrixAllowance, ceiling, healthy, len(localdnsFaultMatrix), + worstOverrun, total, vmssBudget, total-vmssBudget, + ) + } +} + +// TestLocalDNSDiscriminatingFaultIsInMatrix guards localdnsDiscriminatingFault's lookup. +// +// It returns nil if the matrix no longer contains the mode it names, and a nil fault list +// makes validateLocalDNSRestartBudget fail every non-Ubuntu2404 lane with a confusing +// "fault matrix is misconfigured" at runtime. Catch a rename here instead. +func TestLocalDNSDiscriminatingFaultIsInMatrix(t *testing.T) { + if got := localdnsDiscriminatingFault(); len(got) != 1 { + t.Fatalf("localdnsDiscriminatingFault() returned %d faults, want exactly 1; "+ + "the mode it looks up is no longer in localdnsFaultMatrix", len(got)) + } +} diff --git a/e2e/scenario/scenario_localdns_script_size_test.go b/e2e/scenario/scenario_localdns_script_size_test.go new file mode 100644 index 00000000000..aef6b386f3c --- /dev/null +++ b/e2e/scenario/scenario_localdns_script_size_test.go @@ -0,0 +1,137 @@ +package scenario + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "testing" +) + +// bastionMaxScriptBytes is the largest script this package can send to a node. +// +// Scripts are SCP'd to the VM over the Bastion tunnel, and tunnelSession.Write +// (bastionssh.go) forwards whatever the SSH transport hands it as a single websocket +// message with no chunking. Azure Bastion caps an inbound message at 8,192 bytes and closes +// the tunnel with StatusMessageTooBig when one exceeds it — taking the whole scenario down +// mid-run, with no indication that script length was the cause. +// +// The constant below is the last safe script size, measured by sweeping sizes one byte at a +// time against a real x/crypto/ssh client and server. SSH pads each binary packet up to a +// multiple of the cipher block size, so the wire write plateaus and then steps -- there is +// no script size that lands exactly on the 8,192 cap: +// +// 12236..12242 -> 8180 +// 12243..12248 -> 8196 <- first over 8,192 +// +// Do not derive this constant arithmetically from the script size. An earlier version of +// this comment did exactly that and was wrong twice: off by 11 bytes at the plateau, and +// implying a 1-byte granularity the transport does not have. +// +// Gate build 181818040 lost three LocalDNS lanes to exactly this: the lifecycle script had +// been sitting 380 bytes under the cliff and grew 512 bytes, producing an 8,324-byte write. +// +// The right long-term fix is chunking in tunnelSession.Write, which is shared e2e +// infrastructure and out of scope here. Until that lands, this test is the guard. +const bastionMaxScriptBytes = 12242 + +// TestLocalDNSScriptsFitBastionLimit fails the build when any script this package sends +// would kill the Bastion tunnel. +// +// It covers every script, not just the one that broke, because the failure mode gives no +// hint about its cause: the scenario dies with a websocket close frame and loses its node +// logs, so whoever hits it next starts from "the node became unreachable" rather than +// "my script got too long". Cheaper to fail here. +// +// If a script does outgrow the limit, prefer moving its comments into Go — they cost the +// same on the wire as code and buy nothing at runtime. validateLocalDNSLifecycle's doc +// comment is the worked example. +// localdnsScriptsUnderTest is the single inventory of scripts this package sends to a node, +// keyed by declaration name. Both tests below read it, so registering a script and +// size-checking it are the same edit -- there is no way to do one without the other. +// +// Two lists would not be equivalent: a script added to one and missed in the other passes +// both tests while going unchecked, which is the exact failure this is here to prevent. +func localdnsScriptsUnderTest() map[string][]string { + faultRunScripts := make([]string, 0, len(localdnsFaultMatrix)) + for _, fault := range localdnsFaultMatrix { + faultRunScripts = append(faultRunScripts, localdnsFaultRunScript(fault)) + } + return map[string][]string{ + "localdnsLifecycleScript": {localdnsLifecycleScript("true"), localdnsLifecycleScript("false")}, + "localdnsDirectiveAssertScript": {localdnsDirectiveAssertScript}, + "localdnsWorstCycleScript": {localdnsWorstCycleScript}, + "localdnsProvisioningRestartScript": {localdnsProvisioningRestartScript}, + "localdnsFaultHarnessInstallScript": {localdnsFaultHarnessInstallScript}, + "localdnsFaultTeardownScript": {localdnsFaultTeardownScript}, + "localdnsFaultRunScript": faultRunScripts, + } +} + +func TestLocalDNSScriptsFitBastionLimit(t *testing.T) { + scripts := map[string]string{} + for declaration, rendered := range localdnsScriptsUnderTest() { + for i, script := range rendered { + name := declaration + if len(rendered) > 1 { + name = fmt.Sprintf("%s[%d]", declaration, i) + } + scripts[name] = script + } + } + + for name, script := range scripts { + if len(script) > bastionMaxScriptBytes { + t.Errorf( + "%s is %d bytes, over the %d-byte Bastion tunnel limit by %d.\n"+ + "This will not fail as a size error: the tunnel closes with "+ + "StatusMessageTooBig, the scenario reports a dead SSH session, and node log "+ + "collection fails too. Move the script's comments into Go rather than "+ + "deleting them — they cost the same on the wire and do nothing at runtime.", + name, len(script), bastionMaxScriptBytes, len(script)-bastionMaxScriptBytes, + ) + } + } + + if t.Failed() || testing.Verbose() { + for name, script := range scripts { + fmt.Printf(" %-46s %6d bytes (%d headroom)\n", + name, len(script), bastionMaxScriptBytes-len(script)) + } + } +} + +// TestLocalDNSScriptInventoryIsRegistered fails the build when a script is declared but not +// size-checked. +// +// TestLocalDNSScriptsFitBastionLimit covers every script someone remembered to add to its +// map, which is not the same thing. An eighth script lands silently, and the failure it then +// hits is the one with no diagnostic attached — a dead tunnel and no node logs. So derive +// the inventory from the source rather than restating it. +func TestLocalDNSScriptInventoryIsRegistered(t *testing.T) { + registered := localdnsScriptsUnderTest() + declaration := regexp.MustCompile(`(?m)^(?:var|const|func) (localdns\w*Script)\b`) + // Glob rather than a literal file list: go test runs in the package directory, so this + // covers new scenario files on arrival instead of only when someone remembers to add + // them here. Known gap it cannot close -- the regex keys on the localdns*Script naming + // convention, so a script sent through execScriptOnVMForScenario* under another name is + // still unguarded (validate_localdns_exporter_metrics.go does this today). + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatal(err) + } + for _, file := range files { + src, err := os.ReadFile(file) + if err != nil { + t.Fatalf("read %s: %v", file, err) + } + for _, match := range declaration.FindAllStringSubmatch(string(src), -1) { + if _, ok := registered[match[1]]; !ok { + t.Errorf("%s is declared in %s but is not in localdnsScriptsUnderTest, so it is "+ + "never size-checked and can outgrow the Bastion limit unnoticed. Add it "+ + "there -- that one edit both registers and size-checks it.", + match[1], file) + } + } + } +} diff --git a/parts/linux/cloud-init/artifacts/cse_config_localdns.sh b/parts/linux/cloud-init/artifacts/cse_config_localdns.sh index 7c159ac5a09..b8a3d1400c0 100644 --- a/parts/linux/cloud-init/artifacts/cse_config_localdns.sh +++ b/parts/linux/cloud-init/artifacts/cse_config_localdns.sh @@ -107,7 +107,104 @@ enableLocalDNS() { fi echo "localdns should be enabled." - systemctlEnableAndStart localdns 30 || exit $ERR_LOCALDNS_FAIL + # localdns.service budgets StartLimitBurst=5 / StartLimitIntervalSec=720 so steady-state + # failures terminate in 'failed' for NPD to observe. Provisioning restarts draw on that same + # budget -- a manual restart costs a slot just like an automatic one -- so clear it before each + # attempt. Otherwise the first burst wedges the unit for 12 minutes and every retry below is + # refused with "Start request repeated too quickly". daemon-reload is not a substitute: it + # clears start_ratelimit on systemd 249 but not on 255 (Ubuntu 24.04). + local localdns_started=false + local i + # 100 matches what systemctlEnableAndStart did before (systemctl_restart 100 5 30, + # cse_helpers.sh), but it is a backstop, not a budget: check_cse_timeout below decides, + # and only the fastest failure shape ever reaches 100. Do not reason about the loop's + # duration from the count. + # + # Measured against a Type=notify unit with this loop's exact shape (reset-failed, + # timeout 30 systemctl restart, sleep 5). Cost per iteration is dominated by how long + # the start takes to fail, because restart blocks until ready-or-failed: + # + # failure shape per iteration iterations in 780s + # fails at once (pre-flight) 5s ~156 + # fails after ~8s (resolv.conf drain) 13s ~60 + # hangs, capped by 'timeout 30' 35s ~22 + # + # And that is with localdns as the only consumer of the budget, which it never is, so + # the real counts are lower. + # + # That guard is what keeps the slow case safe: if every restart hangs for its full 30s + # timeout, the loop would outlive CSE's 15m kill in cse_start.sh and be SIGKILLed + # mid-iteration, losing the status log and the exit code below. Breaking out early lets + # the give-up path run and report properly, matching the other retry loops in + # cse_helpers.sh. + # + # Every systemd call here is wrapped in timeout, as _systemctl_retry_svc_operation did. + # These all talk to PID 1 over D-Bus; an unbounded one that wedges would never return to + # the top of the loop, so check_cse_timeout above would never be re-evaluated and CSE + # would be SIGKILLed before it could report. + local localdns_giveup_reason="exhausted the restart attempts" + # Hoisted out of the loop: nothing here rewrites a unit between iterations, and + # daemon-reload re-parses every unit on the box. _systemctl_retry_svc_operation did it + # per attempt, but the point of inlining this loop was to stop paying for what that + # helper did blindly. + timeout 30 systemctl daemon-reload + for i in $(seq 1 100); do + if ! check_cse_timeout; then + localdns_giveup_reason="CSE provisioning budget exhausted at attempt ${i}" + break + fi + timeout 30 systemctl reset-failed localdns 2>/dev/null || true + if timeout 30 systemctl restart localdns; then + localdns_started=true + break + fi + # Periodic, bounded diagnostics. systemctlEnableAndStart used to dump 'systemctl status' + # plus an unbounded 'journalctl -u' on every failed attempt (shouldLogRetryInfo=true in + # _systemctl_retry_svc_operation), which is ~99 dumps and a measured 6-8s per iteration -- + # enough to eat most of the provisioning window on its own. Dropping it entirely lost the + # only record of what was going wrong across the retries, so sample it instead: every + # tenth attempt, with the journal bounded by -n. + if [ $((i % 10)) -eq 0 ]; then + echo "localdns restart attempt ${i} failed; unit state and recent journal follow." + timeout 30 systemctl status localdns --no-pager -l || true + timeout 30 journalctl -u localdns --no-pager -n 50 || true + fi + sleep 5 + done + if [ "${localdns_started}" != "true" ]; then + echo "localdns could not be started: ${localdns_giveup_reason}." + # No reset here -- the last failure's auto-restarts land the unit in 'failed', which is the + # terminal state NPD needs. + # + # This snapshot is taken ~5s after the last failed restart, so the unit is normally + # 'activating (auto-restart)' rather than 'failed': the loop cleared the start-limit + # counter on every iteration, so it cannot have accumulated toward the terminal state yet. + # That is why the journal is captured alongside it -- the status line alone describes a + # unit mid-cycle and does not explain why any of the attempts failed. + timeout 30 systemctl status localdns --no-pager -l > /var/log/azure/localdns-status.log || true + timeout 30 journalctl -u localdns --no-pager -n 200 >> /var/log/azure/localdns-status.log || true + exit $ERR_LOCALDNS_FAIL + fi + # Log on this path too. systemctlEnableAndStart wrote a status log when 'systemctl enable' + # failed as well as when the start failed; inlining the loop kept the start path and dropped + # this one, so an enable failure exited with nothing but the code. + # Capture the code rather than using 'if ! ...': '!' inverts before $? is read, so the + # distinction is gone inside the then-block. retrycmd_if_failure returns 2 when + # check_cse_timeout trips (cse_helpers.sh:270, :298) and 1 when it genuinely exhausts + # its attempts. Reporting the first as a systemd failure sends the on-call after the + # wrong thing -- same reason localdns_giveup_reason exists for the start loop above. + retrycmd_if_failure 120 5 25 systemctl enable localdns + local enable_rc=$? + if [ "$enable_rc" -ne 0 ]; then + if [ "$enable_rc" -eq 2 ]; then + echo "localdns could not be enabled: CSE provisioning budget exhausted." + else + echo "localdns could not be enabled by systemctl." + fi + timeout 30 systemctl status localdns --no-pager -l > /var/log/azure/localdns-status.log || true + timeout 30 journalctl -u localdns --no-pager -n 200 >> /var/log/azure/localdns-status.log || true + exit $ERR_LOCALDNS_FAIL + fi echo "Enable localdns succeeded." # Exporter socket setup is deferred to configureLocalDNSExporterSocket() (after ensureKubelet) # to avoid delaying kubelet start. The kubelet node label is added separately in cse_main.sh. diff --git a/parts/linux/cloud-init/artifacts/localdns-delegate.conf b/parts/linux/cloud-init/artifacts/localdns-delegate.conf index 8fd08355ca5..0a5587b6e5e 100644 --- a/parts/linux/cloud-init/artifacts/localdns-delegate.conf +++ b/parts/linux/cloud-init/artifacts/localdns-delegate.conf @@ -1,2 +1,5 @@ [Service] -Delegate=cpu \ No newline at end of file +# Makes systemd enable the cpu controller in this unit's cgroup subtree, so +# /sys/fs/cgroup/localdns.slice/localdns.service/cpu.stat exists for the CPU metrics +# localdns.sh exports. +Delegate=cpu diff --git a/parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf b/parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf new file mode 100644 index 00000000000..7d77ff68266 --- /dev/null +++ b/parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf @@ -0,0 +1,31 @@ +# Installed to /etc/systemd/system/localdns.service.d/99-timeout-terminate.conf. +# +# This directive cannot live in localdns.service. Azure Linux ships a type-wide drop-in at +# /usr/lib/systemd/system/service.d/10-timeout-abort.conf setting +# TimeoutStopFailureMode=abort, and systemd.unit(5) states that drop-ins take precedence +# over unit files wherever located. Setting it in the unit is silently ignored there. +# +# With abort in effect, a stop timeout sends SIGABRT to capture a core dump and then waits +# a SECOND TimeoutStopSec in 'stop-watchdog' before SIGKILL. That doubles the stop side of +# the restart cycle: measured on AzureLinuxV3, 90 + 30 + 30 + 2 = 153s, above the 144s +# threshold (StartLimitIntervalSec/StartLimitBurst in localdns.service). Slow failure modes +# would then restart forever instead of reaching 'failed' -- exactly what that budget exists +# to prevent. Pinned back to the systemd default the cycle is 90 + 30 + 2 = 122s everywhere. +# +# A unit-specific drop-in wins over a type-wide one because they are separate hierarchies +# and systemd applies the unit-specific one last -- not because of the 99- prefix. Verified: +# a type-wide file named 99- (sorting after this one) still loses. The prefix is for human +# readers, so this file reads as "applied late, on purpose". +# +# Kept separate from delegate.conf rather than folded into it: that file is named for +# Delegate=cpu, and whoever comes back asking why the stop timeout behaves this way will not +# think to open a file about cgroup delegation. +# +# The core dump abort exists to produce is already discarded on AKS nodes -- configureCoreDump() +# in cis.sh sets Storage=none and ProcessSizeMax=0 for every distro, asserted by +# testCoreDumpSettings in linux-vhd-content-test.sh. So today that step costs 30s of restart +# cycle and produces nothing. If that hardening is relaxed, revisit: the 30s would have to be +# bought back elsewhere in the cycle or the threshold widened, or Azure Linux goes back +# over 144s. +[Service] +TimeoutStopFailureMode=terminate diff --git a/parts/linux/cloud-init/artifacts/localdns.service b/parts/linux/cloud-init/artifacts/localdns.service index c6ffe9f6c84..8c4684a658e 100644 --- a/parts/linux/cloud-init/artifacts/localdns.service +++ b/parts/linux/cloud-init/artifacts/localdns.service @@ -7,16 +7,57 @@ Before=kubelet.service Before=containerd.service # don't run on old images; we're not compatible ConditionKernelVersion=>=5.15 +# Bound the restart budget so every unrecoverable failure mode terminates in +# 'failed' rather than restarting forever. The threshold is +# StartLimitIntervalSec/StartLimitBurst = 720/5 = 144s, just above the slowest +# restart cycle (~122s = TimeoutStartSec 90 + TimeoutStopSec 30 + RestartSec 2, pinned +# below, plus TimeoutStopFailureMode=terminate pinned in the drop-in -- without which +# Azure Linux spends two stop timeouts and the cycle is 153s, above the threshold), +# so slow failures (PID file never appears, watchdog kill, ready-check timeout, +# hung start) still reach 'failed'. That terminal state is what lets an +# OnFailure= handoff fire and gives NPD a stable state to observe. +# +# This budget is shared with CSE's provisioning restarts, not just systemd's own +# auto-restart cycles: a manual 'systemctl start/restart' counts against +# StartLimitBurst exactly like an automatic restart. A single CSE start therefore +# costs one slot and lets Restart=on-failure spend the remaining four within +# ~11s, after which every further CSE attempt is refused with "Start request +# repeated too quickly" for the rest of the 720s window. enableLocalDNS() runs +# 'systemctl reset-failed localdns' before each provisioning attempt to keep that +# from wedging node provisioning, and deliberately skips it on the give-up path +# so the unit is left in 'failed'. Don't rely on 'systemctl daemon-reload' to +# clear this instead: it resets start_ratelimit on systemd 249 but not on 255 +# (Ubuntu 24.04), which is what AKS ships. +StartLimitIntervalSec=720 +StartLimitBurst=5 [Service] Type=notify NotifyAccess=all WatchdogSec=60 Restart=on-failure +# Load-bearing: give each restart time to sample real system state instead of +# re-entering the transient it is retrying against. cleanup_iptables_and_dns +# runs 'networkctl reload' on every start, and the dummy interface carrying the +# node (169.254.10.10) and cluster (169.254.10.11) listeners is deleted and +# recreated on every start. At sub-second retries these overlap; 2s lets +# networkd settle and an orphaned coredns release its sockets before ExecStart. +RestartSec=2 KillMode=mixed # Revert node DNS configuration even when the supervisor exits unexpectedly. ExecStopPost=/opt/azure/containers/localdns/localdns.sh cleanup +# Pinned, not inherited, so the StartLimit margin above is self-contained. The +# manager default is a systemd build-time constant (-Ddefault-timeout-sec) and it +# is not the same on every image we ship: Ubuntu builds it at 90s, Azure Linux at +# 45s. Leaving it inherited makes the slowest restart cycle -- and therefore the +# margin under the 144s threshold -- differ per distro, and a change to +# DefaultTimeoutStartSec would move it silently. +TimeoutStartSec=90 TimeoutStopSec=30 +# TimeoutStopFailureMode is pinned too, but it cannot be set here: Azure Linux ships a +# type-wide drop-in that overrides unit files. It lives in localdns-timeout-terminate.conf, +# installed as /etc/systemd/system/localdns.service.d/99-timeout-terminate.conf -- see the +# comment there for why, and for the 153s cycle it prevents. Slice=localdns.slice EnvironmentFile=-/etc/localdns/environment ExecStart=/opt/azure/containers/localdns/localdns.sh diff --git a/parts/linux/cloud-init/artifacts/localdns.sh b/parts/linux/cloud-init/artifacts/localdns.sh index 9eabc85dae0..51e2924b240 100644 --- a/parts/linux/cloud-init/artifacts/localdns.sh +++ b/parts/linux/cloud-init/artifacts/localdns.sh @@ -41,6 +41,9 @@ LOCALDNS_NODE_LISTENER_IP="169.254.10.10" LOCALDNS_CLUSTER_LISTENER_IP="169.254.10.11" # Localdns shutdown delay. +# Paid on every exit that finds CoreDNS still running, which since the SIGTERM trap +# (see the trap block near the end of this file) includes every systemd stop, not just +# the script's own error exits. Measured contribution to a stop: 5-6s of a 6-8s restart. LOCALDNS_SHUTDOWN_DELAY=5 # Localdns pid file. @@ -883,6 +886,23 @@ export_resource_metrics() { # The health check is a DNS request to the localdns service IPs. # The health check is run at 20% of the WATCHDOG_USEC interval. # If the health check fails, the script will exit and systemd will restart the service. +# Kill the backgrounded watchdog sleep, if one is in flight. +# +# start_localdns_watchdog waits by backgrounding 'sleep' and waiting on it, so that a +# signal can interrupt the wait rather than being deferred until the sleep returns. With +# KillMode=mixed systemd sends SIGTERM to this script only, so without killing the child +# explicitly the sleep survives as an orphan in the unit's cgroup: systemd then holds the +# unit in stop-sigterm until the sleep finishes on its own (up to HEALTH_CHECK_INTERVAL) +# or TimeoutStopSec expires and it is SIGKILLed -- delaying the stop, and with it the +# next start. +stop_watchdog_sleep() { + if [ -n "${WATCHDOG_SLEEP_PID:-}" ]; then + kill "${WATCHDOG_SLEEP_PID}" 2>/dev/null || true + wait "${WATCHDOG_SLEEP_PID}" 2>/dev/null || true + WATCHDOG_SLEEP_PID="" + fi +} + start_localdns_watchdog() { if [ -n "${NOTIFY_SOCKET:-}" ] && [ -n "${WATCHDOG_USEC:-}" ]; then # Health check at 20% of WATCHDOG_USEC; this means that we should check. @@ -937,8 +957,14 @@ start_localdns_watchdog() { # Update resource metrics .prom file for the exporter (best-effort, non-fatal) export_resource_metrics - # Wait for the next watchdog interval. - sleep "${HEALTH_CHECK_INTERVAL}" + # Wait for the next watchdog interval. Run sleep in a child so SIGTERM can + # interrupt the wait and let the service's signal/exit cleanup run promptly. + # The pid is recorded so the SIGTERM handler can reap the child instead of + # leaving it in the cgroup holding the unit's stop open. + sleep "${HEALTH_CHECK_INTERVAL}" & + WATCHDOG_SLEEP_PID=$! + wait "${WATCHDOG_SLEEP_PID}" + WATCHDOG_SLEEP_PID="" done else # No watchdog configured — write metrics once then wait for CoreDNS to exit @@ -1081,6 +1107,40 @@ build_localdns_iptable_rules trap 'echo "Error occurred. Cleaning up..."; cleanup_localdns_configs; exit $ERR_LOCALDNS_FAIL' ABRT ERR INT PIPE # Always cleanup when exiting. +# SIGTERM is how systemd stops this unit. It was previously untrapped, so bash died on the +# default action: the EXIT cleanup below never ran, and any in-flight watchdog sleep was +# left orphaned in the cgroup. Handle it so the child is reaped and cleanup runs. Exit 0 +# because a requested stop is not a failure -- Restart=on-failure must not fire for it. +# +# Trapping SIGTERM changes what a systemd stop does, and the change is deliberate. 'exit 0' +# fires the EXIT trap below, so cleanup_localdns_configs now runs IN-PROCESS on every stop, +# where previously bash died first and only ExecStopPost ran. Two consequences, both +# measured on a node rather than reasoned about (journal from gate build 181777373, +# AzureLinuxV3, n=11 stops): +# +# 1. A stop takes 5-6s instead of being immediate. Essentially all of it is +# LOCALDNS_SHUTDOWN_DELAY (:44) draining connections before CoreDNS is SIGINTed. +# Measured stop 5-6s, start 1-2s, so a full restart is 6-8s. The tightest bound on +# that path is 'timeout 30 systemctl restart localdns' in enableLocalDNS() +# (cse_config_localdns.sh) and its e2e mirror, leaving ~22-24s of headroom. It does +# not affect the restart-budget cycle math either: the worst cycle is a hung start, +# where this trap cannot run at all because bash defers a trapped signal until the +# foreground command returns, so systemd spends the full TimeoutStopSec regardless. +# +# 2. cleanup_localdns_configs runs to completion, so the dummy interface carrying +# 169.254.10.10/.11 is now torn down on a stop. It was not before: localdns_cleanup_mode +# (the ExecStopPost path) deliberately leaves the link alone in case an orphaned CoreDNS +# is still answering on .11. Both paths are now consistent for a clean stop, and the +# teardown/recreate cycle was clean in the same run (12 teardowns, 18 setups, zero +# address-in-use or RTNETLINK errors). Note this for the pod-DNS fallback (#9486): its +# idempotent-interface-creation requirement is now the common case on a clean stop, not +# the exception. +# +# The graceful path is kept rather than trimmed because the cost is affordable at the only +# bound that matters and the behaviour is better than the alternative -- without it CoreDNS +# is SIGKILLed by the cgroup on every stop, dropping in-flight queries. +trap 'echo "Received SIGTERM, shutting down."; stop_watchdog_sleep; exit 0' TERM + trap 'echo "Executing cleanup function."; cleanup_localdns_configs || echo "Cleanup failed with error code: $ERR_LOCALDNS_FAIL."' EXIT # Configure interface listening on Node listener and cluster listener IPs. diff --git a/spec/parts/linux/cloud-init/artifacts/cse_config_localdns_spec.sh b/spec/parts/linux/cloud-init/artifacts/cse_config_localdns_spec.sh index c57a5fe7b70..64099f736ef 100755 --- a/spec/parts/linux/cloud-init/artifacts/cse_config_localdns_spec.sh +++ b/spec/parts/linux/cloud-init/artifacts/cse_config_localdns_spec.sh @@ -37,10 +37,40 @@ Describe 'cse_config_localdns.sh' touch /etc/systemd/system/localdns.service touch /opt/azure/containers/localdns/localdns.sh - systemctlEnableAndStart() { - echo "systemctlEnableAndStart $@" + # enableLocalDNS's retry loop calls check_cse_timeout, which warns on stderr + # when this is unset. Set it so the real guard is exercised (elapsed ~0s, well + # under CSE_MAX_DURATION_SECONDS) instead of taking its unset short-circuit. + CSE_STARTTIME_SECONDS=$(date +%s) + + # enableLocalDNS drives systemd directly rather than going through + # systemctlEnableAndStart, so it can clear the StartLimit budget + # between attempts. Mock the primitives it actually calls. + systemctl() { + echo "systemctl $*" + return 0 + } + timeout() { + shift + "$@" + } + retrycmd_if_failure() { + echo "retrycmd_if_failure $*" + return 0 + } + # enableLocalDNS captures the journal alongside 'systemctl status' on the failure + # paths. Mock it so the assertions are deterministic and no real journal is read. + journalctl() { + echo "journalctl $*" return 0 } + # The give-up and enable-failure paths redirect into this directory. Without it the + # redirect fails before the command runs and the '|| true' hides it, so the tests + # would pass without ever exercising the capture. + mkdir -p /var/log/azure + rm -f /var/log/azure/localdns-status.log + sleep() { + : + } systemctlEnableAndStartNoBlock() { echo "systemctlEnableAndStartNoBlock $@" return 0 @@ -88,14 +118,113 @@ Describe 'cse_config_localdns.sh' The output should not include "localdns should be enabled." End - It 'should return error when systemctl fails to start localdns' - systemctlEnableAndStart() { - echo "systemctlEnableAndStart $@" - return 1 + # Record an ordered trace of the calls that matter: R for reset-failed, S for a + # start attempt. Asserting on the trace is what pins "before *each* attempt" -- a + # test that only checks both strings appear somewhere would still pass if + # reset-failed were hoisted out of the loop. + tracing_systemctl() { + systemctl() { + case "$1" in + reset-failed) printf 'R' >> "$TMP_DIR/trace" ;; + restart) printf 'S' >> "$TMP_DIR/trace" ;; + esac + echo "systemctl $*" + if [ "$1" = "restart" ]; then + restart_calls=$((restart_calls + 1)) + if [ "$restart_calls" -lt "$restart_failures_before_success" ]; then + return 1 + fi + fi + return 0 } + restart_calls=0 + } + + It 'should reset the StartLimit budget before every attempt, not only the first' + # Two failed restarts then a success, so the loop runs three times. + restart_failures_before_success=3 + tracing_systemctl + When run enableLocalDNS + The status should be success + The output should include "Enable localdns succeeded." + # R before every S, three times over -- not RSSS. + The contents of file "$TMP_DIR/trace" should equal "RSRSRS" + End + + It 'should return error when systemctl fails to start localdns' + # Never succeeds, so the loop exhausts and takes the give-up path. + restart_failures_before_success=99999 + tracing_systemctl When run enableLocalDNS The status should equal 216 The output should include "localdns should be enabled." + The output should include "systemctl reset-failed localdns" + # The give-up path deliberately does not reset, so the unit is left in 'failed' + # for NPD: the trace must end on a start attempt, never on a reset. + The contents of file "$TMP_DIR/trace" should end with "S" + End + + It 'should say why it gave up and capture the journal, not just the unit state' + # systemctlEnableAndStart logged 'systemctl status' plus 'journalctl -u' on every + # failed attempt; inlining the loop dropped all of it. The snapshot alone shows a + # unit mid-restart-cycle and does not explain any of the failures, so the journal + # has to come with it. + restart_failures_before_success=99999 + tracing_systemctl + When run enableLocalDNS + The status should equal 216 + The output should include "localdns could not be started: exhausted the restart attempts." + The contents of file /var/log/azure/localdns-status.log should include "journalctl -u localdns" + End + + It 'should report a CSE budget give-up differently from exhausting the attempts' + # The two give-up reasons need different messages: one means localdns is broken, + # the other means provisioning ran out of time and never finished trying. + restart_failures_before_success=99999 + tracing_systemctl + check_cse_timeout() { return 1; } + When run enableLocalDNS + The status should equal 216 + The output should include "CSE provisioning budget exhausted at attempt 1" + The output should not include "exhausted the restart attempts" + End + + It 'should sample diagnostics during the retries without dumping on every attempt' + # Bounded and periodic on purpose: the old helper dumped status plus an unbounded + # journal on all 99 failed attempts, measured at 6-8s per iteration, which ate the + # provisioning window it was retrying inside. + restart_failures_before_success=11 + tracing_systemctl + When run enableLocalDNS + The status should be success + The output should include "localdns restart attempt 10 failed" + The output should include "journalctl -u localdns --no-pager -n 50" + The output should not include "localdns restart attempt 9 failed" + The output should include "Enable localdns succeeded." + End + + It 'should log and capture status when systemctl enable fails' + # systemctlEnableAndStart wrote a status log on the enable-failure path as well as + # the start-failure path. Inlining the loop kept the first and dropped the second, + # so an enable failure exited with nothing but the code. + retrycmd_if_failure() { return 1; } + When run enableLocalDNS + The status should equal 216 + The output should include "localdns could not be enabled by systemctl." + The output should not include "Enable localdns succeeded." + The contents of file /var/log/azure/localdns-status.log should include "journalctl -u localdns" + End + + It 'should distinguish a CSE budget timeout from a genuine enable failure' + # retrycmd_if_failure returns 2 when check_cse_timeout trips and 1 when it burns + # all its attempts. 'if ! retrycmd ...' would throw that away -- '!' inverts before + # $? is read -- and report a budget timeout as a systemd failure, sending the + # on-call after the wrong thing. + retrycmd_if_failure() { return 2; } + When run enableLocalDNS + The status should equal 216 + The output should include "localdns could not be enabled: CSE provisioning budget exhausted." + The output should not include "could not be enabled by systemctl" End End Describe 'enableLocalDNSForScriptless' @@ -113,10 +242,29 @@ Describe 'cse_config_localdns.sh' touch /etc/systemd/system/localdns.service touch /opt/azure/containers/localdns/localdns.sh - systemctlEnableAndStart() { - echo "systemctlEnableAndStart $@" + # enableLocalDNS's retry loop calls check_cse_timeout, which warns on stderr + # when this is unset. Set it so the real guard is exercised (elapsed ~0s, well + # under CSE_MAX_DURATION_SECONDS) instead of taking its unset short-circuit. + CSE_STARTTIME_SECONDS=$(date +%s) + + # enableLocalDNS drives systemd directly rather than going through + # systemctlEnableAndStart, so it can clear the StartLimit budget + # between attempts. Mock the primitives it actually calls. + systemctl() { + echo "systemctl $*" return 0 } + timeout() { + shift + "$@" + } + retrycmd_if_failure() { + echo "retrycmd_if_failure $*" + return 0 + } + sleep() { + : + } systemctlEnableAndStartNoBlock() { echo "systemctlEnableAndStartNoBlock $@" return 0 diff --git a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh index cc231860374..912f51731a0 100644 --- a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh +++ b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh @@ -1328,6 +1328,77 @@ EOF End +# This section tests the signal traps installed at the bottom of localdns.sh. +#--------------------------------------------------------------------------------------------------- +# These cannot be reached with Include: they sit below "${__SOURCED__:+return}", which is what +# stops sourcing from running the main block. The trap wiring is nonetheless the thing that +# decides what a systemd stop does -- a stop sends SIGTERM, and whether that reaps the watchdog +# sleep, runs cleanup in-process, and exits 0 is entirely determined by those three lines. +# +# So extract the real trap lines out of the shipped script and execute them, rather than +# restating them here. A spec that restated them would pass even if the traps were deleted, +# which is the failure mode worth avoiding: it would report healthy while a stop reverted to +# bash dying on the default action, leaving the watchdog sleep orphaned in the cgroup and +# cleanup to ExecStopPost alone. + Describe 'signal traps' + LOCALDNS_SRC="./parts/linux/cloud-init/artifacts/localdns.sh" + + # Build a harness containing the real trap lines plus stubs for what they call, so the + # assertions below are about the shipped text and not about a copy of it. + build_trap_harness() { + harness=$(mktemp) + { + echo '#!/bin/bash' + echo 'cleanup_localdns_configs() { echo "CLEANUP_RAN"; return 0; }' + echo 'stop_watchdog_sleep() { echo "WATCHDOG_REAPED"; }' + echo 'ERR_LOCALDNS_FAIL=99' + grep -E "^trap .*TERM$" "$LOCALDNS_SRC" + grep -E "^trap .*EXIT$" "$LOCALDNS_SRC" + echo 'kill -TERM $$' + # Long enough that the process is unambiguously still here if the signal is + # ignored, so a missing trap shows up as a timeout rather than a pass. + echo 'sleep 10' + echo 'echo "REACHED_AFTER_SIGNAL"' + } > "$harness" + } + cleanup_harness() { + rm -f "$harness" + } + BeforeEach 'build_trap_harness' + AfterEach 'cleanup_harness' + + It 'reaps the watchdog sleep and runs cleanup in-process on SIGTERM' + # This is what changed a systemd stop: before SIGTERM was trapped, bash died on the + # default action and neither of these ran. Both now do, which is what makes a stop + # cost the LOCALDNS_SHUTDOWN_DELAY drain and tear the dummy interface down. + When run command bash "$harness" + The output should include "Received SIGTERM, shutting down." + The output should include "WATCHDOG_REAPED" + The output should include "Executing cleanup function." + The output should include "CLEANUP_RAN" + The status should be success + End + + It 'exits 0 on SIGTERM so Restart=on-failure does not fire for a requested stop' + # A non-zero exit here would make systemd treat every 'systemctl stop' as a failure + # and restart the unit, and would also spend a slot of the StartLimitBurst budget. + When run command bash "$harness" + The status should equal 0 + The output should not include "REACHED_AFTER_SIGNAL" + End + + It 'reports cleanup failure rather than exiting non-zero' + # The EXIT trap deliberately swallows a cleanup failure: a best-effort cleanup error + # must not turn a requested stop into a systemd failure. + sed -i 's/cleanup_localdns_configs() { echo "CLEANUP_RAN"; return 0; }/cleanup_localdns_configs() { echo "CLEANUP_RAN"; return 1; }/' "$harness" + When run command bash "$harness" + The output should include "CLEANUP_RAN" + The output should include "Cleanup failed with error code: 99." + The status should equal 0 + End + End + + # This section tests - start_localdns_watchdog # These functions is also defined in parts/linux/cloud-init/artifacts/localdns.sh file. #------------------------------------------------------------------------------------------------------------------------------------ @@ -1348,6 +1419,73 @@ EOF End End +# This section tests - stop_watchdog_sleep +# This function is defined in parts/linux/cloud-init/artifacts/localdns.sh file. +# +# The watchdog waits by backgrounding 'sleep' so a signal can interrupt the wait. With +# KillMode=mixed systemd signals only the main process, so the child has to be reaped +# explicitly or it holds the unit's cgroup open and delays the stop. These use real +# processes and real signals rather than mocks -- a mocked kill would prove nothing about +# whether the child actually goes away. +#------------------------------------------------------------------------------------------------------------------------------------ + Describe 'stop_watchdog_sleep' + setup() { + Include "./parts/linux/cloud-init/artifacts/localdns.sh" + } + BeforeEach 'setup' + + It 'should kill an in-flight watchdog sleep' + start_and_stop_sleep() { + sleep 300 & + WATCHDOG_SLEEP_PID=$! + # Keep our own copy: stop_watchdog_sleep clears WATCHDOG_SLEEP_PID, so + # checking that variable afterwards would test 'kill -0 ""' and pass even + # if the child were still alive. + child_pid=$WATCHDOG_SLEEP_PID + # the child must genuinely be running before we try to stop it + kill -0 "$child_pid" 2>/dev/null || { echo "child never started"; return 1; } + stop_watchdog_sleep + # and genuinely gone afterwards + if kill -0 "$child_pid" 2>/dev/null; then + echo "child $child_pid survived stop_watchdog_sleep" + kill -9 "$child_pid" 2>/dev/null + return 1 + fi + echo "child reaped" + return 0 + } + When call start_and_stop_sleep + The status should be success + The output should include "child reaped" + End + + It 'should clear the recorded pid so a second call is a no-op' + stop_twice() { + sleep 300 & + WATCHDOG_SLEEP_PID=$! + stop_watchdog_sleep + [ -z "${WATCHDOG_SLEEP_PID}" ] || { echo "pid not cleared"; return 1; } + # calling again with nothing in flight must not error + stop_watchdog_sleep + echo "second call was a no-op" + } + When call stop_twice + The status should be success + The output should include "second call was a no-op" + End + + It 'should do nothing when no sleep is in flight' + no_sleep() { + WATCHDOG_SLEEP_PID="" + stop_watchdog_sleep + echo "no-op ok" + } + When call no_sleep + The status should be success + The output should include "no-op ok" + End + End + # This section tests - wait_for_localdns_removed_from_resolv_conf # This function is defined in parts/linux/cloud-init/artifacts/localdns.sh file. diff --git a/vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml b/vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml index 9dc642c13f6..ce1ec16bc7d 100644 --- a/vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml +++ b/vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml @@ -337,6 +337,8 @@ os: permissions: 755 - source: /AgentBaker/parts/linux/cloud-init/artifacts/localdns-delegate.conf destination: /etc/systemd/system/localdns.service.d/delegate.conf + - source: /AgentBaker/parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf + destination: /etc/systemd/system/localdns.service.d/99-timeout-terminate.conf permissions: 644 # localdns exporter - source: /AgentBaker/parts/linux/cloud-init/artifacts/localdns_exporter.sh diff --git a/vhdbuilder/packer/packer_source.sh b/vhdbuilder/packer/packer_source.sh index b42157a2a13..8f1a54a11c5 100644 --- a/vhdbuilder/packer/packer_source.sh +++ b/vhdbuilder/packer/packer_source.sh @@ -432,6 +432,13 @@ copyPackerFiles() { LOCALDNS_SERVICE_DELEGATE_DEST=/etc/systemd/system/localdns.service.d/delegate.conf cpAndMode $LOCALDNS_SERVICE_DELEGATE_SRC $LOCALDNS_SERVICE_DELEGATE_DEST 0644 + # Separate drop-in from delegate.conf on purpose: this one pins + # TimeoutStopFailureMode, which cannot be set in localdns.service because Azure Linux's + # type-wide service.d drop-in overrides unit files. See the comment in the artifact. + LOCALDNS_SERVICE_TIMEOUT_SRC=/home/packer/localdns-timeout-terminate.conf + LOCALDNS_SERVICE_TIMEOUT_DEST=/etc/systemd/system/localdns.service.d/99-timeout-terminate.conf + cpAndMode $LOCALDNS_SERVICE_TIMEOUT_SRC $LOCALDNS_SERVICE_TIMEOUT_DEST 0644 + # Skip localdns exporter for Flatcar (EOL June 2026, no new features) if ! isFlatcar "$OS"; then LOCALDNS_EXPORTER_SCRIPT_SRC=/home/packer/localdns_exporter.sh diff --git a/vhdbuilder/packer/test/linux-vhd-content-test.sh b/vhdbuilder/packer/test/linux-vhd-content-test.sh index 6a9b6c185cc..8470d2057b5 100644 --- a/vhdbuilder/packer/test/linux-vhd-content-test.sh +++ b/vhdbuilder/packer/test/linux-vhd-content-test.sh @@ -2508,6 +2508,7 @@ checkLocaldnsScriptsAndConfigs() { ["/opt/azure/containers/localdns/localdns.sh"]=755 ["/etc/systemd/system/localdns.service"]=644 ["/etc/systemd/system/localdns.service.d/delegate.conf"]=644 + ["/etc/systemd/system/localdns.service.d/99-timeout-terminate.conf"]=644 ) # Flatcar is EOL (June 2026) — exporter files are not installed on Flatcar VHDs diff --git a/vhdbuilder/packer/vhd-image-builder-acl-arm64.json b/vhdbuilder/packer/vhd-image-builder-acl-arm64.json index c76eaccb31b..c3b4a8ee778 100644 --- a/vhdbuilder/packer/vhd-image-builder-acl-arm64.json +++ b/vhdbuilder/packer/vhd-image-builder-acl-arm64.json @@ -677,6 +677,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-acl.json b/vhdbuilder/packer/vhd-image-builder-acl.json index af078cdc240..bce4a69bee3 100644 --- a/vhdbuilder/packer/vhd-image-builder-acl.json +++ b/vhdbuilder/packer/vhd-image-builder-acl.json @@ -677,6 +677,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-arm64-gb.json b/vhdbuilder/packer/vhd-image-builder-arm64-gb.json index c4e0de08ff0..bc228c2b9a7 100644 --- a/vhdbuilder/packer/vhd-image-builder-arm64-gb.json +++ b/vhdbuilder/packer/vhd-image-builder-arm64-gb.json @@ -762,6 +762,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-arm64-gen2.json b/vhdbuilder/packer/vhd-image-builder-arm64-gen2.json index 380448676f0..b261740b782 100644 --- a/vhdbuilder/packer/vhd-image-builder-arm64-gen2.json +++ b/vhdbuilder/packer/vhd-image-builder-arm64-gen2.json @@ -747,6 +747,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-base.json b/vhdbuilder/packer/vhd-image-builder-base.json index 3322381f0fa..5757b553890 100644 --- a/vhdbuilder/packer/vhd-image-builder-base.json +++ b/vhdbuilder/packer/vhd-image-builder-base.json @@ -755,6 +755,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-cvm.json b/vhdbuilder/packer/vhd-image-builder-cvm.json index 0db9f2792d8..f08c265c71c 100644 --- a/vhdbuilder/packer/vhd-image-builder-cvm.json +++ b/vhdbuilder/packer/vhd-image-builder-cvm.json @@ -759,6 +759,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-mariner-arm64.json b/vhdbuilder/packer/vhd-image-builder-mariner-arm64.json index 32ebfbebf25..220615a9616 100644 --- a/vhdbuilder/packer/vhd-image-builder-mariner-arm64.json +++ b/vhdbuilder/packer/vhd-image-builder-mariner-arm64.json @@ -721,6 +721,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-mariner-cvm.json b/vhdbuilder/packer/vhd-image-builder-mariner-cvm.json index c7401109b98..6534f1c85c3 100644 --- a/vhdbuilder/packer/vhd-image-builder-mariner-cvm.json +++ b/vhdbuilder/packer/vhd-image-builder-mariner-cvm.json @@ -722,6 +722,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh", diff --git a/vhdbuilder/packer/vhd-image-builder-mariner.json b/vhdbuilder/packer/vhd-image-builder-mariner.json index f243109dc54..6e3e4c9f4e4 100644 --- a/vhdbuilder/packer/vhd-image-builder-mariner.json +++ b/vhdbuilder/packer/vhd-image-builder-mariner.json @@ -723,6 +723,11 @@ "source": "parts/linux/cloud-init/artifacts/localdns-delegate.conf", "destination": "/home/packer/localdns-delegate.conf" }, + { + "type": "file", + "source": "parts/linux/cloud-init/artifacts/localdns-timeout-terminate.conf", + "destination": "/home/packer/localdns-timeout-terminate.conf" + }, { "type": "file", "source": "parts/linux/cloud-init/artifacts/localdns_exporter.sh",