diff --git a/internal/link/enrolment.go b/internal/link/enrolment.go index 52a185d..f39bc52 100644 --- a/internal/link/enrolment.go +++ b/internal/link/enrolment.go @@ -45,7 +45,8 @@ func (e Enrolment) Enrol(ctx context.Context, request EnrolRequest) (reply Enrol // A store that could not be asked right now, or a token another presenter holds for the // moment, is "not now": the node asks again with the same request (novox/hq issue 083). defer func() { - if inventory.Unreachable(err) || errors.Is(err, inventory.ErrTokenInUse) { + if inventory.Unreachable(err) || errors.Is(err, inventory.ErrTokenInUse) || + errors.Is(err, context.Canceled) { err = fmt.Errorf("%w: %w", ErrTryAgain, err) } }() diff --git a/internal/link/serve.go b/internal/link/serve.go index f3aa49e..3bc2d76 100644 --- a/internal/link/serve.go +++ b/internal/link/serve.go @@ -503,6 +503,13 @@ func (s *Server) handleEnrol(ctx context.Context, delivery amqp.Delivery) { // this answer — decides when to ask, and the queue behind it moves (issue 083). reply = EnrolReply{TryAgain: true, Refusal: "the mesh cannot answer right now; ask again"} s.log.Printf("asked %q to enrol again shortly: %v", request.Node, err) + case err != nil && request.Redelivered: + // Said as what it most likely is: the broker handed this request over again after + // the control plane stopped mid-answer, and an enrolment already spent is not + // finished a second time. The node may need a new token. + s.log.Printf("refusing a redelivered enrolment for %q — it may have finished before "+ + "the control plane stopped, and if the node did not get its answer it needs a new "+ + "token: %v", request.Node, err) case err != nil: // Logged in full here, where an operator can see it; sent back as one refusal, so // that somebody guessing learns nothing from which reason came back. @@ -663,6 +670,11 @@ func (s *Server) upgraded(ctx context.Context, delivery amqp.Delivery) { return } if err := s.upgrader.Upgraded(ctx, u); err != nil { + // Shutting down is not an answer about the announcement: left for the broker. + if ctx.Err() != nil { + holding = true + return + } if errors.Is(err, ErrTryAgain) && s.tryLater(ctx, delivery, subject, fmt.Sprintf("%s's move to %s", u.Module, short(u.Commit)), err, s.upgraded) { holding = true diff --git a/internal/link/store_window_test.go b/internal/link/store_window_test.go index 931eeaf..f0fcd2d 100644 --- a/internal/link/store_window_test.go +++ b/internal/link/store_window_test.go @@ -165,3 +165,17 @@ func TestWhatIsHeldLeavesRoomInThePrefetch(t *testing.T) { t.Fatalf("a message past the ceiling was held: %+v", *last) } } + +// An upgrade handled during shutdown is left for the broker too — the upgrader's error is the +// cancelled context, which is no answer about the announcement. +func TestAnUpgradeHandledDuringShutdownIsLeftForTheBroker(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + s := quietServer() + s.upgrader = upgradesWith{err: context.Canceled} + to := &settledAs{} + s.upgraded(ctx, a(t, to, "upgraded", Upgraded{Module: "gitea", Commit: "abcdef0123"})) + if !to.held() { + t.Fatalf("an upgrade handled during shutdown was settled, and so lost: %+v", *to) + } +}