gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 17:11:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(dhcp): assert on persisted KV state, not the acquire counter (#624)

Browse Source 
Five manager tests waited on a fake DHCP counter and then read the KV
store. Fake.Acquire increments that counter before Manager.doAcquire
persists the entry, so the assertion ran in the gap between the two and
read the pre-acquire address whenever the write lost the race.

Idle machines always won the race, which is why this only ever failed
under load: the last three Build and Test failures on dev were this test
twice, on a diff that never touched the package.

Make the persisted KV state the barrier, matching the shape already used
by TestManagerStartKeepsNewLeaseWhenStaleReleaseFails, and keep the
counter as a plain assertion once the write is observed.
This commit is contained in:
Josh Siviour
2026-08-01 16:21:39 +10:00
committed by GitHub
parent d38db57912
commit a104f73306
1 changed files with 24 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spinifex/network/external/dhcp/manager_test.go
Vendored
+24 -17
View File
@@ -84,11 +84,14 @@ func TestManagerStartReacquiresExpiredLeases(t *testing.T) {
require.NoError(t, mgr.Start(context.Background()))
require.Eventually(t, func() bool { return fake.AcquireCount() == 1 }, time.Second, 10*time.Millisecond,
// The KV write trails the DORA, so waiting on the acquire count would race
// the assertion against it. The persisted address is the barrier.
require.Eventually(t, func() bool {
entry, getErr := store.Get(t.Context(), "eipalloc-old")
return getErr == nil && entry.Lease.IP.String() == "192.0.2.100"
}, time.Second, 10*time.Millisecond, "the KV record must name the new address")
assert.Equal(t, 1, fake.AcquireCount(),
"an expired lease whose resource still exists must be re-DORA'd, not dropped")
entry, err := store.Get(t.Context(), "eipalloc-old")
require.NoError(t, err)
assert.Equal(t, "192.0.2.100", entry.Lease.IP.String(), "the KV record must name the new address")
assert.Equal(t, 1, mgr.LoopCount())
// A reaffirm RENEW on a lease the server has already aged out only delays
// the re-DORA behind its timeout.
@@ -115,11 +118,12 @@ func TestManagerStartReleasesExpiredLeaseUpstream(t *testing.T) {
require.NoError(t, mgr.Start(context.Background()))
require.Eventually(t, func() bool { return fake.ReleaseCount() == 1 }, time.Second, 10*time.Millisecond,
require.Eventually(t, func() bool {
entry, getErr := store.Get(t.Context(), "eipalloc-stale")
return getErr == nil && entry.Lease.IP.String() == "192.0.2.100"
}, time.Second, 10*time.Millisecond, "the re-acquired address must be persisted")
assert.Equal(t, 1, fake.ReleaseCount(),
"the superseded address must be returned upstream once the re-DORA lands elsewhere")
entry, err := store.Get(t.Context(), "eipalloc-stale")
require.NoError(t, err)
assert.Equal(t, "192.0.2.100", entry.Lease.IP.String())
}
// captureLogs redirects the default logger for the test and returns an accessor
@@ -293,9 +297,10 @@ func TestManagerRenewOntoNewIPReleasesOldAndFiresHook(t *testing.T) {
mu.Unlock()
assert.Equal(t, 1, fake.ReleaseCount(), "the superseded address must be released upstream")
entry, err := store.Get(t.Context(), "dhcp-gw-lrp-vpc-1")
require.NoError(t, err)
assert.Equal(t, newIP.String(), entry.Lease.IP.String(), "KV must hold the new address")
require.Eventually(t, func() bool {
entry, getErr := store.Get(t.Context(), "dhcp-gw-lrp-vpc-1")
return getErr == nil && entry.Lease.IP.String() == newIP.String()
}, time.Second, 10*time.Millisecond, "KV must hold the new address")
}
// The common case: a renewal that keeps its address must not release anything
@@ -335,9 +340,10 @@ func TestManagerRenewOntoNewIPKeepsLeaseWhenHookFails(t *testing.T) {
require.NoError(t, mgr.Start(context.Background()))
require.Eventually(t, func() bool { return called.Load() }, 5*time.Second, 10*time.Millisecond)
entry, err := store.Get(t.Context(), "dhcp-gw-lrp-vpc-1")
require.NoError(t, err)
assert.Equal(t, newIP.String(), entry.Lease.IP.String())
require.Eventually(t, func() bool {
entry, getErr := store.Get(t.Context(), "dhcp-gw-lrp-vpc-1")
return getErr == nil && entry.Lease.IP.String() == newIP.String()
}, time.Second, 10*time.Millisecond)
assert.Equal(t, 1, mgr.LoopCount(), "the lease loop must survive a failed rebind")
}
@@ -353,9 +359,10 @@ func TestManagerRenewOntoNewIPReleasesOldWithoutHook(t *testing.T) {
require.Eventually(t, func() bool { return fake.ReleaseCount() == 1 }, 5*time.Second, 10*time.Millisecond,
"superseded address must be released even with no hook registered")
entry, err := store.Get(t.Context(), "eipalloc-1")
require.NoError(t, err)
assert.Equal(t, "192.0.2.201", entry.Lease.IP.String())
require.Eventually(t, func() bool {
entry, getErr := store.Get(t.Context(), "eipalloc-1")
return getErr == nil && entry.Lease.IP.String() == "192.0.2.201"
}, time.Second, 10*time.Millisecond)
}
// The release of the superseded address is best-effort: failing it must not cost