diff --git a/cmd/mesh-controller/composed_keys.go b/cmd/mesh-controller/composed_keys.go index 6ae5eea9..2b2ac625 100644 --- a/cmd/mesh-controller/composed_keys.go +++ b/cmd/mesh-controller/composed_keys.go @@ -166,10 +166,12 @@ func followDoneKey(ctx context.Context, open *stores, module string) { err := takeOnceInSlot(ctx, open, module) o := doneKeyObservation(module) if err == nil { - if keeper := conditionsFrom; keeper != nil && failed { + // Cleared on every success, whoever raised it: a controller before a restart or another follower (issue + // 1945). A condition that is not open costs one read. + if keeper := conditionsFrom; keeper != nil { _, _ = keeper.Clear(ctx, o.Key(), "composed and granted") - failed = false } + failed = false if _, again := doneKeysLookAgain.LoadAndDelete(module); again { continue // a wake came while this one ran: once more, for the key written meanwhile } diff --git a/cmd/mesh-controller/composed_keys_cancel_test.go b/cmd/mesh-controller/composed_keys_cancel_test.go index 34292364..0e34904c 100644 --- a/cmd/mesh-controller/composed_keys_cancel_test.go +++ b/cmd/mesh-controller/composed_keys_cancel_test.go @@ -59,3 +59,25 @@ func TestATakeInCutByACancellationRaisesNothing(t *testing.T) { }) } } + +// A take-in that succeeds clears the module's fingerprint-not-composed condition whoever raised it: a controller before a +// restart, or another follower (issue 1945: the operator read "a build needs a look" for take-ins that had since worked). +func TestASucceedingTakeInClearsAConditionAnotherRaised(t *testing.T) { + was, wasRetry, wasSay, wasKeeper := doneKeyTakeOnce, doneKeyRetry, doneKeysSay, conditionsFrom + t.Cleanup(func() { doneKeyTakeOnce, doneKeyRetry, doneKeysSay, conditionsFrom = was, wasRetry, wasSay, wasKeeper }) + store := conditions.NewInMemory() + k := conditions.NewKeeper(t.Context(), conditions.Options{Store: store, History: store, Teller: &conditions.Told{}}) + t.Cleanup(func() { k.Close(context.Background()) }) + conditionsFrom = k + doneKeysSay = func(string) {} + o := doneKeyObservation("raised-before") + o.Summary += "the store refused the connection" + if _, err := k.Observe(t.Context(), o); err != nil { + t.Fatal(err) + } + doneKeyTakeOnce = func(context.Context, *stores, string) error { return nil } + takeDoneKey(t.Context(), nil, "raised-before") + if c, found, err := conditions.ReadOne(t.Context(), store, o.Key()); err != nil || found { + t.Fatalf("a take-in that succeeded left the condition another raised: %+v (found %v, %v)", c, found, err) + } +} diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 7d18c351..70c96fc7 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -179,6 +179,7 @@ func serve(ctx context.Context) (err error) { keeper := openServingConditions(ctx, open, bus) if keeper != nil { defer closeConditions(keeper) + defer stopRunning() // on an error return, the goroutines end before the keeper closes } // The bus's own objects, asserted on every start. **Not created once at genesis**: a stream @@ -247,10 +248,12 @@ func serve(ctx context.Context) (err error) { // next test's `conditions` with "connection closed", issue 1945). handActConn = bus.Conn defer func() { handActConn = nil }() + defer stopRunning() // on an error return, the goroutines end before handActConn is put back // And everything else this controller does on the bus for a moment (novox/hq issue 327). servingBus.Store(server.JetStream()) // No longer serving: nothing is lent, and dead-letters says it is not read here (novox/hq issue 330). defer servingBus.Store(nil) + defer stopRunning() // on an error return, the goroutines end before the bus is taken back // And says when it replaced a value given by hand (novox/hq ADR 0228). givenEvents = bus // And a pull request's merge check, asked when the forge announces its head and said when judged