From c3c511b8cb59b36e38da87994ac521bcdd46fba8 Mon Sep 17 00:00:00 2001 From: Saewon Kwak <23280628+saewoni@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:30:53 -0700 Subject: [PATCH 1/3] fix: reject empty resolver state during LocalDNS startup --- parts/linux/cloud-init/artifacts/localdns.sh | 4 ++-- .../cloud-init/artifacts/localdns_spec.sh | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/parts/linux/cloud-init/artifacts/localdns.sh b/parts/linux/cloud-init/artifacts/localdns.sh index 6f07c1ee455..1da583e476a 100644 --- a/parts/linux/cloud-init/artifacts/localdns.sh +++ b/parts/linux/cloud-init/artifacts/localdns.sh @@ -613,7 +613,7 @@ wait_for_localdns_removed_from_resolv_conf() { current_dns=$(awk '/^nameserver/ {print $2}' "$RESOLV_CONF" 2>/dev/null | paste -sd' ') # Use word boundary matching (-w) with fixed string (-F) to avoid partial IP matches. - if ! grep -qwF "$LOCALDNS_NODE_LISTENER_IP" <<< "$current_dns"; then + if [ -n "$current_dns" ] && ! grep -qwF "$LOCALDNS_NODE_LISTENER_IP" <<< "$current_dns"; then echo "DNS configuration refreshed successfully. Current DNS: ${current_dns}" return 0 fi @@ -658,7 +658,7 @@ EOF cleanup_iptables_and_dns() { # Ensure network variables are initialized if not already set. # This is needed here because this function can be called from cleanup traps or systemd restarts initiated by watchdog. - if [ -z "${NETWORK_DROPIN_FILE:-}" ] || [ -z "${NETWORK_DROPIN_DIR:-}" ]; then + if [ -z "${DEFAULT_ROUTE_INTERFACE:-}" ] || [ -z "${NETWORK_DROPIN_FILE:-}" ] || [ -z "${NETWORK_DROPIN_DIR:-}" ]; then echo "Network variables not initialized, attempting to determine them..." if ! initialize_network_variables; then echo "Failed to initialize network variables during cleanup." diff --git a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh index 65fbaae2cb9..f21c8613ae5 100644 --- a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh +++ b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh @@ -1338,11 +1338,18 @@ EOF The stdout should include "Current DNS:" End - It 'should return success if resolv.conf is empty' + It 'should keep waiting if resolv.conf has no nameservers' > "$RESOLV_CONF" - When run wait_for_localdns_removed_from_resolv_conf 5 - The status should be success - The stdout should include "DNS configuration refreshed successfully" + When run wait_for_localdns_removed_from_resolv_conf 1 + The status should be failure + The stdout should include "Timed out waiting for localdns to be removed" + End + + It 'should keep waiting if resolv.conf contains only comments' + printf '# nameserver 10.0.0.1\n' > "$RESOLV_CONF" + When run wait_for_localdns_removed_from_resolv_conf 1 + The status should be failure + The stdout should include "Timed out waiting for localdns to be removed" End It 'should use default timeout of 5 seconds when not specified' @@ -1354,11 +1361,11 @@ EOF The stdout should include "DNS configuration refreshed successfully" End - It 'should handle resolv.conf not existing gracefully' + It 'should fail when resolv.conf does not exist' rm -f "$RESOLV_CONF" - When run wait_for_localdns_removed_from_resolv_conf 2 - The status should be success - The stdout should include "DNS configuration refreshed successfully" + When run wait_for_localdns_removed_from_resolv_conf 1 + The status should be failure + The stdout should include "Timed out waiting for localdns to be removed" End It 'should not match partial IP addresses' From e59c32ec121aba31d36934f17a5f8a22a5143a0b Mon Sep 17 00:00:00 2001 From: Saewon Kwak <23280628+saewoni@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:15:43 -0700 Subject: [PATCH 2/3] test: add async recovery case for empty resolver during LocalDNS startup wait Asserts the convergence path (empty -> upstream appears mid-wait -> success), not just rejection. Addresses review feedback that the existing empty/comment/ missing tests only prove we stopped false-succeeding, not that we wait out the transient empty window and recover once the upstream nameserver is repopulated. --- .../linux/cloud-init/artifacts/localdns_spec.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh index f21c8613ae5..fad8a6a1ebf 100644 --- a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh +++ b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh @@ -1345,6 +1345,18 @@ EOF The stdout should include "Timed out waiting for localdns to be removed" End + It 'should succeed when nameservers appear during wait (async recovery)' + # Start empty, then have the upstream nameserver appear mid-wait. + # This is the point of the fix: an empty resolver must keep waiting + # until the upstream is repopulated, then succeed - not false-succeed + # on the empty window. + > "$RESOLV_CONF" + (sleep 1 && echo "nameserver 10.0.0.1" > "$RESOLV_CONF") & + When run wait_for_localdns_removed_from_resolv_conf 5 + The status should be success + The stdout should include "Current DNS: 10.0.0.1" + End + It 'should keep waiting if resolv.conf contains only comments' printf '# nameserver 10.0.0.1\n' > "$RESOLV_CONF" When run wait_for_localdns_removed_from_resolv_conf 1 From 6aaf8bdac9f8d75a3da2c1b377fc447153ce371c Mon Sep 17 00:00:00 2001 From: Saewon Kwak <23280628+saewoni@users.noreply.github.com> Date: Wed, 9 Sep 2026 18:31:27 -0700 Subject: [PATCH 3/3] test: cover cleanup init when DEFAULT_ROUTE_INTERFACE is unset Adds a ShellSpec case for the guard change in cleanup_iptables_and_dns that now also checks DEFAULT_ROUTE_INTERFACE. Verifies that when the interface is unset (as on a trap/watchdog-initiated cleanup), initialize_network_variables is invoked to re-derive it and the drop-in is still removed successfully. Addresses review feedback that the new branch lacked ShellSpec coverage. --- .../cloud-init/artifacts/localdns_spec.sh | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh index fad8a6a1ebf..e6b95abe9b1 100644 --- a/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh +++ b/spec/parts/linux/cloud-init/artifacts/localdns_spec.sh @@ -986,6 +986,38 @@ EOF The status should be success The stdout should include "No existing localdns iptables rules found." End + + It 'should initialize network variables when DEFAULT_ROUTE_INTERFACE is unset and still remove the drop-in' + # Regression cover for the guard that now also checks DEFAULT_ROUTE_INTERFACE: + # cleanup can be invoked from a trap/watchdog restart with the interface unset, + # so it must call initialize_network_variables (re-deriving the interface via the + # mocked ip/networkctl) and still complete cleanup successfully. + iptables() { mock_iptables "$@"; } + AZURE_DNS_IP="168.63.129.16" + NETWORKCTL_RELOAD_CMD="true" + # A real network file must exist for verify_network_file during initialization. + NETWORK_FILE="/tmp/test-eth0.network" + touch "$NETWORK_FILE" + networkctl() { + if [[ "$1" == "--json=short" && "$2" == "status" && "$3" == "eth0" ]]; then + echo "{\"NetworkFile\":\"${NETWORK_FILE}\"}" + elif [[ "$1" == "reload" ]]; then + return 0 + else + command networkctl "$@" + fi + } + touch "$NETWORK_DROPIN_FILE" + # Force the new initialization branch: interface not yet known. + unset DEFAULT_ROUTE_INTERFACE + When call cleanup_iptables_and_dns + The status should be success + The stdout should include "Network variables not initialized, attempting to determine them..." + The stdout should include "Removing network drop-in file" + The variable DEFAULT_ROUTE_INTERFACE should equal "eth0" + The file "${NETWORK_DROPIN_FILE}" should not be exist + rm -f "$NETWORK_FILE" + End End