mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
Josh Siviour
eff11b611b
* fix(dhcp): release stranded leases and label them with the owning node Two paths dropped a lease from KV without returning it to the upstream server, so a source="dhcp" pool leaked addresses permanently. Manager.Start deleted an expired entry outright. Dropping the entry alone abandons the binding rather than returning it, and a server that ages leases by liveness holds the address for as long as anything answers ARP for it. Adoption now sends a bounded, best-effort RELEASE first, skipping leases with no stored offer/ack because nclient4 cannot rebuild those. Release-by-IP answered SUCCESS when the IP was absent from KV, telling the caller the address was freed when nothing went on the wire. It now returns an error. Every ReleaseIP call site logs and continues, so this surfaces the strand without wedging teardown. Leases were also unattributable upstream: chaddr is a derived HashMAC and option 61 carries that same MAC, so no field identified the host that took the lease. Option 12 is now "<node>-<client-id>" and option 60 is "mulga-spinifex/<node>", giving the lease table a per-lease identity and a per-host column to group on. HashMAC is untouched — encoding the node there would cut per-node entropy and reverse a documented decision. Observed live: 20 office-LAN addresses held by one node's orphaned OVN gateway LRPs, with no lease records and no renewals. * fix: rebind the datapath when a DHCP lease moves to a new address A lease re-issued on a different IP was persisted to KV with no notification of any kind. doAcquire replaced the lease after an expiry and doRenew accepted whatever a rebind answered with, while the gateway LRP's address is written once at attach and never revisited. Each such event burned one address permanently: the old one squatted by OVN's ARP responder with no renewer and no DHCPRELEASE, the new one renewed forever attached to nothing. Route both paths through applyLease, which releases the superseded address and invokes an IPChangeHook so the datapath can follow. vpcd registers a hook that moves the gateway LRP's networks and gateway_ip stamp, re-runs the routed leg so the VPC SNAT and host ingress route follow, and waits on the flows barrier. EIP, ENI-public and NATGW addresses are recorded outside vpcd in DescribeAddresses and DescribeInstances, so the hook reports those rather than repointing a datapath the control plane no longer agrees with. The release half still applies to every purpose, so the leak itself is closed either way. A failed rebind does not fail the lease loop: the new address is held regardless, and dropping it would strand both. * fix: reconcile API-visible records when their DHCP lease moves An EIP or auto-assigned public IP whose lease was re-issued elsewhere left DescribeAddresses and DescribeNetworkInterfaces advertising an address vpcd had already released. The datapath kept the old dnat_and_snat too, so the address was dead on both sides while the upstream server was free to hand it to someone else. vpcd cannot fix this alone: it holds the lease, but the records naming those addresses live daemon-side. Add vpc.dhcp.lease-changed, which vpcd requests and the daemon serves, dispatching to the service owning the moved address. RebindPublicIP and RebindENIPublicIP commit the new external IP through vpc.add-nat first — AddEIP scrubs the predecessor row keyed on the logical IP, so one commit is enough — and only then move the record, so a NAT failure never leaves a record advertising an address that does not route. Both refuse to move a record naming neither address, so a stale request cannot overwrite an allocation the lease does not own, and both are idempotent under duplicate delivery. A failed or unanswered reconcile propagates back to the lease loop rather than reading as success. vpc.dhcp.lease-expired, listed in the service manifest but never implemented, is replaced by the subject that now exists. * fix: dedupe retried AllocateAddress so one call takes one address EC2 AllocateAddress carries no ClientToken, and vpcd's DORA ladder can outlast the AWS client read timeout, so every SDK-level retry minted a fresh allocation ID and drew another upstream DHCP lease. The caller kept one EIP and the rest stayed leased with no record naming them, so nothing ever released them. Retries are now identified by the SDK's amz-sdk-invocation-id, which is stable across attempts of one call. The gateway lifts it off the request, NATSRequest forwards it as a header, and the daemon puts it back on the context, so the EIP service sees the same token every attempt. singleflight joins a retry to the call still in flight — the common case — and a TTL'd KV record answers retries that arrive after the first call completed. * fix: reclaim DHCP leases whose owner is gone and stop dropping expired ones Three leaks that all end the same way: an address stays leased upstream with nothing renewing it on purpose and nothing that will ever release it. Manager.Start deleted any lease that expired while vpcd was down, after releasing it. The resource it was taken for still existed, so the datapath kept forwarding on an address the upstream server now considers free — a duplicate-IP conflict waiting for the server to reissue it — and nothing ever re-acquired. Expired leases now get a renewal loop like any other, which re-DORAs on its first pass; the existing IP-change hook rebinds the datapath when the new address differs. The reaffirm RENEW is skipped for these, since a lease the server has already aged out will only fail it. Nothing reconciled leases against the resources that justify them, so a resource deleted out from under its lease left the address renewed forever. ReapOrphans sweeps them: vpcd asks the daemon over vpc.dhcp.owner-check whether the EIP, ENI or VPC still exists, and releases the lease when it does not. A gateway lease also has a router port to remove, so DetachIGW runs before the address goes back — releasing it while OVN still answers ARP for it invites the same conflict. Anything the daemon cannot resolve is reported unknown and kept: an unreachable daemon must never read as a fleet-wide deletion. The acquire ladder ran to ~62s, just past the 60s read timeout botocore defaults to, which is what turned one AllocateAddress into two leases. It now totals 40s with a 45s budget, and the DORA RPC 50s, so the caller hears the outcome instead of giving up and retrying. This is a mitigation, not a fix, for callers that send no retry token: botocore emits no amz-sdk-invocation-id, so the dedupe added earlier cannot see their retries at all. Fixes mulga-j19gj, mulga-9exe6, mulga-7qyzj. * feat: add spx get dhcp-leases so held external addresses are visible The upstream leases behind a source="dhcp" external pool lived in a per-AZ KV bucket with no read path, so working out which addresses were held, by what, and whether the owner still existed meant writing a NATS client or reading journals. That was the main obstacle to diagnosing every leak in this series. The command lists each lease with its purpose, VPC, address, MAC, holding node and remaining time, and resolves the owner over vpc.dhcp.owner-check so an orphan is visible as such. --orphans narrows the output to leaks; --skip-owner-check drops the lookup when only the addresses matter. Fixes mulga-655iu. * fix(vpcd): reconcile gateway ports onto their lease address at startup A gateway LRP configured from a static pool keeps its address when the pool is switched to source="dhcp", so the VPC forwards on an address nothing leases while the lease it holds goes unused. No lease event ever fires to correct it. vpcd now sweeps every gw-lrp lease on startup and rebinds the port to the address the lease actually holds. RebindGatewayIP is idempotent, so a port already in agreement costs one OVSDB read. * test: cover dhcp lease owner-check handlers and the lease CLI The owner-check path decides whether the reaper releases an address, so every inconclusive branch is asserted to report unknown rather than gone. * fix(dhcp): make the outcome of a stale lease release observable A stale RELEASE was logged only when the transport failed. A lease with no raw offer/ack was skipped in silence, and a RELEASE the server ignored looked identical to one it honoured, so an address left bound upstream could not be traced to either cause. The skip now warns, and a successful release logs the chaddr it used, which is what a server matches a RELEASE against. * fix(dhcp): take the RELEASE chaddr from the lease being released A server matches a RELEASE to a binding on chaddr, and the release path re-resolved it live. For a UseIfaceMAC lease whose interface MAC had changed since, that sends an address the binding never had: the server drops the RELEASE and says nothing, so the address stays held with no renewer. The chaddr now comes from the lease record, which carries what took the lease out, falling back to live resolution only when the record has none.