daemon: fix DHCP-server-in-cluster enablement — untagged reth1.0 filter + runtime knob-flip (#4647) - #4649
Merged
Merged
Conversation
DHCP-server-in-cluster was non-functional with the canonical Junos
config `set system services dhcp-local-server group g0 interface
reth1.0`. filterDHCPConfigForMasterRGs kept a dhcp-local-server group
only when its resolved interface string-equalled a master-RG reth
member. For an UNTAGGED reth unit the two sides disagreed:
- rethInterfacesForRG emits the bare member "ge-0-0-1" for a
VlanID==0 unit (no suffix), while
- resolveDHCPRethInterfaces resolves "reth1.0" -> "ge-0-0-1.0"
(ResolveReth keeps the ".0", pkg/config/types.go).
The exact compare failed, the group was filtered to nil, and the
cluster-commit path (ApplyClusterCommit(nil) -> clearFamilyLocked)
REMOVED /etc/kea/kea-dhcp4.conf, so Kea stayed failed. Changing the
config to `interface reth1` (which resolves to the bare "ge-0-0-1")
was the only way to make Kea generate + serve — proving the filter,
not the loader, was the fault.
Fix: normalize a trailing ".0" (untagged unit) on BOTH sides of the
compare via stripUntaggedUnitSuffix, AND keep the normalized bare
member in the kept set. "ge-0-0-1" is the real kernel device Kea binds
to; "ge-0-0-1.0" is not a device, so keeping the bare name is required
for Kea to actually serve. A TAGGED unit (reth1.100 -> ge-0-0-1.100)
is left intact and still matches only its VLAN member — the ".0"
normalization cannot collapse a tagged unit or match the wrong reth.
Tests (daemon_dhcp_filter_4647_test.go): an untagged reth1.0 group on a
MASTER RG survives the filter with kept interface [ge-0-0-1]
(RED-on-revert: filtered to nil with the old exact compare); a tagged
reth1.100 group still matches [ge-0-0-1.100]; a group on a BACKUP RG is
still filtered to nil.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015oARShYtiJJ2H4UB4nXGqi
A runtime `set chassis cluster dhcp-lease-synchronization; commit` on a RUNNING cluster was a silent no-op. runDHCPLeaseSyncLoop was launched ONLY from the cluster connect-time block (startClusterComms), gated on the knob at peer-connect. Toggling the knob on later did not start the push loop — the #2239 lease-sync counters stayed 0/0 until an xpfd restart re-ran the connect-time launch. Fix: reconcile the loop from the config-apply path. A new idempotent ensureDHCPLeaseSyncLoop(enabled) starts the loop when the knob is on and not already running, and stops it when the knob goes off. It is invoked from applyTailReconciles (section 20, alongside the #87 cluster transport reconcile) with the just-committed knob, so a knob-ON commit (re)launches it and a knob-OFF commit stops it without a restart. The connect-time block now routes through the SAME helper, so the two launch sites share one start/stop path and cannot double-launch (guarded by the loopCancel handle under loopMu). The loop is scoped to the live cluster comms context (clusterCommsCtx, newly held on the daemon) so it shares the lifetime of the session-sync channel it pushes over — the loop dereferences d.sessionSync, which is torn down with comms. stopClusterComms nils clusterCommsCtx and clears the loop handle (resetDHCPLeaseSyncLoop); the loop's context is a child of the comms context, so cancelling comms already stops it, and the next comms session's connect-time launch starts a fresh loop. The apply-path call is a no-op while comms are down (clusterCommsCtx/sessionSync nil) — that case stays owned by the connect-time launch, so the knob-ON state is never lost. Also fix the lab smoke harness test/incus/dhcp-lease-failover.sh so it runs as-is: invoke `/usr/local/sbin/cli` (the binary is `cli`, not `xpf-cli`) and default DHCP_CLIENT_IFACE to eth0 (the loss LAN host presents its DHCP client on eth0, not eth1; still overridable). Tests (daemon_dhcp_leasesync_4647_test.go): a knob-ON toggle launches the loop (RED-on-revert: loopCancel stays nil without the launch), a second ON toggle does not double-launch (same cancel handle), a knob-OFF toggle stops it, and an ON toggle with comms down does not launch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015oARShYtiJJ2H4UB4nXGqi
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two HA/cluster enablement issues around DHCP-server-in-cluster: (1) untagged rethX.0 interface resolution mismatching the master-RG filter (preventing Kea config generation), and (2) the DHCP lease-sync push loop not starting/stopping when the dhcp-lease-synchronization knob is toggled at runtime.
Changes:
- Normalize untagged
.0interface suffix handling in the master-RG DHCP config filter so canonicalinterface reth1.0survives and Kea binds to the real kernel device. - Add an idempotent start/stop reconciler (
ensureDHCPLeaseSyncLoop) and wire it into both connect-time and apply-time paths for runtime knob flips. - Update docs/tests and the lab smoke script to reflect the fixed behavior and correct CLI/iface defaults.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/incus/dhcp-lease-failover.sh | Adjusts smoke defaults (eth0) and CLI invocation path for lab validation. |
| pkg/dhcpserver/README.md | Documents the #4647 cluster enablement bugs and fixes for future operators/devs. |
| pkg/daemon/daemon.go | Stores the live cluster comms context for comms-scoped loop lifetimes. |
| pkg/daemon/daemon_ha.go | Fixes DHCP group filtering by normalizing untagged .0 interface suffixes. |
| pkg/daemon/daemon_ha_sync.go | Persists comms context and routes connect-time lease-sync start through the new helper; resets on comms stop. |
| pkg/daemon/daemon_dhcp_filter_4647_test.go | Adds regression tests for untagged/tagged reth master-RG filtering behavior. |
| pkg/daemon/daemon_dhcp_lease_sync.go | Adds runtime start/stop reconciliation for the lease-sync loop and scopes it to comms lifetime. |
| pkg/daemon/daemon_dhcp_leasesync_4647_test.go | Adds regression tests for knob-toggle behavior and comms-down behavior. |
| pkg/daemon/daemon_apply.go | Reconciles lease-sync loop enablement on config apply to support runtime knob flips. |
| _Log.md | Logs the write/edit action per repo process. |
Comment on lines
+148
to
+152
| if d.dhcpServer == nil || d.sessionSync == nil || d.clusterCommsCtx == nil { | ||
| return | ||
| } | ||
| loopCtx, cancel := context.WithCancel(d.clusterCommsCtx) | ||
| d.dhcpLeaseSync.loopCancel = cancel |
Comment on lines
+37
to
+41
| // Idempotent: a second ON toggle must NOT replace the running loop. | ||
| d.ensureDHCPLeaseSyncLoop(true) | ||
| if reflect.ValueOf(d.dhcpLeaseSync.loopCancel).Pointer() != reflect.ValueOf(first).Pointer() { | ||
| t.Fatal("double-launch: second ON toggle replaced the running loop's cancel handle") | ||
| } |
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 #4647
Two enablement bugs the #2261 DHCP-lease-failover live smoke surfaced on the
loss cluster. The memfile loader itself PASSED; both bugs are in the
surrounding daemon-side plumbing.
BUG A (serious) — cluster DHCP server never starts with the canonical config
filterDHCPConfigForMasterRGs(pkg/daemon/daemon_ha.go) kept adhcp-local-servergroup only if its resolved interface string-equalled amaster-RG reth member. For an UNTAGGED reth unit the two derivations disagree:
rethInterfacesForRGemits the bare memberge-0-0-1for aVlanID==0unit (no suffix), while
resolveDHCPRethInterfacesresolvesreth1.0→ge-0-0-1.0(ResolveRethkeeps the
.0,pkg/config/types.go).The exact compare failed → the group was filtered to nil →
ApplyClusterCommit(nil)→clearFamilyLockedREMOVED/etc/kea/kea-dhcp4.conf→ Kea stayedfailed. Changing onlyinterface reth1.0→reth1flipped Kea from never-generating toPRESENT+serving, proving the filter (not the loader) was the fault. So
DHCP-server-in-cluster was entirely non-functional with the canonical Junos
interface reth1.0indocs/ha-cluster-userspace.conf.Fix: normalize a trailing
.0(untagged unit) on BOTH sides of the compare(
stripUntaggedUnitSuffix) AND keep the normalized bare member in the kept set—
ge-0-0-1is the real kernel device Kea binds to;ge-0-0-1.0is not adevice. A TAGGED unit (
reth1.100→ge-0-0-1.100) is left intact and stillmatches only its VLAN member.
BUG B — runtime knob-flip was a silent no-op
runDHCPLeaseSyncLoopwas launched ONLY from the cluster connect-time block,gated on the knob at peer-connect. A runtime
set chassis cluster dhcp-lease-synchronization; commitdid NOT start the pushloop — counters stayed 0/0 until an xpfd restart.
Fix: a new idempotent
ensureDHCPLeaseSyncLoop(enabled)reconciled from theapply path (
applyTailReconciles, section 20) starts the loop when the knob ison and stops it when off, without a restart. The connect-time block now routes
through the same helper, so both launch sites share one start/stop path and
cannot double-launch (guarded by
loopCancelunderloopMu). The loop isscoped to the live cluster comms context (
clusterCommsCtx) so it shares thelifetime of the session-sync channel it pushes over;
stopClusterCommsclearsit.
Smoke script (lab-env, minor)
test/incus/dhcp-lease-failover.sh:xpf-cli→/usr/local/sbin/cli(thebinary is
cli), andDHCP_CLIENT_IFACEdefaulteth1→eth0(the loss LANhost presents its DHCP client on eth0; still overridable).
Tests
daemon_dhcp_filter_4647_test.go): untaggedreth1.0group on aMASTER RG survives the filter with kept
[ge-0-0-1](RED-on-revert: filteredto nil with the old exact compare); tagged
reth1.100still matches[ge-0-0-1.100]; a BACKUP-RG group is still filtered to nil.daemon_dhcp_leasesync_4647_test.go): a knob-ON toggle launches theloop (RED-on-revert: not launched); a second ON toggle does not double-launch;
a knob-OFF toggle stops it; an ON toggle with comms down does not launch.
go test ./pkg/daemon/... ./pkg/dhcpserver/...green;go build ./...ok;gofmt/vet clean;
bash -non the smoke script ok.Cluster validation flag: this touches cluster/HA DHCP plumbing. The #2261
test/incus/dhcp-lease-failover.shsmoke (lab-gated) should be re-run toconfirm the canonical
interface reth1.0config now serves and the knob-ONcounters increment without a restart.
🤖 Generated with Claude Code
https://claude.ai/code/session_015oARShYtiJJ2H4UB4nXGqi