feat: support localdns hosts plugin refresh override#8895
Open
saewoni wants to merge 1 commit into
Open
Conversation
saewoni
requested review from
AbelHu,
Devinwong,
SriHarsha001,
awesomenix,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
karenychen,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
runzhen,
sulixu,
surajssd,
timmy-wright,
titilambert,
xuexu6666 and
zachary-bailey
as code owners
Contributor
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for configuring the AKS LocalDNS hosts plugin refresh interval end-to-end (datamodel → template/provisioning env → CSE behavior), replacing reliance on the fixed 15-minute systemd timer cadence when an explicit positive interval is provided.
Changes:
- Extend the AgentBaker datamodel and template func map to carry
hostsPluginRefreshIntervalInSecondsinto provisioning. - Extend aks-node-controller proto + parser to emit
LOCALDNS_HOSTS_PLUGIN_REFRESH_INTERVAL_IN_SECONDSfor scriptless provisioning. - Update Linux CSE (
cse_config.sh) to rewriteaks-localdns-hosts-setup.timerOnUnitActiveSecwhen a valid positive interval is provided, with new/updated tests.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/parts/linux/cloud-init/artifacts/cse_config_spec.sh | Adds ShellSpec coverage for timer interval override behavior. |
| pkg/agent/datamodel/types.go | Adds HostsPluginRefreshIntervalInSeconds to LocalDNSProfile JSON schema. |
| pkg/agent/baker.go | Adds template function to render refresh interval into CSE templates. |
| pkg/agent/baker_test.go | Unit tests for the new template function behavior (nil/invalid/valid). |
| parts/linux/cloud-init/artifacts/cse_config.sh | Applies the provided interval by editing the systemd timer before enabling it. |
| parts/linux/cloud-init/artifacts/cse_cmd.sh | Emits the new refresh interval env var for legacy CSE flow. |
| aks-node-controller/proto/aksnodeconfig/v1/localdns_config.proto | Adds optional refresh interval field to LocalDNS proto. |
| aks-node-controller/pkg/gen/aksnodeconfig/v1/localdns_config.pb.go | Regenerates protobuf bindings for the new field. |
| aks-node-controller/parser/parser.go | Exposes the new interval as a CSE env var in scriptless mode. |
| aks-node-controller/parser/parser_test.go | Validates env var emission in integration-style parser tests. |
| aks-node-controller/parser/helper.go | Adds helper to stringify/validate refresh interval from proto config. |
| aks-node-controller/parser/helper_test.go | Unit tests for the new helper’s nil/invalid/valid cases. |
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/localdns_config.pb.go: Generated file
saewoni
force-pushed
the
sakwa/localdns-hosts-plugin-refresh-interval
branch
from
38e6e82 to
d03db7d
Compare
awesomenix
reviewed
Jul 12, 2026
| ;; | ||
| *) | ||
| if [ "${hosts_plugin_refresh_interval}" -gt 0 ]; then | ||
| sed -i "s/^OnUnitActiveSec=.*/OnUnitActiveSec=${hosts_plugin_refresh_interval}s/" "${hosts_setup_timer}" |
Contributor
There was a problem hiding this comment.
Seems very hacky. I would think that there would be some form of timer.template and then you render it either on agentbaker side or aks-node-controller side?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This change lets AgentBaker carry a LocalDNS hosts plugin refresh interval from node bootstrapping config down to provisioning, instead of relying on the hardcoded 15 minute timer cadence.
It adds the new LocalDNS field to the datamodel and aks-node-controller proto/parser paths, emits the value into both legacy CSE and scriptless env, and updates
cse_config.shto rewriteaks-localdns-hosts-setup.timerwhen a positive interval is provided. When the field is unset or invalid, provisioning keeps the existing default behavior.This complements the corresponding AKS-RP change in PR 16401188.
Validation run:
make validate-shelldocker run --rm -v /home/sakwa/AgentBaker:/workspace -w /workspace shellspec-docker --shell bash --format d spec/parts/linux/cloud-init/artifacts/cse_config_spec.shgo test ./pkg/agent/...cd aks-node-controller && go test ./parser./hack/tools/bin/golangci-lint run ./pkg/agent/...cd aks-node-controller && ../hack/tools/bin/golangci-lint run ./parser/...Which issue(s) this PR fixes:
Fixes #