feat(localdns): add systemd-based Prometheus-format metrics exporter#7917
Open
feat(localdns): add systemd-based Prometheus-format metrics exporter#7917
Conversation
saewoni
requested review from
AbelHu,
Devinwong,
awesomenix,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
juan-lee,
junjiezhang1997,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
timmy-wright and
zachary-bailey
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a lightweight Prometheus-compatible metrics exporter for the localdns.service using systemd socket activation. The exporter exposes CPU and memory metrics on port 9353 with zero overhead when not being scraped, making it suitable for production monitoring.
Changes:
- Added
localdns_exporter.shbash script that queries systemd accounting metrics and formats them as Prometheus metrics - Added systemd socket (
localdns-exporter.socket) and service (localdns-exporter@.service) units for on-demand activation - Integrated the exporter into all Linux VHD builds (Ubuntu, Mariner, Flatcar, ARM64/x64)
- Added VHD content validation tests and a standalone test script
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| parts/linux/cloud-init/artifacts/localdns_exporter.sh | Bash script that scrapes systemd metrics (CPUUsageNSec, MemoryCurrent) and outputs Prometheus-formatted HTTP response |
| parts/linux/cloud-init/artifacts/localdns-exporter@.service | Systemd template service for per-connection worker instances with security hardening |
| parts/linux/cloud-init/artifacts/localdns-exporter.socket | Systemd socket unit listening on port 9353 with Accept=yes for on-demand activation |
| vhdbuilder/packer/vhd-image-builder-*.json | Added file provisioners to copy exporter artifacts to all Linux VHD variants |
| vhdbuilder/packer/packer_source.sh | Added file copying logic and systemctl enable command for the socket |
| vhdbuilder/packer/imagecustomizer/azlosguard/azlosguard.yml | Added exporter files to OSGuard VHD build but missing socket enablement |
| vhdbuilder/packer/test/linux-vhd-content-test.sh | Added validation for exporter files and permissions |
| e2e/test-localdns-exporter.sh | Standalone test script for manual validation |
saewoni
changed the title
saewoni
force-pushed
the
sakwa/add_localdns_metrics__systemd
branch
from
223b0c3 to
83fc4e2
Compare
…mplate Remove the [Install] section from localdns-exporter@.service since it's a socket-activated template unit with Accept=yes. Why this change is needed: - Socket-activated services with Accept=yes spawn instances on-demand - Service instances are started by the socket, not by systemd at boot - The [Install] section with WantedBy=multi-user.target is misleading because it suggests the template should be enabled directly - Only the socket (localdns-exporter.socket) needs to be enabled Current behavior (unchanged): - CSE enables localdns-exporter.socket via systemctlEnableAndStartNoBlock - When connections arrive on 127.0.0.1:9353, systemd spawns instances of localdns-exporter@N.service (where N is the connection number) - Service instances inherit settings from the template unit The [Install] section in a template unit is never used and only causes confusion about the activation mechanism. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…driven test Replace 12 nearly-identical test functions with a single table-driven test to reduce code duplication and improve maintainability. Before: - 12 separate test functions (Test_Ubuntu2204LocalDns_ExporterMetrics, etc.) - 235 lines of duplicated code - Hard to maintain: any behavior change requires updating 12 functions - Easy to introduce inconsistencies across tests After: - 1 table-driven test (Test_LocalDns_ExporterMetrics) with 12 subtests - 90 lines of code (62% reduction) - Easy to add new VHDs: just add one table entry - Single source of truth for test behavior - Consistent validation across all VHDs Test output remains the same: - go test -v shows: Test_LocalDns_ExporterMetrics/Ubuntu2204 - Each VHD still runs as a distinct subtest with proper isolation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…heck Exclude the inherited stdin/stdout socket when checking for network sockets in socket-activated service instances. Problem: - Socket-activated services with Accept=yes inherit the accepted TCP connection as stdin (fd 0) and stdout (fd 1) - This inherited socket is AF_INET (matches ListenStream=127.0.0.1:9353) - The runtime check was flagging this expected socket as a violation - This caused false positives when the service instance was still running during the test (curl still reading metrics response) Why the inherited socket is expected: - systemd passes the accepted connection to the service via stdin/stdout - This is how socket activation works with Accept=yes - The socket is inherited, not created by the service - RestrictAddressFamilies=AF_UNIX prevents the service from creating NEW network sockets, but doesn't block inherited sockets Solution: - Identify the stdin socket inode before checking - Skip the inherited socket when counting network sockets - Only flag sockets the service creates itself as violations This allows the test to correctly validate that the service cannot create network connections while still allowing the expected inherited activation socket. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add kubernetes.azure.com/localdns-exporter=enabled label to nodes when the localdns-exporter.socket service is successfully enabled during provisioning. This allows targeting nodes with localdns metrics enabled using node selectors. Changes: - Add addKubeletNodeLabel() call in enableLocalDNS() when exporter succeeds - Add unit tests to verify label is added on success and not on failure - Regenerate all snapshot test data Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ACL VHD builds were failing with exit code 113 (ERR_PACKER_COPY_FILE) because localdns_exporter.sh, localdns-exporter.socket, and localdns-exporter@.service were never uploaded to the build VM. These file provisioner entries existed in all other templates but were missing from the ACL and ACL ARM64 templates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ot in pipeline build
Instead of spinning up 10 dedicated VMs in Test_LocalDns_ExporterMetrics, run ValidateLocalDNSExporterMetrics as part of ValidateCommonLinux for all localdns-supported distros. This also fixes two issues: the 8KB bastion SSH buffer overflow (via chunked base64 upload) and systemd template unit property queries returning empty (by spawning a held-open connection to inspect a live instance). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…aping to localdns.sh Rewrite the forward IP parser to capture all corefile server blocks with a "block" label (e.g., block=".:53", block="cluster.local:53") instead of only parsing the root zone. Move systemd resource metric scraping (CPU, memory, status) from the exporter into localdns.sh via a new export_resource_metrics() function that writes resources.prom atomically, avoiding D-Bus failures under DynamicUser=yes. The exporter now simply cats pre-generated .prom files. E2e validation is strengthened with DNS load generation, non-zero resource assertions, multi-line forward metric validation, and POSIX-compatible socket inode extraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The localdns-exporter socket was bound to 127.0.0.1:9353, preventing vmagent from scraping metrics via the CCP overlay proxy (which connects to the node's IP). Add a systemd drop-in override generated at CSE time that rebinds the socket to the node IP, matching node-exporter's approach. The VHD retains 127.0.0.1 as a safe default. Also update the e2e validation script to dynamically detect the listen address and log the drop-in config for debugging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
hostname -I returns empty when network interfaces aren't fully configured during CSE, causing ListenStream=:9353 (invalid) in the systemd socket drop-in. This affected ~64% of VMs in pipeline testing. Switch to get_primary_nic_ip() which reads from the IMDS metadata cache populated earlier in CSE by fetch_and_cache_imds_instance_metadata. This is the same proven pattern used by the kubelet IMDS restriction drop-in and is guaranteed to have valid data at this point in the provisioning flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…compat Move the localdns-exporter socket configuration from enableLocalDNS() (basePrep) into a new configureLocalDNSExporterSocket() function called from nodePrep(). This fixes VHDCaching where basePrep runs on a capture VM with a different IP than the final node — the drop-in with a baked IP would cause socket bind failures on Stage 2. The new function: - Creates the systemd socket drop-in with the actual node IP - Enables the exporter socket - Adds the kubelet node label (before ensureKubelet so it takes effect) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change the VHD-baked socket default from 127.0.0.1:9353 to 0.0.0.0:9353 so vmagent can scrape metrics via the node's InternalIP without requiring a drop-in. CSE still creates an optional drop-in to narrow binding to the node IP via IMDS when available. Removes hostname -I fallback chain — if IMDS is empty, the VHD default 0.0.0.0 already works. Adds || true at the call site so exporter socket setup never blocks provisioning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add systemctl cat check before attempting to enable the exporter socket. On old VHDs without the socket unit file, this prevents systemctlEnableAndStartNoBlock from retrying for ~8 minutes before giving up. Also changes VHD default listen address from 127.0.0.1 to 0.0.0.0 so vmagent can scrape without requiring a drop-in. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The exporter process exits after each response, causing systemd to close the connection. Adding the header makes this explicit to HTTP clients so they don't expect to reuse the connection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The connection closes regardless when the process exits. The header is unnecessary and untested with vmagent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…porterSocket On old VHDs without the localdns-exporter.socket unit, the function was creating a stale drop-in directory, calling get_primary_nic_ip, and running daemon-reload before the guard skipped the enable step. Move the guard to the top so the function is a clean no-op when the unit is missing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename localdns_memory_usage_mb to localdns_memory_usage_bytes per Prometheus base-unit naming convention. Expose raw bytes from systemd MemoryCurrent instead of dividing by 1048576. This must ship before the metric name becomes permanent in production dashboards. Increase CPU precision from %.4f to %.9f to preserve nanosecond source precision from CPUUsageNSec, avoiding step-function artifacts in rate() calculations at low CPU usage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…st filter Add Connection: close header to HTTP responses in localdns_exporter.sh for HTTP/1.1 compliance (message framing without Content-Length). Add localdns_metrics_last_update_timestamp_seconds gauge to detect stale .prom data when the watchdog stops updating. Consumers can alert on: time() - localdns_metrics_last_update_timestamp_seconds > 120. Fix run-localdns-test.sh to actually pass the test name argument as -run flag to go test. Previously the filter was silently ignored. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add MaxConnections=10 to localdns-exporter.socket to prevent unbounded process spawning from port scans or misbehaving scrapers. Excess connections are refused with TCP RST; Prometheus retries next interval. Widen RestrictAddressFamilies to AF_UNIX AF_INET AF_INET6 in the service unit. Socket activation passes an AF_INET fd as stdin/stdout; current systemd exempts inherited fds from seccomp filtering, but this is an implementation detail not a guarantee. Adding AF_INET/AF_INET6 future-proofs against stricter seccomp enforcement in newer systemd. Pass LOCALDNS_SCRIPT_PATH via Environment= in the service unit so the exporter derives .prom file paths from the same source as localdns.sh, eliminating silent divergence if the path ever changes. Update e2e test to validate the new RestrictAddressFamilies allowlist. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove run-localdns-test.sh which was unused by any code in the repo. Localdns tests can be run via standard TAGS_TO_RUN filtering instead. Tighten ss port grep from ':9353' to ':9353[[:space:]]' to prevent false substring matches against ports like 93539. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ile parser - Switch from systemctlEnableAndStartNoBlock to systemctlEnableAndStart for the exporter socket unit. Socket units activate in <10ms so blocking is fine and ensures the socket is active before adding the kubelet node label. - Fix stale comment in localdns-exporter@.service that said "reads systemd properties" when the exporter only reads .prom files. - Simplify corefile parser: store full zone:port from $1 instead of stripping :53 and re-appending it. Same metric output, port now comes from one source (the corefile) instead of being hardcoded in two places. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… status check Move localdns-exporter socket setup after ensureKubelet to avoid adding latency to kubelet startup. The kubelet node label (which must be set before kubelet starts) is split out and added separately before ensureKubelet (~0ms variable append). Additional PR feedback fixes: - Replace systemctl is-active with kill -0 $COREDNS_PID for accurate service status (systemctl always returns "active" from within the service) - Combine two systemctl show calls into one to reduce D-Bus overhead - Add read -t 5 timeout in exporter to prevent indefinite blocking - Use systemctlEnableAndStartNoBlock since post-kubelet ordering removes the need to wait for socket activation - Update HELP comments to reflect kill -0 based status check Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update shellspec tests to reflect code changes: - Remove enableLocalDNS exporter tests (moved to configureLocalDNSExporterSocket) - Add block= label to forward IP metric assertions in localdns_spec - Fix missing status assertions to include block="none" Fix OSGuard imagecustomizer config: localdns.sh was placed at /opt/azure/containers/localdns.sh but localdns.service ExecStart expects /opt/azure/containers/localdns/localdns.sh. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…k to test fixtures Replace nc (netcat) with bash /dev/tcp for holding open a TCP connection in e2e validation — nc may not be installed on all AKS distros. Add the production health-check server block (bind 169.254.10.10 169.254.10.11, whoami, no forward) to all corefile test fixtures so the parser is tested against the real corefile structure and regressions from the health-check block are caught. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e comments Only add the localdns-exporter kubelet node label if the exporter socket unit exists on the VHD. This prevents old VHDs (without exporter files) from advertising exporter=enabled when no exporter is available to scrape. Also fix stale comments in enableLocalDNS and configureLocalDNSExporterSocket to accurately describe the current boot sequence and guard purpose. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Flatcar compiles bash without --enable-net-redirections, so /dev/tcp is not available. Revert to nc (available on all AKS VHD distros) for holding the TCP connection open during security inspection. Use `sleep 120 | nc` to keep stdin open and prevent nc from closing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The e2e validation script now checks if localdns-exporter.socket exists on the VHD before running. Older VHDs built from main don't have the exporter units — exit 0 gracefully instead of hard-failing. If the unit IS installed, all checks still run and any failure is a real bug. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add three unit tests covering the branching logic in export_resource_metrics: - Active status when COREDNS_PID is alive (CPU/memory conversion verified) - Inactive status when COREDNS_PID is empty - Defaults to zero when systemctl returns [not set] values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… validation gating Move the exporter validation skip logic from a silent `exit 0` in the bash script to a Go-level node label check. The label `kubernetes.azure.com/localdns-exporter` is only set by CSE when the VHD has the exporter socket installed, so it's the right signal. If the label is absent, the test logs a WARNING (visible in output) and skips. If the label is present, full validation runs and hard-fails on any issue. This avoids silently passing when something is broken. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Adds CPU and memory metrics for localdns.service using systemd accounting and socket activation for efficient, zero-overhead monitoring.
Also exports IP addresses configured for forward plugins for kubedns and vnetdns overrides.
Implementation:
NoNewPrivileges=yes)
Metrics exposed:
Test coverage:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #