Add LocalDNS CoreDNS reload plugin - #9018
Conversation
Add the top-level CoreDNS reload plugin to generated LocalDNS Corefiles so CoreDNS can pick up LPS-managed Corefile updates without process restarts. Update both AgentBaker Corefile generators and their expectations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates LocalDNS Corefile generation to include the CoreDNS reload plugin in each generated server block, ensuring both AgentBaker’s generator and aks-node-controller’s parser produce consistent Corefiles and that unit test expectations match.
Changes:
- Add
reloadimmediately afterreadyin the LocalDNS Corefile templates for both VnetDNS and KubeDNS override blocks. - Update unit test expected Corefile strings in both generation paths to include the new plugin line.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/agent/baker.go | Adds reload to the LocalDNS Corefile Go template for both VnetDNS and KubeDNS override server blocks. |
| pkg/agent/baker_test.go | Updates expected generated LocalDNS Corefile strings to include reload. |
| aks-node-controller/parser/templates/localdns.toml.gtpl | Adds reload to the LocalDNS Corefile template used by aks-node-controller parsing/generation. |
| aks-node-controller/parser/helper_test.go | Updates expected Corefile strings to include reload for parser-related tests. |
Fetch the opaque LocalDNS live-patching payload from LPS before CoreDNS starts, render the Corefile from the per-agent-pool localDnsProfile, and report the applied corefileVersion after kubeconfig is available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| if "${AKS_NODE_CONTROLLER_BINARY}" fetch-localdns-config --output "${LOCALDNS_CORE_FILE}"; then | ||
| echo "Completed LocalDNS LPS config fetch." | ||
| return 0 | ||
| fi | ||
|
|
||
| echo "LocalDNS LPS config fetch failed; continuing with existing corefile." | ||
| return 0 |
| if err != nil { | ||
| if code := status.Code(err); code == codes.NotFound || code == codes.PermissionDenied || code == codes.Unauthenticated { | ||
| return "", &lpsUnavailableError{statusCode: int(code)} | ||
| } |
| # Fetch LocalDNS config from LPS if present. This is fail-open: no config or fetch errors keep the | ||
| # locally generated corefile. If LPS returns a usable profile/corefile, LOCALDNS_CORE_FILE is updated | ||
| # before VNET DNS replacement builds UPDATED_LOCALDNS_CORE_FILE for CoreDNS. | ||
| # --------------------------------------------------------------------------------------------------------------------- | ||
| refresh_localdns_corefile_from_lps |
Add an E2E scenario that simulates the LocalDNS LPS fetch path, restarts localdns, and verifies the patched Corefile, version persistence, and live-patching status stamp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/localdns.sh:1149
- The PR description focuses on adding the CoreDNS reload plugin, but this change also adds a new LocalDNS live-patching bootstrap flow (calling aks-node-controller fetch-localdns-config) and a node annotation path for live-patching status. This extra behavior is substantial and should be reflected in the PR description so reviewers/operators understand the added runtime dependency and on-node behavior.
# Fetch LocalDNS config from LPS if present. This is fail-open: no config or fetch errors keep the
# locally generated corefile. If LPS returns a usable profile/corefile, LOCALDNS_CORE_FILE is updated
# before VNET DNS replacement builds UPDATED_LOCALDNS_CORE_FILE for CoreDNS.
# ---------------------------------------------------------------------------------------------------------------------
refresh_localdns_corefile_from_lps
| local current_status | ||
| current_status=$(/opt/bin/kubectl --kubeconfig "${kubeconfig}" get node "${node_name}" -o "jsonpath={.metadata.annotations['kubernetes\.azure\.com/live-patching-status']}" 2>/dev/null || true) | ||
| if [ -z "${current_status}" ]; then |
| // Corefile is created using localdns.toml.gtpl template and aksnodeconfig values. | ||
| // includeHostsPlugin controls whether the hosts plugin block is included in the generated Corefile. | ||
| func generateLocalDnsCorefileFromAKSNodeConfig(aksnodeconfig *aksnodeconfigv1.Configuration, includeHostsPlugin bool) (string, error) { | ||
| func GenerateLocalDNSCorefileFromAKSNodeConfig(aksnodeconfig *aksnodeconfigv1.Configuration, includeHostsPlugin bool) (string, error) { |
Use grpc.NewClient, reduce payload parsing complexity, avoid shadowing, tighten version-file permissions, and simplify LocalDNS test setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
parts/linux/cloud-init/artifacts/localdns.sh:328
refresh_localdns_corefile_from_lpstreats the aks-node-controller exit code as a success/failure signal, butfetch-localdns-configis explicitly fail-open (it returns nil from the CLI action even on errors). This makes theif ...; then ... else ...misleading (the failure branch is effectively unreachable today) and also couples correctness to the command never returning non-zero; if it ever does, removing theiflater could causeset -eto abort the service. Prefer running the command fail-open and unconditionally continuing.
if "${AKS_NODE_CONTROLLER_BINARY}" fetch-localdns-config --output "${LOCALDNS_CORE_FILE}"; then
echo "Completed LocalDNS LPS config fetch."
return 0
fi
echo "LocalDNS LPS config fetch failed; continuing with existing corefile."
return 0
parts/linux/cloud-init/artifacts/localdns.sh:1149
- The PR title/summary focuses on adding the CoreDNS
reloadplugin to generated LocalDNS Corefiles, but this diff also introduces a new LocalDNS live-patching bootstrap path (callingaks-node-controller fetch-localdns-configfromlocaldns.shplus annotatingkubernetes.azure.com/live-patching-status). That’s a significant functional addition that should be reflected in the PR description/title so reviewers and release notes capture the behavioral change.
# Fetch LocalDNS config from LPS if present. This is fail-open: no config or fetch errors keep the
# locally generated corefile. If LPS returns a usable profile/corefile, LOCALDNS_CORE_FILE is updated
# before VNET DNS replacement builds UPDATED_LOCALDNS_CORE_FILE for CoreDNS.
# ---------------------------------------------------------------------------------------------------------------------
refresh_localdns_corefile_from_lps
| status := node.Annotations["kubernetes.azure.com/live-patching-status"] | ||
| return strings.Contains(status, `"localDNS":{"current":"`+desiredVersion+`"}`), nil | ||
| }) | ||
| require.NoError(s.T, err, "node did not report LocalDNS live-patching current version %q", desiredVersion) |
Summary\n- add the top-level CoreDNS reload plugin to generated LocalDNS Corefiles\n- update both AgentBaker Corefile generation paths: pkg/agent and aks-node-controller/parser\n- update LocalDNS Corefile expectations to include reload\n\n## Testing\n- (cd aks-node-controller && go test ./parser -count=1)\n- go test ./pkg/agent -ginkgo.focus "GetGeneratedLocalDNSCoreFile" -count=1