DRA: block a dedicated allocation over a device's live persisted share

Project: kubernetes/kubernetes | Language: Go | Concepts: consistency-models, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/kubernetes/kubernetes/pull/140798

The problem

The Kubernetes Dynamic Resource Allocation (DRA) allocator could incorrectly grant exclusive (dedicated) access to a device that still has a live, persisted shared allocation. This could happen if a device's allowMultipleAllocations property is changed from true to false while a shared allocation is still active. The allocator's existing checks did not account for persisted shared allocations stored in AllocatedSharedDeviceIDs, allowing a dedicated request to "steal" a device that should be unavailable.

Why it's non-trivial

A naive fix might simply check for any allocation, but that would also block legitimate further shared allocations on devices that are still intended to allow multiple allocations. The logic must be careful to only block dedicated requests on devices that have been switched to exclusive mode but still have active shared allocations, without breaking shared allocation workflows or admin access exemptions.

The approach

The fix adds a targeted guard in the allocation logic for both the experimental and incubating allocators. It also introduces new test cases to verify the behavior.

Key code changes:

1. Guard in the allocator to block dedicated allocation over a live share:

In both allocator_experimental.go and allocator_incubating.go:

+	// A non-allow-multiple device can still carry a live share after an
+	// allowMultipleAllocations true->false update, which deviceInUse above does not see.
+	if !request.adminAccess() && !allowMultipleAllocations && alloc.deviceCapacityInUse(device.id) {
+		alloc.logger.V(7).Info("Device has an active shared allocation, cannot be allocated exclusively", "device", device.id)
+		return false, nil, nil
+	}

What it does:

• Checks if the request is not for admin access, the device does not allow multiple allocations, and there is still capacity in use (i.e., a live share).
• If all are true, the device is rejected for exclusive allocation.
• This prevents a dedicated request from being granted on top of a live shared allocation after the device's sharing policy changes.

2. New and updated test cases:

In allocator_testing.go, three new cases are added to cover the edge conditions:

+		"consumable-capacity-dedicated-request-blocked-by-persisted-share": {
+			// device1 still carries a live share after allowMultipleAllocations flipped to false,
+			// so a dedicated request must not be handed the device on top of it.
+			...
+			expectResults: []any{},
+		},
+		"consumable-capacity-dedicated-request-falls-back-past-persisted-share": {
+			// The guard drops device1 without aborting the search, so the request still lands on device2.
+			...
+			expectResults: []any{allocationResult(
+				localNodeSelector(node1),
+				deviceAllocationResult(req0, driverA, pool1, device2, false),
+			)},
+		},
+		"consumable-capacity-with-admin-access-request-allowed-over-persisted-share": {
+			// Admin access skips the availability checks, so the live share does not withhold device1.
+			...
+			expectResults: []any{allocationResult(
+				localNodeSelector(node1),
+				deviceAllocationResult(req0, driverA, pool1, device1, true),
+			)},
+		},

What they do:

• The first test ensures a dedicated request is blocked if a live share remains.
• The second ensures that if one device is blocked, the allocator can still allocate another available device.
• The third confirms that admin access can still allocate the device, even if a share is active.

From the review thread

All three cases now carry the prefix:

• consumable-capacity-dedicated-request-blocked-by-persisted-share
• consumable-capacity-dedicated-request-falls-back-past-persisted-share
• consumable-capacity-with-admin-access-request-allowed-over-persisted-share

I prefixed the middle one too so the three sit together. For the admin-access case I read your prefix as the consumable-capacity-with-<feature> shape that consumable-capacity-with-partitionable-device-... already uses, so it replaced the leading admin-access- rather than sitting in front of it. Say the word if you meant it literally.

While I was there I also trimmed the comments, on those three cases and on the guard. They were 5, 4 and 3 lines while almost every case in this file has none, so they now give just the reason and don't name another case.

Takeaways

• Always account for persisted state when enforcing exclusivity or sharing semantics, especially if configuration can change dynamically.
• Avoid over-broad checks: ensure that fixes do not block legitimate workflows (e.g., further shared allocations or admin access) by targeting only the problematic cases.
• Add regression tests that specifically exercise subtle state transitions and edge cases, especially after bug fixes involving concurrency or resource accounting.