fix(workloadmanager): include sandbox name/namespace in store delete error log

Signed-off-by: Abhinav-kodes <183825080+Abhinav-kodes@users.noreply.github.com>
Signed-off-by: Abhinav Singh <abhinavsingh717073@gmail.com>

Author: Abhinav-kodes

pkg/workloadmanager/handlers.go

@@ -39,6 +39,9 @@ import (
 // errSandboxCreationTimeout is returned when the internal sandbox-ready wait exceeds the 2-minute deadline.
 var errSandboxCreationTimeout = errors.New("sandbox creation timed out")
 
+// storeCleanupTimeout is the maximum duration allowed to clean up a store placeholder.
+const storeCleanupTimeout = 30 * time.Second
+
 // isContextError reports whether err is a context cancellation or deadline error.
 func isContextError(err error) bool {
 	return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
@@ -284,7 +287,7 @@ func (s *Server) createSandbox(ctx context.Context, dynamicClient dynamic.Interf
 // placeholder when creation fails. It runs in a fresh context so that a
 // canceled request context does not prevent cleanup.
 func (s *Server) rollbackSandboxCreation(dynamicClient dynamic.Interface, sandbox *sandboxv1alpha1.Sandbox, sandboxClaim *extensionsv1alpha1.SandboxClaim, sessionID string) {
-	ctxTimeout, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	ctxTimeout, cancel := context.WithTimeout(context.Background(), storeCleanupTimeout)
 	defer cancel()
 	if sandboxClaim != nil {
 		if err := deleteSandboxClaim(ctxTimeout, dynamicClient, sandboxClaim.Namespace, sandboxClaim.Name); err != nil {
@@ -357,13 +360,13 @@ func (s *Server) handleDeleteSandbox(c *gin.Context) {
 
 	// Use a detached context for the store delete so a client disconnect
 	// after K8s deletion doesn't orphan the store entry.
-	deleteCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	deleteCtx, cancel := context.WithTimeout(context.Background(), storeCleanupTimeout)
 	defer cancel()
 
 	// Delete sandbox from store
 	err = s.storeClient.DeleteSandboxBySessionID(deleteCtx, sessionID)
 	if err != nil {
-		klog.Errorf("delete sandbox from store by sessionID %s failed: %v", sessionID, err)
+		klog.Errorf("delete %s %s/%s from store by sessionID %s failed: %v", sandbox.Kind, sandbox.SandboxNamespace, sandbox.Name, sessionID, err)
 		respondError(c, http.StatusInternalServerError, "internal server error")
 		return
 	}

pkg/workloadmanager/handlers_test.go

@@ -475,3 +475,59 @@ func TestHandleSandboxCreate(t *testing.T) {
 		})
 	}
 }
+
+/*
+This test verifies that the deleteSandbox handler correctly handles scenarios where the client disconnects (Context Cancellation) before the deletion operation completes.
+
+Key Points:
+
+The handler creates a new context for the K8s deletion operation using context.WithTimeout(ctx, deletionTimeout).
+This derived context remains valid even if the parent context (c.Request.Context()) is canceled.
+The test simulates a client disconnect by canceling the request context immediately after calling the deleteSandbox function.
+It verifies that the store deletion (the final cleanup step) still occurs by checking that the store's DeleteSandboxBySessionID method was called with a valid, non-canceled context.
+*/
+func TestHandleDeleteSandbox_DetachedContext(t *testing.T) {
+	gin.SetMode(gin.TestMode)
+	fakeServer := newFakeServer()
+
+	fakeStoreInst := &fakeStore{}
+	fakeServer.storeClient = fakeStoreInst
+
+	patches := gomonkey.NewPatches()
+	defer patches.Reset()
+
+	patches.ApplyMethod(reflect.TypeOf((*fakeStore)(nil)), "GetSandboxBySessionID", func(_ *fakeStore, _ context.Context, _ string) (*types.SandboxInfo, error) {
+		return &types.SandboxInfo{
+			Kind:             types.AgentRuntimeKind,
+			SandboxNamespace: "ns-1",
+			Name:             "sandbox-1",
+			SessionID:        "sess-1",
+		}, nil
+	})
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	req := httptest.NewRequest(http.MethodDelete, "/sandboxes/sess-1", nil)
+
+	reqCtx, cancelReq := context.WithCancel(context.Background())
+	req = req.WithContext(reqCtx)
+	c.Request = req
+	c.Params = gin.Params{{Key: "sessionId", Value: "sess-1"}}
+
+	patches.ApplyFunc(deleteSandbox, func(_ context.Context, _ dynamic.Interface, _, _ string) error {
+		cancelReq()
+		return nil
+	})
+
+	storeDeleteCalled := false
+	patches.ApplyMethod(reflect.TypeOf((*fakeStore)(nil)), "DeleteSandboxBySessionID", func(_ *fakeStore, ctx context.Context, _ string) error {
+		require.NoError(t, ctx.Err(), "Store deletion context MUST NOT be canceled despite client disconnect")
+		storeDeleteCalled = true
+		return nil
+	})
+
+	fakeServer.handleDeleteSandbox(c)
+
+	require.True(t, storeDeleteCalled, "DeleteSandboxBySessionID should be called even if the request context is canceled")
+	require.Equal(t, http.StatusOK, w.Code)
+}