From e51a24227a58406ea194884f8eaaf555a12aa0c3 Mon Sep 17 00:00:00 2001 From: Sarthak Date: Sat, 26 Sep 2026 16:20:44 +0530 Subject: [PATCH] fix: implement LeaderElectionRunnable for CostAccrual (#95) Signed-off-by: Sarthak --- cmd/main.go | 7 ++----- internal/controller/cost_accrual.go | 18 ++++++++++++++---- internal/controller/cost_accrual_test.go | 6 ++++++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index ed42e5f..fd7cea6 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -462,11 +462,8 @@ func setupControllers(mgr ctrl.Manager, blocklist *failover.Blocklist, kubeletSr } } - // Cost accrual is a clock-driven loop, not a reconciler, so it is added directly. It opts into - // leader election by NOT implementing LeaderElectionRunnable, which buys two things: N replicas - // would mean N times the writes, and each would book windows into its OWN cost counter, leaving - // increase() to read a series that holds only the races that replica won. The ledger itself is - // safe either way (see CostAccrual.accrue). + // Cost accrual is a clock-driven loop, not a reconciler, so it is added directly. It runs on + // the leader only (see CostAccrual.NeedLeaderElection). if err := mgr.Add(controller.NewCostAccrual(mgr.GetClient())); err != nil { return fmt.Errorf("unable to add the cost accrual loop: %w", err) } diff --git a/internal/controller/cost_accrual.go b/internal/controller/cost_accrual.go index 1158812..9e894b9 100644 --- a/internal/controller/cost_accrual.go +++ b/internal/controller/cost_accrual.go @@ -70,9 +70,8 @@ const accrualWorkers = 16 // CostAccrual advances each claim's durable spend ledger on a ticker. // // A Runnable rather than a hook on the reconcile path: spend accrues with the CLOCK, not with -// events, and a Bound claim can sit for hours without a reconcile. Leader election is the -// manager's default for a plain Runnable and is load-bearing here — two replicas accruing the -// same fleet would double every dollar. +// events, and a Bound claim can sit for hours without a reconcile. Leader election is +// load-bearing here (see NeedLeaderElection). // // It lives beside the reconciler rather than in pkg/metrics because it WRITES: the ledger is a // status field, and instrumentation that patches API objects is no longer instrumentation. @@ -83,7 +82,18 @@ type CostAccrual struct { now func() time.Time } -var _ manager.Runnable = (*CostAccrual)(nil) +var ( + _ manager.Runnable = (*CostAccrual)(nil) + _ manager.LeaderElectionRunnable = (*CostAccrual)(nil) +) + +// NeedLeaderElection pins the loop to the leader. It matches the manager's default for a plain +// Runnable, but is stated explicitly so a change to that default cannot silently let N replicas +// accrue the same fleet: N times the writes, and each replica's cost counter would hold only the +// windows it won the race for. The ledger itself is safe either way (see CostAccrual.accrue). +func (a *CostAccrual) NeedLeaderElection() bool { + return true +} // NewCostAccrual builds the accrual loop over the manager's client: cached reads, direct writes. func NewCostAccrual(c client.Client) *CostAccrual { diff --git a/internal/controller/cost_accrual_test.go b/internal/controller/cost_accrual_test.go index 108b827..b619c07 100644 --- a/internal/controller/cost_accrual_test.go +++ b/internal/controller/cost_accrual_test.go @@ -1045,3 +1045,9 @@ func TestCostAccrual_StartAccrues(t *testing.T) { } t.Fatal("Start persisted nothing after 5s of 1ms ticks") } + +func TestCostAccrualNeedsLeaderElection(t *testing.T) { + if !NewCostAccrual(nil).NeedLeaderElection() { + t.Fatal("cost accrual must run on the leader only") + } +}