[KEP-4817] Drop deallocated devices from resourceclaim.status.devices

Signed-off-by: Lionel Jouin <lionel.jouin@est.tech>

pkg/registry/resource/resourceclaim/strategy.go

@@ -24,11 +24,13 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/registry/generic"
 	"k8s.io/apiserver/pkg/storage"
 	"k8s.io/apiserver/pkg/storage/names"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/dynamic-resource-allocation/structured"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/resource"
 	"k8s.io/kubernetes/pkg/apis/resource/validation"
@@ -140,6 +142,7 @@ func (resourceclaimStatusStrategy) PrepareForUpdate(ctx context.Context, obj, ol
 	newClaim.Spec = oldClaim.Spec
 	metav1.ResetObjectMetaForStatus(&newClaim.ObjectMeta, &oldClaim.ObjectMeta)
 
+	dropDeallocatedStatusDevices(newClaim, oldClaim)
 	dropDisabledFields(newClaim, oldClaim)
 }
 
@@ -238,3 +241,47 @@ func dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim *resource
 		newClaim.Status.Devices = nil
 	}
 }
+
+// dropDeallocatedStatusDevices removes the status.devices that were allocated
+// in the oldClaim and that have been removed in the newClaim.
+func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) {
+		return
+	}
+
+	deallocatedDevices := sets.New[structured.DeviceID]()
+
+	if oldClaim.Status.Allocation != nil {
+		// Get all devices in the oldClaim.
+		for _, result := range oldClaim.Status.Allocation.Devices.Results {
+			deviceID := structured.DeviceID{Driver: result.Driver, Pool: result.Pool, Device: result.Device}
+			deallocatedDevices.Insert(deviceID)
+		}
+	}
+
+	// Remove devices from deallocatedDevices that are still in newClaim.
+	if newClaim.Status.Allocation != nil {
+		for _, result := range newClaim.Status.Allocation.Devices.Results {
+			deviceID := structured.DeviceID{Driver: result.Driver, Pool: result.Pool, Device: result.Device}
+			deallocatedDevices.Delete(deviceID)
+		}
+	}
+
+	// Remove from newClaim.Status.Devices.
+	for i := len(newClaim.Status.Devices) - 1; i >= 0; i-- {
+		deviceID := structured.DeviceID{
+			Driver: newClaim.Status.Devices[i].Driver,
+			Pool:   newClaim.Status.Devices[i].Pool,
+			Device: newClaim.Status.Devices[i].Device,
+		}
+		// Device was in the oldClaim.Status.Allocation.Devices but is no longer in the
+		// newClaim.Status.Allocation.Devices so it must be removed from the newClaim.Status.Devices.
+		if deallocatedDevices.Has(deviceID) {
+			newClaim.Status.Devices = append(newClaim.Status.Devices[:i], newClaim.Status.Devices[i+1:]...)
+		}
+	}
+
+	if len(newClaim.Status.Devices) == 0 {
+		newClaim.Status.Devices = nil
+	}
+}

pkg/registry/resource/resourceclaim/strategy_test.go

@@ -133,78 +133,10 @@ var objWithAdminAccessStatus = &resource.ResourceClaim{
 	},
 }
 
-var objWithRequestAndStatus = &resource.ResourceClaim{
+var testRequest = "test-request"
-	ObjectMeta: metav1.ObjectMeta{
+var testDriver = "test-driver"
-		Name:      "valid-claim",
+var testPool = "test-pool"
-		Namespace: "default",
+var testDevice = "test-device"
-	},
-	Spec: resource.ResourceClaimSpec{
-		Devices: resource.DeviceClaim{
-			Requests: []resource.DeviceRequest{
-				{
-					Name:            "test-request",
-					DeviceClassName: "test-device-class",
-					AllocationMode:  resource.DeviceAllocationModeExactCount,
-					Count:           1,
-				},
-			},
-		},
-	},
-	Status: resource.ResourceClaimStatus{
-		Allocation: &resource.AllocationResult{
-			Devices: resource.DeviceAllocationResult{
-				Results: []resource.DeviceRequestAllocationResult{
-					{
-						Request: "test-request",
-						Driver:  "test-driver",
-						Pool:    "test-pool",
-						Device:  "test-device",
-					},
-				},
-			},
-		},
-	},
-}
-
-var objWithGatedStatusFields = &resource.ResourceClaim{
-	ObjectMeta: metav1.ObjectMeta{
-		Name:      "valid-claim",
-		Namespace: "default",
-	},
-	Spec: resource.ResourceClaimSpec{
-		Devices: resource.DeviceClaim{
-			Requests: []resource.DeviceRequest{
-				{
-					Name:            "test-request",
-					DeviceClassName: "test-device-class",
-					AllocationMode:  resource.DeviceAllocationModeExactCount,
-					Count:           1,
-				},
-			},
-		},
-	},
-	Status: resource.ResourceClaimStatus{
-		Allocation: &resource.AllocationResult{
-			Devices: resource.DeviceAllocationResult{
-				Results: []resource.DeviceRequestAllocationResult{
-					{
-						Request: "test-request",
-						Driver:  "test-driver",
-						Pool:    "test-pool",
-						Device:  "test-device",
-					},
-				},
-			},
-		},
-		Devices: []resource.AllocatedDeviceStatus{
-			{
-				Driver: "test-driver",
-				Pool:   "test-pool",
-				Device: "test-device",
-			},
-		},
-	},
-}
 
 func TestStrategy(t *testing.T) {
 	if !Strategy.NamespaceScoped() {
@@ -422,20 +354,69 @@ func TestStatusStrategyUpdate(t *testing.T) {
 			}(),
 		},
 		"drop-fields-devices-status": {
-			oldObj:                  objWithRequestAndStatus,
+			oldObj: func() *resource.ResourceClaim {
-			newObj:                  objWithGatedStatusFields,
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
+				return obj
+			}(),
+			newObj: func() *resource.ResourceClaim { // Status is added
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
+				addStatusDevices(obj, testDriver, testPool, testDevice)
+				return obj
+			}(),
 			deviceStatusFeatureGate: false,
-			expectObj:               objWithRequestAndStatus,
+			expectObj: func() *resource.ResourceClaim { // Status is no longer there
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
+				return obj
+			}(),
 		},
 		"keep-fields-devices-status": {
-			oldObj:                  objWithRequestAndStatus,
+			oldObj: func() *resource.ResourceClaim {
-			newObj:                  objWithGatedStatusFields,
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
+				return obj
+			}(),
+			newObj: func() *resource.ResourceClaim { // Status is added
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
+				addStatusDevices(obj, testDriver, testPool, testDevice)
+				return obj
+			}(),
 			deviceStatusFeatureGate: true,
-			expectObj: func() *resource.ResourceClaim {
+			expectObj: func() *resource.ResourceClaim { // Status is still there
-				expectObj := objWithGatedStatusFields.DeepCopy()
+				obj := obj.DeepCopy()
-				// Spec remains unchanged.
+				addSpecDevicesRequest(obj, testRequest)
-				expectObj.Spec = objWithRequestAndStatus.Spec
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
-				return expectObj
+				addStatusDevices(obj, testDriver, testPool, testDevice)
+				return obj
+			}(),
+		},
+		"drop-status-deallocated-device": {
+			oldObj: func() *resource.ResourceClaim {
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
+				addStatusDevices(obj, testDriver, testPool, testDevice)
+				return obj
+			}(),
+			newObj: func() *resource.ResourceClaim { // device is deallocated
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				addStatusDevices(obj, testDriver, testPool, testDevice)
+				return obj
+			}(),
+			deviceStatusFeatureGate: true,
+			expectObj: func() *resource.ResourceClaim { // Status is no longer there
+				obj := obj.DeepCopy()
+				addSpecDevicesRequest(obj, testRequest)
+				return obj
 			}(),
 		},
 	}
@@ -469,3 +450,29 @@ func TestStatusStrategyUpdate(t *testing.T) {
 		})
 	}
 }
+
+func addSpecDevicesRequest(resourceClaim *resource.ResourceClaim, request string) {
+	resourceClaim.Spec.Devices.Requests = append(resourceClaim.Spec.Devices.Requests, resource.DeviceRequest{
+		Name: request,
+	})
+}
+
+func addStatusAllocationDevicesResults(resourceClaim *resource.ResourceClaim, driver string, pool string, device string, request string) {
+	if resourceClaim.Status.Allocation == nil {
+		resourceClaim.Status.Allocation = &resource.AllocationResult{}
+	}
+	resourceClaim.Status.Allocation.Devices.Results = append(resourceClaim.Status.Allocation.Devices.Results, resource.DeviceRequestAllocationResult{
+		Request: request,
+		Driver:  driver,
+		Pool:    pool,
+		Device:  device,
+	})
+}
+
+func addStatusDevices(resourceClaim *resource.ResourceClaim, driver string, pool string, device string) {
+	resourceClaim.Status.Devices = append(resourceClaim.Status.Devices, resource.AllocatedDeviceStatus{
+		Driver: driver,
+		Pool:   pool,
+		Device: device,
+	})
+}