agent: add nspawn LocalDNS - #554
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional “LocalDNS” feature for the agent: a two-listener CoreDNS cache running inside the active systemd-nspawn machine, with wiring to route machine/default DNS and ClusterFirst pod DNS through separate link-local listeners. It extends artifact handling (online/offline), preflight checks, lifecycle (reboot/repave/reset), API/CRD surfaces, and an e2e Kind/QEMU scenario to validate the end-to-end behavior.
Changes:
- Add LocalDNS goal state + config validation, wire kubelet
--cluster-dns/--resolv-conf, and integrate rootfs + nodestart phases. - Add CoreDNS artifact sourcing (online + offline bundles), checksum/plugin validation, and preflight reachability checks.
- Add host network reconciliation + reset cleanup for NOTRACK rules/interface, plus tests and e2e coverage.
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/phases/rootfs/resolved.go | Rewrite machine resolv.conf for LocalDNS |
| pkg/agent/phases/rootfs/provision.go | Add rootfs LocalDNS task |
| pkg/agent/phases/rootfs/preflight_remote.go | Add CoreDNS artifact reachability check |
| pkg/agent/phases/rootfs/preflight_goal_state.go | Conditionally append LocalDNS preflight |
| pkg/agent/phases/rootfs/localdns.go | Install CoreDNS + units/config into rootfs |
| pkg/agent/phases/rootfs/localdns_test.go | Rootfs LocalDNS unit/config tests |
| pkg/agent/phases/rootfs/assets/localdns.slice | Rootfs systemd slice template |
| pkg/agent/phases/rootfs/assets/localdns.service | Rootfs LocalDNS systemd service |
| pkg/agent/phases/rootfs/assets/localdns-supervisor.sh | CoreDNS supervisor/watchdog script |
| pkg/agent/phases/reset/tasks_test.go | Update reset task naming assertion |
| pkg/agent/phases/reset/network.go | Add LocalDNS host cleanup in reset |
| pkg/agent/phases/nodestart/start.go | Integrate LocalDNS into start ordering |
| pkg/agent/phases/nodestart/localdns.go | Refresh inputs + host network + readiness wait |
| pkg/agent/phases/nodestart/localdns_test.go | Unit tests for readiness helper |
| pkg/agent/phases/nodestart/kubelet.go | Pass ResolvConf into kubelet unit template |
| pkg/agent/phases/nodestart/assets/unbounded-localdns-network.sh | Host dummy iface + nft notrack reconciliation |
| pkg/agent/phases/nodestart/assets/unbounded-localdns-network.service | Host oneshot unit for LocalDNS network prep |
| pkg/agent/phases/nodestart/assets/localdns.slice | Nodestart slice template for runtime updates |
| pkg/agent/phases/nodestart/assets/kubelet.service | Template kubelet --resolv-conf |
| pkg/agent/phases/host/preflight_localdns.go | Host preflight for nft notrack capability |
| pkg/agent/phases/host/preflight_localdns_test.go | Tests for host LocalDNS preflight |
| pkg/agent/phases/host/preflight_host.go | Add LocalDNS conntrack preflight |
| pkg/agent/phases/host/os.go | Document nftables required for LocalDNS |
| pkg/agent/goalstates/rootfs.go | Add LocalDNS to RootFS goal state |
| pkg/agent/goalstates/resolve.go | Resolve LocalDNS + adjust kubelet DNS inputs |
| pkg/agent/goalstates/offline_artifacts.go | Require CoreDNS version when LocalDNS enabled |
| pkg/agent/goalstates/offline_artifacts_test.go | Tests for CoreDNS offline requirements/overrides |
| pkg/agent/goalstates/nodestart.go | Add LocalDNS to NodeStart goal state |
| pkg/agent/goalstates/localdns.go | LocalDNS resolution, upstream discovery, Corefile render |
| pkg/agent/goalstates/localdns_test.go | LocalDNS goal state unit tests |
| pkg/agent/goalstates/kubelet.go | Add Kubelet.ResolvConf field |
| pkg/agent/goalstates/downloads.go | Add Downloads.CoreDNS override |
| pkg/agent/goalstates/constants.go | Add LocalDNS resolv.conf path const |
| pkg/agent/goalstates/assets/default-localdns.Corefile.tmpl | Default two-listener Corefile template |
| pkg/agent/config/localdns.go | Agent config schema + validation for LocalDNS |
| pkg/agent/config/localdns_test.go | Validation + DeepCopy tests for LocalDNS config |
| pkg/agent/config/config.go | Add LocalDNS to AgentConfig + deepcopy/validate |
| pkg/agent/bootstrapartifacts/paths.go | Add offline path helpers for CoreDNS |
| pkg/agent/bootstrapartifacts/manifest.go | Add versions.coredns to manifest normalization |
| pkg/agent/artifactsource/source.go | Add option to extract a file from tar.gz sources |
| pkg/agent/artifactsource/source_test.go | Tests for tar.gz extraction + retry behavior |
| internal/provision/agent_config.go | Plumb LocalDNS + CoreDNS downloads from API spec |
| internal/agentartifacts/artifacts.go | Add CoreDNS artifact URL resolution |
| internal/agentartifacts/artifacts_test.go | Tests for CoreDNSArchive behavior |
| hack/cmd/agent-artifacts-builder/artifacts/artifacts.go | Include/extract CoreDNS in artifact bundles |
| hack/cmd/agent-artifacts-builder/artifacts/artifacts_test.go | Tests for CoreDNS artifact plan entries |
| hack/agent/e2e-kind/node-configs/README.md | Document localDNS scenario knob |
| hack/agent/e2e-kind/node-configs/localdns.json | Add LocalDNS e2e node config |
| hack/agent/e2e-kind/e2e.py | Wire LocalDNS scenario + validations + cleanup |
| docs/content/reference/agent/configuration.md | Document LocalDNS config fields/constraints |
| designs/agent-nspawn-localdns.md | Update design notes re nftables requirement |
| deploy/machina/crd/unbounded-cloud.io_machines.yaml | CRD fields for LocalDNS + CoreDNS downloads |
| deploy/machina/crd/unbounded-cloud.io_machineconfigurationversions.yaml | CRD fields for versioned LocalDNS |
| deploy/machina/crd/unbounded-cloud.io_machineconfigurations.yaml | CRD fields for versioned LocalDNS |
| cmd/kubectl-unbounded/app/machine_manual_bootstrap.go | Add --local-dns bootstrap flag |
| cmd/agent/internal/daemon/nodeoperator.go | Treat LocalDNS changes as drift |
| cmd/agent/internal/daemon/controller_node.go | Apply LocalDNS from MachineConfiguration templates |
| api/machina/v1alpha3/zz_generated.deepcopy.go | DeepCopy support for LocalDNS/CoreDNS fields |
| api/machina/v1alpha3/machineconfiguration_types.go | Add LocalDNS to MachineConfigurationAgent |
| api/machina/v1alpha3/machine_types.go | Add LocalDNS + CoreDNS download fields to API |
Files not reviewed (1)
- api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 60 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file
Suppressed comments (3)
pkg/agent/phases/rootfs/assets/localdns.slice:5
- CPUQuota in the rootfs localdns.slice template is missing the required % suffix. The code renders CPUQuota as a numeric percent (millicores/10), so systemd will reject the unit or treat the value incorrectly, preventing LocalDNS from starting during provisioning.
pkg/agent/phases/rootfs/resolved.go:67 - The nameserver filtering for LocalDNS only matches lines starting with "nameserver " (space). /etc/resolv.conf commonly uses tabs (e.g. "nameserver\t127.0.0.53"), which would not be removed and could leave extra nameservers ahead of the LocalDNS listener.
pkg/agent/phases/reset/network.go:81 - Dummy-interface detection during reset is brittle:
ip -d -o link showoutput does not reliably contain " dummy " as a space-delimited token, so this can incorrectly refuse to delete the LocalDNS dummy interface and fail reset.
852a445 to
9918cf0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 60 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file
Suppressed comments (5)
pkg/agent/phases/nodestart/localdns.go:1
- This task writes to
/usr/local/libexec/...and to a systemd drop-in directory path, but it doesn’t ensure the parent directories exist. Other parts of this PR explicitly create directories before writing files (e.g., rootfs LocalDNS setup), which suggestsutilio.WriteFilemay not create parents. Create the parent dirs (e.g.,os.MkdirAll(filepath.Dir(path), 0o755)) before these writes to avoid failing on hosts where/usr/local/libexecor the drop-in directory doesn’t exist.
pkg/agent/phases/nodestart/localdns.go:1 - This task writes to
/usr/local/libexec/...and to a systemd drop-in directory path, but it doesn’t ensure the parent directories exist. Other parts of this PR explicitly create directories before writing files (e.g., rootfs LocalDNS setup), which suggestsutilio.WriteFilemay not create parents. Create the parent dirs (e.g.,os.MkdirAll(filepath.Dir(path), 0o755)) before these writes to avoid failing on hosts where/usr/local/libexecor the drop-in directory doesn’t exist.
pkg/agent/phases/nodestart/localdns.go:1 - This duplicates the nameserver-stripping logic that also exists as
localDNSResolvConf()inpkg/agent/phases/rootfs/localdns.go. Duplicating this parsing makes it easy for the two code paths to drift (e.g., handling of whitespace, empty lines, ordering). Consider extracting a shared helper (e.g., intogoalstatesor a small internal util package) and reusing it from both rootfs and nodestart.
pkg/agent/phases/rootfs/localdns.go:1 template.Must(...)will panic if the embedded template is invalid, which would crash provisioning rather than returning a controlled error. Even though the template is embedded, it’s safer to parse the template withoutMustand return a normal error (or parse once at init and store a compiled template).
pkg/agent/artifactsource/source.go:262- Changing
Source.DownloadToLocalFilefrom a fixed signature to a variadic options signature is a breaking change for any callers outside this repo/package set (the method is exported). If this package is intended to be a stable API surface, consider preserving the originalDownloadToLocalFile(ctx, filename, perm)method and introducing a new method (e.g.,DownloadToLocalFileWithOptions) for the option-based behavior.
func (s Source) DownloadToLocalFile(ctx context.Context, filename string, perm os.FileMode, optionFuncs ...DownloadToLocalFileOption) error {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 60 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file
Suppressed comments (4)
pkg/agent/phases/reset/network.go:1
- Reset cleanup removes the LocalDNS host network unit and script, but it doesn't remove the systemd-nspawn drop-in created by
SetupLocalDNSNetwork(/etc/systemd/system/systemd-nspawn@<machine>.service.d/10-localdns.conf). Leaving that drop-in behind can keep stale ordering/Requires relationships across subsequent deployments afterreset-agent. Add deletion of the LocalDNS nspawn drop-in(s) as part of reset cleanup (e.g., removesystemd-nspawn@*.service.d/10-localdns.confor remove the specific directory entry if the machine name is known at reset time).
pkg/agent/phases/rootfs/localdns.go:1 template.Must(...Parse(...))will panic on parse errors, which is generally undesirable in agent code paths (even if the template is embedded). Prefer parsing withoutMustand returning a normal error, or parse once at init/package scope and reuse a pre-parsed template so runtime execution cannot panic.
pkg/agent/phases/rootfs/localdns.go:1- Archive detection is currently based on
strings.Contains(source.String(), \".tgz\"). If a user override points to a.tar.gzCoreDNS release (or another valid tarball extension), the code will treat it as a raw binary download and later fail when executing-plugins. Consider detecting tar.gz via parsed path extension(s) (e.g.,.tgzOR.tar.gz) rather than a substring check, and/or drive the behavior from the actual content type/known artifact format.
pkg/agent/phases/nodestart/localdns.go:1 - This resolver-rewrite logic duplicates
localDNSResolvConf(...)inpkg/agent/phases/rootfs/localdns.go(same behavior: strip allnameserverlines and append the listener). To reduce drift risk between provisioning and managed-start behavior, consider moving the helper to a shared package (e.g., goalstates/util) or otherwise reusing a single implementation.
| machine.Spec.Agent = &unboundedv1alpha3.AgentSpec{Image: h.ociImage} | ||
| if h.localDNS { | ||
| machine.Spec.Agent.LocalDNS = &unboundedv1alpha3.LocalDNSSpec{Enabled: true} | ||
| } | ||
|
|
||
| if downloads := h.buildDownloadsSpec(); downloads != nil { | ||
| if machine.Spec.Agent == nil { | ||
| machine.Spec.Agent = &unboundedv1alpha3.AgentSpec{} | ||
| } | ||
|
|
||
| machine.Spec.Agent.Downloads = downloads | ||
| } |
Summary
PR structure
This is stacked on draft design PR #552. After #552 merges, this PR can be retargeted to
main.Validation
make buildgo test ./pkg/agent/... ./internal/provision ./internal/agentartifacts ./hack/cmd/agent-artifacts-builder/... ./cmd/agent/internal/cmdgolangci-lintfor all modified Go packages: 0 issuesgo test ./cmd/kubectl-unbounded/app -run ManualBootstrap -count=1python3 -m py_compile hack/agent/e2e-kind/e2e.pybash -nfor LocalDNS supervisor and host-network scriptsgit diff --checkThe
agent-e2e-kindworkflow discoversnode-configs/localdns.jsonand validates bootstrap, DNS wiring, metrics, NOTRACK rules, managed node reboot, repave, and reset cleanup in PR CI.Local environment limitation
The full repository
make test/make fmtlint phase could not type-check the TPM simulator package because local OpenSSL development headers are absent. The modified package test and lint sets pass, andmake buildpasses.