From c59359289f190b138efa04314a2739400a39b3ae Mon Sep 17 00:00:00 2001 From: Lionel Jouin Date: Tue, 22 Oct 2024 20:11:36 +0200 Subject: [PATCH] [KEP-4817] Drop deallocated devices from resourceclaim.status.devices Signed-off-by: Lionel Jouin --- .../resource/resourceclaim/strategy.go | 47 +++++ .../resource/resourceclaim/strategy_test.go | 171 +++++++++--------- 2 files changed, 136 insertions(+), 82 deletions(-) diff --git a/pkg/registry/resource/resourceclaim/strategy.go b/pkg/registry/resource/resourceclaim/strategy.go index e8b783f4c7d..6aa41f5bcfc 100644 --- a/pkg/registry/resource/resourceclaim/strategy.go +++ b/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 + } +} diff --git a/pkg/registry/resource/resourceclaim/strategy_test.go b/pkg/registry/resource/resourceclaim/strategy_test.go index 96741ad79fd..fe227f7eaf7 100644 --- a/pkg/registry/resource/resourceclaim/strategy_test.go +++ b/pkg/registry/resource/resourceclaim/strategy_test.go @@ -133,78 +133,10 @@ var objWithAdminAccessStatus = &resource.ResourceClaim{ }, } -var objWithRequestAndStatus = &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", - }, - }, - }, - }, - }, -} - -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", - }, - }, - }, -} +var testRequest = "test-request" +var testDriver = "test-driver" +var testPool = "test-pool" +var testDevice = "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, - newObj: objWithGatedStatusFields, + oldObj: func() *resource.ResourceClaim { + 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, - newObj: objWithGatedStatusFields, + oldObj: func() *resource.ResourceClaim { + 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 := objWithGatedStatusFields.DeepCopy() - // Spec remains unchanged. - expectObj.Spec = objWithRequestAndStatus.Spec - return expectObj + expectObj: func() *resource.ResourceClaim { // Status is still there + obj := obj.DeepCopy() + addSpecDevicesRequest(obj, testRequest) + addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest) + 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, + }) +}