Review of 083: an upgrade handled during shutdown is left for the broker; an enrolment cut short by shutdown is told to try again; a refused redelivery says what it probably is

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)
 		}
 	}()

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

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)
+	}
+}