feat: Detect upstream/downstream control-plane DNS drift via metrics + alerting#65
Open
scotwells wants to merge 9 commits into
Open
feat: Detect upstream/downstream control-plane DNS drift via metrics + alerting#65scotwells wants to merge 9 commits into
scotwells wants to merge 9 commits into
Conversation
…+ alerting
Adds detection for the failure class behind engineering#346, where a leftover downstream DNSRecordSet kept "reserving" a hostname after its upstream owner was deleted (a multicluster-runtime sharding bug let it linger), so later records for that name were rejected as duplicates and the customer domain was unresolvable for ~24h. There was no signal for the underlying condition — a resource existing at one replication seam with no owner at the seam above it — which is directly observable as a discrepancy between per-object metrics collected from each control plane.
The approach uses milo-os/resource-metrics (discovery.mode=milo + collectRootControlPlane) to emit one gauge series per DNSRecordSet/DNSZone from every project (upstream) control plane and the Milo core (downstream) control plane, lifting the replicator's meta.datumapis.com/upstream-* annotations onto labels so the two sides share a join key. Prometheus recording rules then diff them: dns:recordset_downstream_orphan (downstream with no surviving upstream owner — the #346 case) and dns:recordset_downstream_missing (upstream never replicated), with the DNSDownstreamOrphanRecordSet / DNSDownstreamMissingRecordSet / DNSRecordSetNotAccepted alerts. Rules ship as a standard PrometheusRule (victoria-metrics-operator converts it) labeled for the resource-metrics aggregator vmalert; no new operator code is required for the primary signal.
config/observability holds the dns-metrics ResourceMetricsPolicy and the dns-drift PrometheusRule. config/dependencies/{milo,resource-metrics} and config/overlays/{replicator-milo,agent-powerdns-milo} bring up the production-accurate Milo topology (mode:milo replicator with 2 active replicas, PowerDNS agent reading the core CP) for the e2e. test/e2e/controlplane-drift is a Chainsaw suite (happy-path / orphan / missing) verified end-to-end against a from-scratch rebuild, wired via new Taskfile tasks (env:milo-all-up, env:milo-bootstrap, env:upstream-milo-up, env:metrics-up, env:observability-up, env:chainsaw-milo). See docs/enhancements/controlplane-drift-detection.md for the full design, the datum-cloud/infra alignment, and the live-validated findings (e.g. the cluster-_<project> join-key normalization).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the engineering design/journey write-up with a product-focused doc centered on what the feature detects and how on-call responds: a plain-language explanation of the failure, a table of the three alerts with what each means and what to do, a short conceptual diagram, and a brief try-it/enable-in-prod section. Drops the internal build plan, phase/sequencing notes, code-path references, and open questions that belonged to the implementation rather than the product. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ored manifests Replace the vendored resource-metrics controller manifests (namespace, RBAC, Deployment, server-config, milo-kubeconfig Secret) with a Flux OCIRepository + Kustomization referencing the operator's published kustomize bundle (oci://ghcr.io/milo-os/resource-metrics-kustomize, overlays/test-infra) — the same pattern config/dependencies/milo already uses for milo. That overlay already provides the Deployment, RBAC, namespace, the token-based milo-kubeconfig Secret, and a mode:milo + collectRootControlPlane:true server-config, so we only override the controller image tag and the OTLP endpoint (our observability stack's collector lives in telemetry-system, not the overlay's default otel-collector-system). This drops the vendored manifests that would drift as the operator evolves and removes the local image build — the pinned bundle ships a multi-arch (amd64+arm64) image. The ResourceMetricsPolicy CRD + dns-metrics policy still install onto the Milo core CP unchanged. env:metrics-up now deploys via Flux and waits on the Kustomization instead of loading a local image. Verified live: the controller comes up on the published arm64 image, picks up the telemetry-system endpoint, and collects root + project control planes with no export errors. Once milo-os/resource-metrics#13 (multi-arch) and #14 (configurable OTLP endpoint) land in main, the pin bumps to a v0.0.0-main tag and the endpoint patch collapses to an OTEL_EXPORTER_OTLP_ENDPOINT env patch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stop vendoring the ResourceMetricsPolicy CRD. Replace it with a Flux Kustomization that installs the CRD onto the Milo core control plane straight from the operator's published bundle (path: crd) via kubeConfig.secretRef — mirroring infra apps/resource-metrics-system/base/milo-control-plane.yaml. The Flux object lives on the local cluster and targets the core CP through a milo-kubeconfig Secret in flux-system (in-cluster milo-apiserver + the test-only test-admin-token; production uses a cert-based kubeconfig). This was the one piece still vendored after moving the controller to the bundle; now the entire resource-metrics deployment (controller + CRD) comes from upstream, and only the dns-operator-owned dns-metrics policy is applied directly. env:metrics-up applies both Flux Kustomizations on the local cluster and waits on the CRD one before applying the policy. Verified live: the CRD Kustomization reconciles Ready ("Applied revision: <bundle>@sha256…") and the CRD on the core CP is now Flux-managed (kustomize.toolkit.fluxcd.io/name=resource-metrics-crd).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The replicator-milo overlay disabled leader election by restating the manager container's entire args list minus --leader-elect. Strategic-merge replaces scalar lists wholesale, so that overlay silently drops any flag the base later adds (it was already dropping the metrics/webhook flags other patches contribute). Default the leader-elect flag from a LEADER_ELECT env var (cmd/main.go), set it in the base manager env, and have the overlay patch just that env var — env lists merge by name, so the overlay now changes only what it means to and inherits the rest of the base args. The flag still works and still wins if passed explicitly. Same pattern the agent-powerdns-milo overlay already uses for KUBECONFIG. Verified: go vet passes and the rendered overlay keeps the full base args with LEADER_ELECT=false + the milo mount. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… in code Follow-up to c6d332f: revert the Go-side env parsing (envBool) and instead reference the env var from the flag via Kubernetes' native $(VAR) arg substitution — `--leader-elect=$(LEADER_ELECT)` with LEADER_ELECT set in the container env. This keeps the operator binary unchanged (standard --leader-elect flag) while still letting overlays toggle leader election by patching a single env var (env lists merge by name) rather than restating the whole args list. Verified live with a freshly built image: LEADER_ELECT=true acquires a leader-election Lease (the flag default is false and there is no bare --leader-elect, so it only turns on via the substituted env), LEADER_ELECT=false runs both replicas with no lease. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Trim the API comments on the ResourceMetricsPolicy and PrometheusRule per the Google technical-writing style: present tense, active voice, one idea per sentence, and no restating what the names and expressions already say. Halves the comment volume (policy 29→15 lines, rules 49→25) with no change to any generator, label, CEL expression, or rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apply the Google technical-writing style to the READMEs added for this feature (present tense, active voice, no restating the obvious, drop resolved "assumptions" sections) and fix content that went stale as the implementation settled. The resource-metrics README no longer describes the deleted controller/server-config.yaml or claims the image defaults to :latest (it references the published bundle now). The control-plane drift README documents the actual four-cluster chainsaw contract — including the replicator cluster on dns-upstream and the scale on deployment/dns-operator-controller-manager, not the earlier "infra hosts the replicator" / "scale --all" assumptions — and points at the kubeconfigs' real location. Cuts the two large READMEs roughly in half with no change to any command, path, or manifest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rewrite it purpose-first: lead with what the controller does (emit per-object metrics so the rules can spot records that fell out of sync) and that env:metrics-up applies it, then explain the two-cluster split in plain language. Drop the Flux resource file-tables and the reproduced kubectl command blocks (those duplicated the Taskfile and were how the doc went stale), keeping only what an editor needs: where to change the image tag / OTel endpoint / bundle pin, and the upstream follow-up note. Co-Authored-By: Claude Opus 4.8 <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
1 participant
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.
Note
Stacked on #60 (
feat/dns-federation-test-env) — review/merge that first. This PR's diff is scoped to the drift-detection work on top of the federation e2e environment.Summary
Adds detection for the failure class behind engineering#346: a leftover downstream
DNSRecordSetkept "reserving" a hostname after its upstream owner was deleted (amulticluster-runtimesharding bug let it linger), so later records for that name were rejected as duplicates and the customer domain was unresolvable for ~24h. We had no signal for the underlying condition — a resource existing at one replication seam with no owner at the seam above it — which is directly observable as a discrepancy between per-object metrics collected from each control plane.Approach
Use
milo-os/resource-metrics(discovery.mode: milo+collectRootControlPlane) to emit one gauge series perDNSRecordSet/DNSZonefrom every project (upstream) control plane and the Milo core (downstream) control plane, lifting the replicator'smeta.datumapis.com/upstream-*annotations onto labels so both sides share a join key. Prometheus recording rules diff them:dns:recordset_downstream_orphan— downstream object with no surviving upstream owner (the #346 case)dns:recordset_downstream_missing— upstream object never replicatedwith
DNSDownstreamOrphanRecordSet/DNSDownstreamMissingRecordSet/DNSRecordSetNotAcceptedalerts. Rules ship as a standardPrometheusRule(victoria-metrics-operator converts it toVMRule) labeled for the resource-metrics aggregator vmalert. No new operator code is required for the primary signal.What's included
config/observability/— thedns-metricsResourceMetricsPolicy+dns-driftPrometheusRuleconfig/dependencies/{milo,resource-metrics}/+config/overlays/{replicator-milo,agent-powerdns-milo}/— the production-accurate Milo topology for the e2e (mode:milo replicator with 2 active replicas; PowerDNS agent reading the core CP)test/e2e/controlplane-drift/— Chainsaw suite (happy-path / orphan / missing)Taskfile.yaml—env:milo-all-up,env:milo-bootstrap,env:upstream-milo-up,env:metrics-up,env:observability-up,env:chainsaw-milodocs/enhancements/controlplane-drift-detection.md— full design,datum-cloud/infraalignment, and live-validated findingsTest plan
task env:milo-all-upthentask env:chainsaw-milo— all three scenarios (happy-path / orphan / missing) PASS on a pristine 2-cluster kind environment (Milo core CP + resource-metrics + Victoria Metrics/OTel; mode:milo replicator).dns:recordset_downstream_orphanfires and the alert becomes active → remediation clears it.collectRootControlPlane: truein the infraresource-metricsconfig (not set today) — required for the downstream/core-CP series in staging/prod.Notes
resource-metricsimage on arm64 — see ci: Publish multi-arch (amd64+arm64) controller image milo-os/resource-metrics#13 (published image is amd64-only today).🤖 Generated with Claude Code