diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index 50e3471..4e42669 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -481,13 +481,19 @@ func (p *passer) staleAsks(ctx context.Context, trunk map[string]delivery.Module } for key := range p.w.read(delivery.AssignerAsksBucket) { if p.staleAsk(key, trunk) { + // The condition is cleared first and the ask goes only once that was said: the ask is what brings this key back at the + // next pass, so a clear that failed once is made again, with no state of its own (issue 1906, note 29). + k, kerr := delivery.ParseAnyAskKey(key) + held := kerr == nil && (k.Kind == delivery.AskKindHold || k.Kind == delivery.AskKindRestore) + if held && !p.clearHeld(k.Module, k.FP) { + continue + } p.logf("the ask %s is about a fingerprint or composition the trunk has left behind and is taken away", key) p.remove(ctx, delivery.AssignerAsksBucket, key) // The held record stays: it is the memory that this fingerprint failed (ADR 0408 ยง4), and a revert returns the trunk to // exactly it. Only the condition, which would else ask for an answer that can no longer be given, is cleared, and it is // raised again if the trunk returns to the fingerprint. - if k, err := delivery.ParseAnyAskKey(key); err == nil && (k.Kind == delivery.AskKindHold || k.Kind == delivery.AskKindRestore) { - p.clearHeld(k.Module, k.FP) + if held { delete(p.said, assign.HeldCondition(k.Module, k.FP, "", "", false, false).Key+".raised") } } @@ -566,16 +572,18 @@ func (p *passer) answeredWith(key, option string) bool { } // clearHeld clears, once, the condition of a hold the operator released. -func (p *passer) clearHeld(module, fp string) { +// It says whether nothing is left to say: the clear was said now or before, or there is no router to say it to. +func (p *passer) clearHeld(module, fp string) bool { c := assign.HeldCondition(module, fp, "", "", false, false) if p.say == nil || p.said[c.Key+".cleared"] { - return + return true } if err := p.say(c); err != nil { p.logf("the condition of the released %s could not be cleared, and is cleared at the next pass: %v", module, err) - return + return false } p.said[c.Key+".cleared"] = true + return true } // recordAsk writes, once, the ask about a move of a module of policy record that waits for the operator's yes. diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index 537f2e2..2a13988 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -606,6 +606,36 @@ func TestAsksOfAFingerprintTheTrunkLeftBehindAreTakenAway(t *testing.T) { }) } + t.Run("a clear that failed to be said is said at the next pass, and the ask goes after it", func(t *testing.T) { + asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}} + p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now()) + var said []asks.Condition + fail := true + p.say = func(c asks.Condition) error { + if fail { + fail = false + return errors.New("router unreachable") + } + said = append(said, c) + return nil + } + a := assign.HoldAsk(holdOld, "web", fpOld, "novox", "w", 1, time.Now()) + v, err := delivery.EncodeAssignerAsk(holdOld, a) + if err != nil { + t.Fatal(err) + } + asksKV.values[holdOld] = v + p.w.put(delivery.AssignerAsksBucket, holdOld, v) + p.staleAsks(context.Background(), trunkOf(fpNew)) + if _, there := asksKV.values[holdOld]; !there || len(said) != 0 { + t.Fatalf("after a failed clear: ask left = %v, conditions said = %d; want the ask kept and nothing said", there, len(said)) + } + p.staleAsks(context.Background(), trunkOf(fpNew)) + if _, there := asksKV.values[holdOld]; there || len(said) != 1 || said[0].State != asks.StateCleared { + t.Errorf("at the next pass: ask left = %v, conditions said = %v; want the ask gone and the clear said", there, said) + } + }) + t.Run("a lift takes the restore asks of its fingerprint on every node", func(t *testing.T) { asksKV, wantsKV := &memKV{values: map[string][]byte{}}, &memKV{values: map[string][]byte{}} p := newPasser(buckets{delivery.AssignerAsksBucket: asksKV, delivery.AssignerWantsBucket: wantsKV}, time.Now())