Give organization deletion time for a busy tenant loop to stop

244d322dce made organization deletion refuse, and keep the data, when
the tenant's monitoring loop has not exited. That is the right guard,
but it reused tenantMonitorShutdownTimeout, a two-second budget that
had only ever been a best-effort wait before logging and carrying on.
A loop that is still starting up or finishing a tick can take longer
to see its cancelled context. In E2E run 37329726585 a just-used
organization's loop took 8-10 seconds, so both attempts of
13-api-token-scope's org cleanup got 503 tenant_shutdown_incomplete.
Any real delete shortly after a tenant was busy hits the same refusal
until a retry.

Deletion now waits on its own tenantDeletionShutdownTimeout (30s)
before it refuses. Process shutdown and RemoveTenant keep the
two-second budget, and a loop that never exits still refuses and
retains the data. Both budgets are variables so tests can shorten
them; the never-stopping-loop test no longer waits on the real budget.

docs/release-control/v6/internal/subsystems/monitoring.md

@@ -1403,6 +1403,14 @@ reporting source of truth. `MultiTenantMonitor.ListOrganizationIDs` may expose
 persisted organization IDs to API-owned background workers, but it must not
 initialize monitors, start pollers, or reinterpret tenant IDs as monitored
 resource health.
+Organization deletion waits up to `tenantDeletionShutdownTimeout` (30 seconds)
+for the tenant monitoring loop to exit before it refuses with
+`tenant_shutdown_incomplete` and keeps the data. Process shutdown and
+`RemoveTenant` keep the two-second `tenantMonitorShutdownTimeout`. Deletion
+gates the data on that wait, so the budget must cover a loop that is still
+starting up or finishing a tick: under CI load a just-used organization's loop
+took 8-10 seconds to see its cancelled context, and the two-second budget turned
+an ordinary delete into a refusal. A loop that never exits still refuses.
 Proxmox physical-disk polling is also a continuity boundary. A failed or
 permission-denied `disks/list` call must remain an error so the monitor can use
 linked host-agent inventory or retain same-instance, same-node prior evidence;

internal/monitoring/guest_metadata_shutdown_test.go

@@ -79,6 +79,7 @@ func TestTenantDeletionRetainsBlockedGuestWriterOwnership(t *testing.T) {
 	}
 }
 func TestTenantDeletionRetainsUnfinishedMonitorLoop(t *testing.T) {
+	shortenTenantShutdownBudgets(t, 50*time.Millisecond, 100*time.Millisecond)
 	mtm := NewMultiTenantMonitor(&config.Config{}, nil, nil)
 	_, cancel := context.WithCancel(context.Background())
 	done := make(chan struct{})

internal/monitoring/monitor_additional_test.go

@@ -1102,3 +1102,57 @@ func TestAutomaticGuestMemoryLinkNextPoll(t *testing.T) {
 		t.Fatalf("ambiguous hostname selected %q/%q/%q", node, linkedVM, ct)
 	}
 }
+
+// shortenTenantShutdownBudgets makes the shutdown waits fast enough for tests
+// while keeping deletion's budget longer than the plain one, as in production.
+func shortenTenantShutdownBudgets(t *testing.T, plain, deletion time.Duration) {
+	t.Helper()
+	previousPlain, previousDeletion := tenantMonitorShutdownTimeout, tenantDeletionShutdownTimeout
+	tenantMonitorShutdownTimeout, tenantDeletionShutdownTimeout = plain, deletion
+	t.Cleanup(func() {
+		tenantMonitorShutdownTimeout, tenantDeletionShutdownTimeout = previousPlain, previousDeletion
+	})
+}
+
+// A tenant monitor that is still starting up or finishing a tick can take
+// longer than the plain shutdown budget to see its cancelled context. In CI a
+// just-used organization's loop took 8-10s, so deletion refused with
+// tenant_shutdown_incomplete. Deletion waits on its own, longer budget instead.
+func TestTenantDeletionWaitsForSlowMonitoringLoop(t *testing.T) {
+	shortenTenantShutdownBudgets(t, 50*time.Millisecond, 2*time.Second)
+
+	slowLoop := func(mtm *MultiTenantMonitor, orgID string) *Monitor {
+		ctx, cancel := context.WithCancel(context.Background())
+		done := make(chan struct{})
+		go func() {
+			<-ctx.Done()
+			// Exits well after the plain budget but inside deletion's.
+			time.Sleep(300 * time.Millisecond)
+			close(done)
+		}()
+		monitor := &Monitor{}
+		mtm.monitors[orgID] = monitor
+		mtm.tenantCancel[orgID] = cancel
+		mtm.tenantDone[orgID] = done
+		return monitor
+	}
+
+	mtm := NewMultiTenantMonitor(&config.Config{}, nil, nil)
+	t.Cleanup(mtm.Stop)
+
+	// RemoveTenant keeps the plain budget, gives up and retains the owner.
+	retained := slowLoop(mtm, "removal")
+	mtm.RemoveTenant("removal")
+	if got, ok := mtm.PeekMonitor("removal"); !ok || got != retained {
+		t.Fatal("plain removal discarded a monitor whose loop had not stopped")
+	}
+
+	slowLoop(mtm, "deletion")
+	if err := mtm.BeginTenantDeletion("deletion"); err != nil {
+		t.Fatalf("deletion refused a loop that stopped inside its budget: %v", err)
+	}
+	if _, ok := mtm.PeekMonitor("deletion"); ok {
+		t.Fatal("completed deletion left the monitor registered")
+	}
+	mtm.FinishTenantDeletion("deletion")
+}

internal/monitoring/multi_tenant_monitor.go

@@ -47,7 +47,19 @@ func NewMultiTenantMonitor(baseCfg *config.Config, persistence *config.MultiTena
 	}
 }
 
-const tenantMonitorShutdownTimeout = 2 * time.Second
+// Both shutdown budgets are variables so tests can shorten them.
+var (
+	// tenantMonitorShutdownTimeout bounds how long process shutdown and
+	// RemoveTenant wait for a tenant's monitoring loop to exit.
+	tenantMonitorShutdownTimeout = 2 * time.Second
+
+	// tenantDeletionShutdownTimeout bounds how long organization deletion
+	// waits for the loop. Deletion refuses, and keeps the data, while the loop
+	// still runs, so this is a correctness gate rather than a best-effort
+	// wait: a loop that is still starting up or finishing a tick can take
+	// several seconds to see its cancelled context.
+	tenantDeletionShutdownTimeout = 30 * time.Second
+)
 
 // SetRecoveryManager wires a recovery store manager into all existing and future tenant monitors.
 func (mtm *MultiTenantMonitor) SetRecoveryManager(manager *recoverymanager.Manager) {
@@ -377,7 +389,11 @@ func (mtm *MultiTenantMonitor) removeTenant(orgID string, deleting bool) error {
 	if cancel != nil {
 		cancel()
 	}
-	if !waitForTenantMonitorShutdown(orgID, done) {
+	shutdownTimeout := tenantMonitorShutdownTimeout
+	if deleting {
+		shutdownTimeout = tenantDeletionShutdownTimeout
+	}
+	if !waitForTenantMonitorShutdown(orgID, done, shutdownTimeout) {
 		return fmt.Errorf("organization %q monitoring loop has not stopped", orgID)
 	}
 	if monitor != nil {
@@ -397,19 +413,19 @@ func (mtm *MultiTenantMonitor) removeTenant(orgID string, deleting bool) error {
 	return nil
 }
 
-func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}) bool {
+func waitForTenantMonitorShutdown(orgID string, done <-chan struct{}, timeout time.Duration) bool {
 	if done == nil {
 		return true
 	}
 
-	timer := time.NewTimer(tenantMonitorShutdownTimeout)
+	timer := time.NewTimer(timeout)
 	defer timer.Stop()
 
 	select {
 	case <-done:
 		return true
 	case <-timer.C:
-		logger := log.Warn().Dur("timeout", tenantMonitorShutdownTimeout)
+		logger := log.Warn().Dur("timeout", timeout)
 		if orgID != "" {
 			logger = logger.Str("org_id", orgID)
 		}