fix(orchestrator): finish rejected sandbox lifecycles

GitOrigin-RevId: acee027ae59efd1def5709680fba178199cc9c0c

Author: ValentaTomas

packages/orchestrator/pkg/sandbox/map_test.go

@@ -506,6 +506,77 @@ func TestMapConcurrentMarkStoppingAndStoppedDoesNotResurrectLifecycle(t *testing
 	}
 }
 
+func TestMapMarkRunningRefusesCollidingLifecycle(t *testing.T) {
+	t.Parallel()
+
+	sandboxes := NewSandboxesMap()
+	oldSbx := testMapSandbox(t, "lifecycle-old")
+	newSbx := testMapSandbox(t, "lifecycle-new")
+
+	require.NoError(t, sandboxes.MarkRunning(t.Context(), oldSbx))
+
+	// The old lifecycle is still live (e.g. it crashed and nothing removed it
+	// yet): the new lifecycle must be refused, not silently dropped — a silent
+	// drop leaves its FC process running with no id-based way to reach it.
+	require.ErrorIs(t, sandboxes.MarkRunning(t.Context(), newSbx), ErrSandboxAlreadyRunning)
+
+	live, ok := sandboxes.Get(oldSbx.Runtime.SandboxID)
+	require.True(t, ok)
+	require.Same(t, oldSbx, live)
+	require.Len(t, sandboxes.LifecycleItems(), 1)
+}
+
+func TestMapMarkRunningIdempotentForSameLifecycle(t *testing.T) {
+	t.Parallel()
+
+	sandboxes := NewSandboxesMap()
+	sbx := testMapSandbox(t, "lifecycle-1")
+
+	require.NoError(t, sandboxes.MarkRunning(t.Context(), sbx))
+	require.NoError(t, sandboxes.MarkRunning(t.Context(), sbx))
+	require.Len(t, sandboxes.Items(), 1)
+	require.Len(t, sandboxes.LifecycleItems(), 1)
+}
+
+func TestSandboxCloseClearsLiveEntryWithoutMarkStopping(t *testing.T) {
+	t.Parallel()
+
+	sandboxes := NewSandboxesMap()
+	sbx := testMapSandbox(t, "lifecycle-1")
+	sbx.cleanup = NewCleanup()
+	sbx.sandboxes = sandboxes
+
+	sandboxes.MarkRunning(t.Context(), sbx)
+
+	// A crash ends the lifecycle without any explicit MarkStopping. Close must
+	// clear the live entry too, or the next resume of the same sandbox id will
+	// lose its registration against this dead entry.
+	require.NoError(t, sbx.Close(t.Context()))
+	require.Empty(t, sandboxes.Items())
+	require.Empty(t, sandboxes.LifecycleItems())
+}
+
+func TestSandboxCloseDoesNotRemoveNewerLiveLifecycle(t *testing.T) {
+	t.Parallel()
+
+	sandboxes := NewSandboxesMap()
+	oldSbx := testMapSandbox(t, "lifecycle-old")
+	oldSbx.cleanup = NewCleanup()
+	oldSbx.sandboxes = sandboxes
+	newSbx := testMapSandbox(t, "lifecycle-new")
+
+	sandboxes.MarkRunning(t.Context(), oldSbx)
+	require.True(t, sandboxes.MarkStopping(t.Context(), oldSbx.Runtime.SandboxID, oldSbx.LifecycleID))
+	require.NoError(t, sandboxes.MarkRunning(t.Context(), newSbx))
+
+	// The old lifecycle's Close must not evict the newer live lifecycle.
+	require.NoError(t, oldSbx.Close(t.Context()))
+
+	live, ok := sandboxes.Get(newSbx.Runtime.SandboxID)
+	require.True(t, ok)
+	require.Same(t, newSbx, live)
+}
+
 func testMapSandbox(t *testing.T, lifecycleID string) *Sandbox {
 	t.Helper()
 

packages/orchestrator/pkg/sandbox/sandbox.go

@@ -216,9 +216,10 @@ type StopReason string
 const (
 	// StopReasonKilled covers Delete and the teardowns the orchestrator does
 	// itself after an operation leaves the sandbox unusable.
-	StopReasonKilled        StopReason = "killed"
-	StopReasonPaused        StopReason = "paused"
-	StopReasonCheckpointing StopReason = "checkpointing"
+	StopReasonKilled             StopReason = "killed"
+	StopReasonPaused             StopReason = "paused"
+	StopReasonCheckpointing      StopReason = "checkpointing"
+	StopReasonRegistrationFailed StopReason = "registration_failed"
 	// StopReasonCrashed is the absence of a recorded reason: nothing asked the
 	// sandbox to stop and it went down anyway.
 	StopReasonCrashed StopReason = "crashed"
@@ -1748,6 +1749,8 @@ func (s *Sandbox) Wait(ctx context.Context) error {
 func (s *Sandbox) Close(ctx context.Context) error {
 	err := s.cleanup.Run(ctx)
 	if s.sandboxes != nil {
+		// A guest exit can reach Close without an explicit MarkStopping.
+		s.sandboxes.MarkStopping(context.WithoutCancel(ctx), s.Runtime.SandboxID, s.LifecycleID)
 		s.sandboxes.MarkStopped(context.WithoutCancel(ctx), s)
 	}
 

packages/orchestrator/pkg/server/create_duplicate_test.go

@@ -230,6 +230,7 @@ func TestMarkSandboxLiveRejectsForeignReservationBeforeHealthChecks(t *testing.T
 	t.Cleanup(foreign.Release)
 	sbx := drainTestSandbox(t, "candidate")
 	require.Error(t, s.markSandboxLive(t.Context(), sbx, foreign))
+	require.Equal(t, sandbox.StopReasonRegistrationFailed, sbx.GetStopReason())
 	require.Empty(t, s.sandboxFactory.Sandboxes.Items())
 	require.NoError(t, owner.MarkRunning(t.Context(), sbx))
 }

packages/orchestrator/pkg/server/sandboxes.go

@@ -1665,7 +1665,7 @@ func (s *Server) finishSandboxStart(ctx context.Context, reservation *sandbox.Re
 
 func (s *Server) markSandboxLive(ctx context.Context, sbx *sandbox.Sandbox, reservation *sandbox.Reservation) error {
 	if err := reservation.MarkRunning(ctx, sbx); err != nil {
-		sbx.SetStopReason(sandbox.StopReasonKilled)
+		sbx.SetStopReason(sandbox.StopReasonRegistrationFailed)
 
 		return err
 	}