From 7f3ff3236149252f1d19fea2bc905ff288ad7975 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 00:08:48 +0200 Subject: [PATCH] Give a fast-renewing test lease a serving controller's time to renew, and prove the minted hash's cost A renewal timed out at the renew interval, so the lease test's 1 s renewals could lose the lease on a slow build machine (novox/hq issue 591, review of #279). --- cmd/mesh-controller/acting.go | 5 +++-- cmd/mesh-controller/lease_test.go | 3 ++- cmd/mesh-controller/plans_follow_test.go | 2 +- internal/inventory/bususers.go | 11 ++++++++--- internal/inventory/bususers_test.go | 8 ++++++-- internal/lease/lease.go | 9 ++++++++- 6 files changed, 28 insertions(+), 10 deletions(-) diff --git a/cmd/mesh-controller/acting.go b/cmd/mesh-controller/acting.go index 46b2bb07..6ca1dbac 100644 --- a/cmd/mesh-controller/acting.go +++ b/cmd/mesh-controller/acting.go @@ -66,7 +66,8 @@ type actor struct { resetSaid string // renewEvery and poll, when set, are how often the serving controller renews the lease it holds and // looks again at one another holds, in place of the lease's own: a test's, so two controllers hand the - // lease over in a second rather than in a renewal of five (novox/hq issue 591). + // lease over in a second rather than in a renewal of five (novox/hq issue 591). A renewal is given + // the lease's own RenewEvery to answer however often it is made, so a slow bus does not lose it. renewEvery, poll time.Duration } @@ -227,7 +228,7 @@ func (a *actor) serveUnderTheLease(ctx context.Context, inv *inventory.Inventory } say := func(format string, args ...any) { fmt.Printf(format+"\n", args...) } l, err := lease.Open(ctx, api, broker.LeaseBucket, lease.Options{Holder: holderOf(instance), - RenewEvery: a.renewEvery, Poll: a.poll, Floor: inv.HighestEpoch, Say: say, Health: controllerHealth, Moved: func(was, floor uint64) { + RenewEvery: a.renewEvery, RenewWithin: lease.RenewEvery, Poll: a.poll, Floor: inv.HighestEpoch, Say: say, Health: controllerHealth, Moved: func(was, floor uint64) { a.mu.Lock() defer a.mu.Unlock() a.reset = time.Now() diff --git a/cmd/mesh-controller/lease_test.go b/cmd/mesh-controller/lease_test.go index 3d715d54..5356a116 100644 --- a/cmd/mesh-controller/lease_test.go +++ b/cmd/mesh-controller/lease_test.go @@ -57,7 +57,8 @@ func TestTwoControllersOneActs(t *testing.T) { // Each renews five times and looks again ten times as often as a serving controller does, so the // handover and the loss below are seen in a second, not in a renewal of five (novox/hq issue 591): what - // is checked is the order of events, which does not change with the interval. + // is checked is the order of events, which does not change with the interval. Each renewal still has a + // serving controller's five seconds to answer (acting.go), so a slow bus does not lose A the lease. const renewEvery, poll = lease.RenewEvery / 5, lease.Poll / 10 // Controller A takes the lease. diff --git a/cmd/mesh-controller/plans_follow_test.go b/cmd/mesh-controller/plans_follow_test.go index cc8e0db0..0f25870d 100644 --- a/cmd/mesh-controller/plans_follow_test.go +++ b/cmd/mesh-controller/plans_follow_test.go @@ -175,7 +175,7 @@ func TestAPlanFollowedFromTheStoreAnswersWhenItMoves(t *testing.T) { if got.Came != "tier" || got.Plan.Tier != 1 || got.Plan.State != inventory.PlanRolling { t.Fatalf("the wait answered %+v", got) } - if waited := time.Since(began); waited > 10*time.Second { + if waited := time.Since(began); waited > 3*time.Second { t.Errorf("it answered after %s, not when the plan moved", waited) } out = printed(t, func() error { diff --git a/internal/inventory/bususers.go b/internal/inventory/bususers.go index e5ef9dc4..73ccefec 100644 --- a/internal/inventory/bususers.go +++ b/internal/inventory/bususers.go @@ -38,6 +38,11 @@ func busPasswordCost() int { return bcrypt.DefaultCost } +// hashBusPassword is the hash a bus password is kept as, minted, recorded or seeded alike. +func hashBusPassword(password string) ([]byte, error) { + return bcrypt.GenerateFromPassword([]byte(password), busPasswordCost()) +} + // BusUser is one user of the bus, as the mesh records it. type BusUser struct { Username string @@ -82,7 +87,7 @@ func (i *Inventory) RecordBusPassword(ctx context.Context, u BusUser, password s if password == "" { return errors.New("a bus user needs a password") } - hash, err := bcrypt.GenerateFromPassword([]byte(password), busPasswordCost()) + hash, err := hashBusPassword(password) if err != nil { return fmt.Errorf("cannot hash a bus password: %w", err) } @@ -116,7 +121,7 @@ func (i *Inventory) MintBusPassword(ctx context.Context, u BusUser) (string, err // The cost the server will pay on every connection. Left at the library's default rather than // raised: a node reconnecting after a network blip pays it, and the mesh's own links reconnect // far more often than a person logs in anywhere. - hash, err := bcrypt.GenerateFromPassword([]byte(password), busPasswordCost()) + hash, err := hashBusPassword(password) if err != nil { return "", fmt.Errorf("cannot hash a bus password: %w", err) } @@ -198,7 +203,7 @@ func (i *Inventory) SeedBusUser(ctx context.Context, u BusUser, password string) if u.Username == "" || u.Kind == "" || password == "" { return errors.New("a bus user needs a username, a kind and the credential it is using") } - hash, err := bcrypt.GenerateFromPassword([]byte(password), busPasswordCost()) + hash, err := hashBusPassword(password) if err != nil { return fmt.Errorf("cannot hash a bus password: %w", err) } diff --git a/internal/inventory/bususers_test.go b/internal/inventory/bususers_test.go index f330f93c..401c6220 100644 --- a/internal/inventory/bususers_test.go +++ b/internal/inventory/bususers_test.go @@ -164,7 +164,11 @@ func TestABusPasswordIsHashedAtTheDefaultCostOutsideATest(t *testing.T) { was := busPasswordCostForTest.Load() t.Cleanup(func() { busPasswordCostForTest.Store(was) }) busPasswordCostForTest.Store(0) - if got := busPasswordCost(); got != bcrypt.DefaultCost { - t.Fatalf("a bus password is hashed at cost %d, not bcrypt's default %d", got, bcrypt.DefaultCost) + hash, err := hashBusPassword("a password") + if err != nil { + t.Fatal(err) + } + if got, err := bcrypt.Cost(hash); err != nil || got != bcrypt.DefaultCost { + t.Fatalf("a bus password is hashed at cost %d (%v), not bcrypt's default %d", got, err, bcrypt.DefaultCost) } } diff --git a/internal/lease/lease.go b/internal/lease/lease.go index a3050947..28d561ec 100644 --- a/internal/lease/lease.go +++ b/internal/lease/lease.go @@ -75,6 +75,10 @@ type Options struct { Holder Holder // RenewEvery, Margin and Poll default to the package's. RenewEvery, Margin, Poll time.Duration + // RenewWithin is how long one renewal may take before it counts as failed: RenewEvery when zero. Apart + // from it so a test that renews often still gives a slow bus a serving controller's time to answer + // (novox/hq issue 591). + RenewWithin time.Duration // Floor is the highest epoch this mesh has issued, from the controller's own store. **An epoch is // never issued twice**: a lease bucket raised again from nothing — a bus whose data was replaced — // starts its revisions over, and every machine that heard epoch 57 would refuse the next controller @@ -138,6 +142,9 @@ func Open(ctx context.Context, js jetstream.JetStream, bucket string, o Options) if o.RenewEvery <= 0 { o.RenewEvery = RenewEvery } + if o.RenewWithin <= 0 { + o.RenewWithin = o.RenewEvery + } if o.Margin <= 0 { o.Margin = Margin } @@ -406,7 +413,7 @@ func (l *Lease) Renew(ctx context.Context) error { if err != nil { return l.lose(err) } - renewing, cancel := context.WithTimeout(ctx, l.o.RenewEvery) + renewing, cancel := context.WithTimeout(ctx, l.o.RenewWithin) defer cancel() next, err := l.kv.Update(renewing, Key, value, revision) if err != nil {