Declare DHCP reservations on the guest shapes and reconcile them - #423
Merged
Conversation
22 reservations were hand-made, five shapes across four stacks carried "⚠ needs a DHCP reservation" comments, and nothing detected a lost or edited one. Six of them now carry a per-client local_dns_record, so the hand-made object stopped being just an address — it is the name services are targeted by. A member declares `network.reservation` and converge reconciles it after create, reading the guest's live MAC off `pct config` rather than making the shape predict one. That keeps the existing decision not to pin hwaddr intact, and the UniFi network is resolved from the VLAN the shape already declares. Two blockers had to go first. converge-unifi could not authenticate at all. The legacy adapter only knew session auth, so it wanted UNIFI_USERNAME/UNIFI_PASSWORD, and secrets.env has never carried them — meaning the path that owns the public :443 port-forward was dead on any machine rebuilt from secrets-sync.sh. Chrison.UnifiSharp 0.2.0 adds X-API-KEY on the legacy API and derives its site URL from UNIFI_LOCAL_HOST. UnifiConverge.Plan matched on name alone, so a port-forward whose target had been re-pointed in the UI read as "present" and converge reported success forever. It now compares field by field and corrects the drift — the same write-once failure the CF Access bypass had (#417). Drift IS the failure mode for reservations, so that had to be true before this was worth building. Three guardrails, all load-bearing: - Add-only by identity. The controller holds a row per MAC it has ever seen — 242 of them against 19 reservations — so every write is keyed to a MAC a shape declares. Undeclared reservations are reported as candidates, never pruned. - `parked:` states why a reservation is held for a deliberately-stopped guest, so it is neither resurrected nor flagged as drift every run. Without it the only way to tell a parked reservation from an orphan is to cross-check Proxmox by hand. - A shape that declares no localDnsRecord does not claim the field, so converge never blanks a record set by hand. `homelab-infra unifi-reservations <stacks-dir>` audits declared against live and names what no shape accounts for. It matches on address, not MAC, so it runs from the repo alone — no Proxmox, nothing converged first. Backfills the four in-tree reservations (Media seerr/audiobookshelf/podman-host, monitoring podman-host). SmartHome and DevOps hold eleven more and are submodules, so they follow separately once the schema-v1 release carries the new key. Also fixes the stack-defaults merge for `network`, which was all-or-nothing: a member declaring only `network.reservation` would have silently forfeited its bridge, VLAN and address. Field-wise now, and a no-op for all three shapes that declare the block. A default reservation is deliberately not inherited — it would hand every member in the stack the same fixed address. Refs #416. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ChrisonSimtian
marked this pull request as ready for review
August 16, 2026 05:19
Collaborator
Author
|
The blocker is cleared: Chrison.UnifiSharp 0.2.0 is on nuget.org (UnifiSharp#17 merged, tagged
One gotcha if anyone hits |
Open
3 tasks
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.
Closes #416.
Important
Draft — blocked on Chrison-dev/UnifiSharp#17. The engine pins
Chrison.UnifiSharp 0.2.0, which isn't on nuget.org yet, so restore fails in CI until that PR merges and av0.2.0tag ships the package. Everything here was built and tested against a locally-packed 0.2.0.What this does
A member declares its reservation on its own shape:
…and converge reconciles it after create, reading the guest's live MAC off
pct configrather than making the shape predict one. That was the design question #416 left open, and reading it live keeps the existing decision not to pinhwaddr(homeassistant.lxc.yaml:73) intact. The UniFi network is resolved from the VLAN the shape already declares, so it's never restated.Two blockers had to go first
converge-unificould not authenticate at all. The legacy adapter only knew session auth, so it wantedUNIFI_USERNAME/UNIFI_PASSWORD— andsecrets.envhas never carried them. The path that owns the public:443port-forward was dead on any machine rebuilt fromsecrets-sync.sh:Now, from the same shell:
UnifiConverge.Planmatched on name alone, so a port-forward whose target had been re-pointed in the UI read asAlreadyPresentand converge reported success forever — the same write-once failure the CF Access bypass had (#417). It now compares field by field and corrects the drift. That had to be true before reservations were worth building, because drift is the failure mode for a reservation.Three guardrails
parked:records why a reservation is held for a deliberately-stopped guest, so it is neither resurrected nor flagged as drift on every run. This matters more than it sounds: of the 22 reservations audited, four were parked (HA VM 2000's three NICs,Plex-VM) and three were genuine orphans, and the only way to tell them apart is to cross-check Proxmox by hand.localDnsRecorddoesn't claim the field, so converge never blanks a record that was set by hand for a reason.The audit command
It matches on address, not MAC, so it runs from the repo alone — no Proxmox, nothing converged first.
A subtle fix worth reviewing
member.Network ??= defaults.Networkwas all-or-nothing, so a member declaring onlynetwork.reservationwould have silently forfeited its bridge, VLAN and address, and been rendered onto the wrong network. It's field-wise now — a no-op for all three shapes that declare the block today. A default reservation is deliberately not inherited, or every member in a stack would be handed the same fixed address. Both cases are tested.Scope
Backfills the four in-tree reservations. SmartHome (9) and DevOps (1) are submodules and follow separately — their
validate.ymlpulls the schema from theschema-v1release, so that asset has to carry the newreservationkey before they can adopt it. Four more have no guest shape to live on: the NAS, the Zigbee gateway, PDM (CT 2002, no shape exists) andPlex-VM.Verification
converge-unifiandunifi-reservationsboth run live against the real controller.okfor each. The underlyingPUT /rest/user/<id>is proven: it's what cleared the three orphans found during the audit.🤖 Generated with Claude Code