diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 48f306a7cf3..dbda812aad1 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -1106,6 +1106,8 @@ func dropDisabledPodStatusFields(podStatus, oldPodStatus *api.PodStatus, podSpec dropImageVolumeWithDigest(podStatus) } + dropPodNodeAllocatableResourceStatus(podStatus, oldPodStatus) + } // dropDisabledDynamicResourceAllocationFields removes pod claim references from @@ -1127,6 +1129,20 @@ func draExendedResourceInUse(podStatus *api.PodStatus) bool { return false } +func dropPodNodeAllocatableResourceStatus(podStatus, oldPodStatus *api.PodStatus) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRANodeAllocatableResources) || draNodeAllocatableResourceStatusInUse(oldPodStatus) { + return + } + podStatus.NodeAllocatableResourceClaimStatuses = nil +} + +func draNodeAllocatableResourceStatusInUse(podStatus *api.PodStatus) bool { + if podStatus == nil { + return false + } + return len(podStatus.NodeAllocatableResourceClaimStatuses) > 0 +} + func resourceHealthStatusInUse(podStatus *api.PodStatus) bool { if podStatus == nil { return false diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 6cd8d23c519..510b59c3a23 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -917,16 +917,66 @@ func TestDropDynamicResourceAllocation(t *testing.T) { }, }, } + podWithDRANodeAllocatableResourceStatus := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Resources: api.ResourceRequirements{ + Claims: []api.ResourceClaim{{Name: "my-claim"}}, + }, + }, + }, + InitContainers: []api.Container{{}}, + EphemeralContainers: []api.EphemeralContainer{{}}, + ResourceClaims: []api.PodResourceClaim{ + { + Name: "my-claim", + ResourceClaimName: &resourceClaimName, + }, + }, + }, + Status: api.PodStatus{ + NodeAllocatableResourceClaimStatuses: []api.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "node-allocatable-claim", + Resources: map[api.ResourceName]resource.Quantity{ + api.ResourceMemory: resource.MustParse("100Mi"), + }, + }, + }, + }, + } + + podWithoutDRANodeAllocatableResourceStatus := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Resources: api.ResourceRequirements{ + Claims: []api.ResourceClaim{{Name: "my-claim"}}, + }, + }, + }, + InitContainers: []api.Container{{}}, + EphemeralContainers: []api.EphemeralContainer{{}}, + ResourceClaims: []api.PodResourceClaim{ + { + Name: "my-claim", + ResourceClaimName: &resourceClaimName, + }, + }, + }, + } var noPod *api.Pod testcases := []struct { - description string - enabled bool - extendedEnabled bool - oldPod *api.Pod - newPod *api.Pod - wantPod *api.Pod + description string + enabled bool + extendedEnabled bool + enableDRANodeAllocatableResouces bool + oldPod *api.Pod + newPod *api.Pod + wantPod *api.Pod }{ { description: "old with claims / new with claims / disabled", @@ -1057,6 +1107,38 @@ func TestDropDynamicResourceAllocation(t *testing.T) { newPod: podWithExtendedResource, wantPod: podWithExtendedResource, }, + { + description: "DRA node allocatable resources / no old pod / new with DRA node allocatable resource / disabled", + enabled: true, + enableDRANodeAllocatableResouces: false, + oldPod: noPod, + newPod: podWithDRANodeAllocatableResourceStatus, + wantPod: podWithoutDRANodeAllocatableResourceStatus, + }, + { + description: "DRA node allocatable resources / no old pod / new with DRA node allocatable resource / enabled", + enabled: true, + enableDRANodeAllocatableResouces: true, + oldPod: noPod, + newPod: podWithDRANodeAllocatableResourceStatus, + wantPod: podWithDRANodeAllocatableResourceStatus, + }, + { + description: "DRA node allocatable resources / old without node allocatable resource status / new with node allocatable resource status / disabled", + enabled: true, + enableDRANodeAllocatableResouces: false, + oldPod: podWithoutDRANodeAllocatableResourceStatus, + newPod: podWithDRANodeAllocatableResourceStatus, + wantPod: podWithoutDRANodeAllocatableResourceStatus, + }, + { + description: "DRA node allocatable resources / old without node allocatable resource status / new with node allocatable resource status / enabled", + enabled: true, + enableDRANodeAllocatableResouces: true, + oldPod: podWithoutDRANodeAllocatableResourceStatus, + newPod: podWithDRANodeAllocatableResourceStatus, + wantPod: podWithDRANodeAllocatableResourceStatus, + }, } for _, tc := range testcases { @@ -1064,10 +1146,14 @@ func TestDropDynamicResourceAllocation(t *testing.T) { if !tc.enabled { featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.34")) } - featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ + overrides := featuregatetesting.FeatureOverrides{ features.DynamicResourceAllocation: tc.enabled, features.DRAExtendedResource: tc.extendedEnabled, - }) + } + if tc.enableDRANodeAllocatableResouces { + overrides[features.DRANodeAllocatableResources] = true + } + featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, overrides) oldPod := tc.oldPod.DeepCopy() newPod := tc.newPod.DeepCopy() diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index a735b8b727e..9732d4bfef9 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -65,6 +65,7 @@ import ( "k8s.io/kubernetes/pkg/apis/core/helper/qos" podshelper "k8s.io/kubernetes/pkg/apis/core/pods" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" + v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/capabilities" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/fieldpath" @@ -6050,6 +6051,7 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions allErrs = append(allErrs, ValidateEphemeralContainerStateTransition(newPod.Status.EphemeralContainerStatuses, oldPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"))...) allErrs = append(allErrs, validatePodResourceClaimStatuses(newPod.Status.ResourceClaimStatuses, newPod.Spec.ResourceClaims, fldPath.Child("resourceClaimStatuses"))...) allErrs = append(allErrs, validatePodExtendedResourceClaimStatus(newPod.Status.ExtendedResourceClaimStatus, &newPod.Spec, fldPath.Child("extendedResourceClaimStatus"))...) + allErrs = append(allErrs, validateNodeAllocatableResourceClaimStatus(newPod.Status, &newPod.Spec, fldPath.Child("nodeAllocatableResourceClaimStatuses"))...) if newIPErrs := validatePodIPs(newPod, oldPod); len(newIPErrs) > 0 { allErrs = append(allErrs, newIPErrs...) @@ -6139,6 +6141,67 @@ func validatePodResourceClaimStatuses(statuses []core.PodResourceClaimStatus, po return allErrs } +// validateNodeAllocatableResourceClaimStatus validates NodeAllocatableResourceClaimStatuses in a pod status +func validateNodeAllocatableResourceClaimStatus(podStatus core.PodStatus, podSpec *core.PodSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if len(podStatus.NodeAllocatableResourceClaimStatuses) == 0 { + return allErrs + } + + for i, nodeAllocatableStatus := range podStatus.NodeAllocatableResourceClaimStatuses { + statusFldPath := fldPath.Index(i) + if nodeAllocatableStatus.ResourceClaimName == "" { + allErrs = append(allErrs, field.Required(statusFldPath.Child("resourceClaimName"), "must not be empty")) + } + + // First check the podSpec to see if the ResourceClaim is directly referenced. + // If not, check the podStatus to see if the ResourceClaim was generated from a template. + found := false + for _, claimRef := range podSpec.ResourceClaims { + if (claimRef.ResourceClaimName != nil) && (*claimRef.ResourceClaimName == nodeAllocatableStatus.ResourceClaimName) { + found = true + break + } + } + if !found { + for _, claimRef := range podStatus.ResourceClaimStatuses { + if (claimRef.ResourceClaimName != nil) && (*claimRef.ResourceClaimName == nodeAllocatableStatus.ResourceClaimName) { + found = true + break + } + } + } + + if !found { + allErrs = append(allErrs, field.Invalid(statusFldPath.Child("resourceClaimName"), nodeAllocatableStatus.ResourceClaimName, "no mapping found in pod reference")) + } + + // TODO(KEP-5517): Evaluate if its ok to have no containers referencing a node allocatable resource claim. + // This is pending on defining kubelet cgroup enforcement. + if len(nodeAllocatableStatus.Containers) == 0 { + allErrs = append(allErrs, field.Required(statusFldPath.Child("containers"), "must not be empty")) + } + + resourcesFldPath := statusFldPath.Child("resources") + if len(nodeAllocatableStatus.Resources) == 0 { + allErrs = append(allErrs, field.Required(resourcesFldPath, "must not be empty")) + } + + for resourceName, quantity := range nodeAllocatableStatus.Resources { + keyPath := resourcesFldPath.Key(string(resourceName)) + if !v1helper.IsNativeResource(v1.ResourceName(resourceName)) { + allErrs = append(allErrs, field.Invalid(keyPath, resourceName, "must be a node allocatable resource name")) + } + if quantity.Cmp(resource.Quantity{}) < 0 { + allErrs = append(allErrs, field.Invalid(keyPath, quantity.String(), "must be non-negative")) + } + } + } + + return allErrs +} + // validatePodExtendedResourceClaimStatus validates the ExtendedResourceClaimStatus in a pod status. func validatePodExtendedResourceClaimStatus(status *core.PodExtendedResourceClaimStatus, spec *core.PodSpec, fldPath *field.Path) field.ErrorList { if status == nil { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index cc24db75df6..a5f3d6e6235 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -16259,6 +16259,304 @@ func TestValidatePodStatusUpdate(t *testing.T) { } } +func TestValidateNodeAllocatableResourceClaimStatus(t *testing.T) { + validPodSpec1 := core.PodSpec{ + Containers: []core.Container{ + { + Name: "c1", + Image: "image", + Resources: core.ResourceRequirements{ + Claims: []core.ResourceClaim{ + {Name: "claim1"}, + }, + }, + }, + }, + ResourceClaims: []core.PodResourceClaim{ + {Name: "claim1", ResourceClaimName: ptr.To("my-claim1")}, + }, + } + validPodSpec2 := core.PodSpec{ + Containers: []core.Container{ + { + Name: "c1", + Image: "image", + Resources: core.ResourceRequirements{ + Claims: []core.ResourceClaim{ + {Name: "claim1"}, + {Name: "claim2"}, + }, + }, + }, + }, + ResourceClaims: []core.PodResourceClaim{ + {Name: "claim1", ResourceClaimName: ptr.To("my-claim1")}, + {Name: "claim2", ResourceClaimName: ptr.To("my-claim2")}, + }, + } + + testCases := []struct { + name string + podStatus core.PodStatus + spec core.PodSpec + expectError bool + errorType field.ErrorType + errorField string + errorMsg string + }{ + { + name: "Valid NodeAllocatableResourceClaimStatus", + spec: validPodSpec1, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + core.ResourceMemory: resource.MustParse("1Gi"), + }, + }, + }, + }, + expectError: false, + }, + { + name: "Valid Multiple NodeAllocatableResourceClaimStatus", + spec: validPodSpec2, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ResourceClaimName: "my-claim2", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }, + }, + expectError: false, + }, + { + name: "Invalid Resource Name", + spec: validPodSpec1, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + "example.com/foo": resource.MustParse("1"), + }, + }, + }, + }, + expectError: true, + errorType: field.ErrorTypeInvalid, + errorField: "status.nodeAllocatableResourceClaimStatuses[0].resources[example.com/foo]", + errorMsg: "must be a node allocatable resource name", + }, + { + name: "Negative Quantity", + spec: validPodSpec1, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("-1"), + }, + }, + }, + }, + expectError: true, + errorType: field.ErrorTypeInvalid, + errorField: "status.nodeAllocatableResourceClaimStatuses[0].resources[cpu]", + errorMsg: "must be non-negative", + }, + { + name: "Empty containers list", + spec: validPodSpec1, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + expectError: true, + errorType: field.ErrorTypeRequired, + errorField: "status.nodeAllocatableResourceClaimStatuses[0].containers", + errorMsg: "must not be empty", + }, + { + name: "Missing ResourceClaimName", + spec: validPodSpec1, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + expectError: true, + errorType: field.ErrorTypeRequired, + errorField: "status.nodeAllocatableResourceClaimStatuses[0].resourceClaimName", + errorMsg: "must not be empty", + }, + { + name: "Empty Resources", + spec: validPodSpec1, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{}, + }, + }, + }, + expectError: true, + errorType: field.ErrorTypeRequired, + errorField: "status.nodeAllocatableResourceClaimStatuses[0].resources", + errorMsg: "must not be empty", + }, + { + name: "Valid ResourceClaimName from PodSpec", + spec: core.PodSpec{ + Containers: []core.Container{ + { + Name: "c1", + Image: "image", + }, + }, + ResourceClaims: []core.PodResourceClaim{ + {Name: "claim1", ResourceClaimName: ptr.To("my-claim1")}, + }, + }, + podStatus: core.PodStatus{ + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "my-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + expectError: false, + }, + { + name: "Valid ResourceClaimName from PodStatus", + spec: core.PodSpec{ + Containers: []core.Container{ + { + Name: "c1", + Image: "image", + }, + }, + }, + podStatus: core.PodStatus{ + ResourceClaimStatuses: []core.PodResourceClaimStatus{ + {Name: "claim1", ResourceClaimName: ptr.To("generated-claim1")}, + }, + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "generated-claim1", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + expectError: false, + }, + { + name: "Invalid ResourceClaimName not found", + spec: core.PodSpec{ + Containers: []core.Container{ + { + Name: "c1", + Image: "image", + }, + }, + ResourceClaims: []core.PodResourceClaim{ + {Name: "claim1", ResourceClaimName: ptr.To("my-claim1")}, + }, + }, + podStatus: core.PodStatus{ + ResourceClaimStatuses: []core.PodResourceClaimStatus{ + {Name: "claim2", ResourceClaimName: ptr.To("generated-claim2")}, + }, + NodeAllocatableResourceClaimStatuses: []core.NodeAllocatableResourceClaimStatus{ + { + ResourceClaimName: "non-existent-claim", + Containers: []string{"c1"}, + Resources: map[core.ResourceName]resource.Quantity{ + core.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + expectError: true, + errorType: field.ErrorTypeInvalid, + errorField: "status.nodeAllocatableResourceClaimStatuses[0].resourceClaimName", + errorMsg: "not found in PodSpec.ResourceClaims or PodStatus.ResourceClaimStatuses", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + errs := validateNodeAllocatableResourceClaimStatus(tc.podStatus, &tc.spec, field.NewPath("status", "nodeAllocatableResourceClaimStatuses")) + + if !tc.expectError { + if len(errs) != 0 { + t.Errorf("Unexpected errors: %v", errs) + } + return + } + + if len(errs) == 0 { + t.Errorf("Expected errors, but got none") + return + } + + found := false + for _, err := range errs { + if err.Type == tc.errorType && err.Field == tc.errorField { + found = true + break + } + } + if !found { + if tc.errorMsg != "" { + t.Errorf("Expected error [Type: %v, Field: %s, Message containing: %q], but got %v", + tc.errorType, tc.errorField, tc.errorMsg, errs) + } else { + t.Errorf("Expected error [Type: %v, Field: %s], but got %v", + tc.errorType, tc.errorField, errs) + } + } + }) + } +} + func makeValidService() core.Service { clusterInternalTrafficPolicy := core.ServiceInternalTrafficPolicyCluster return core.Service{ diff --git a/pkg/apis/resource/v1beta1/conversion.go b/pkg/apis/resource/v1beta1/conversion.go index 73519860675..6d2c970d8c4 100644 --- a/pkg/apis/resource/v1beta1/conversion.go +++ b/pkg/apis/resource/v1beta1/conversion.go @@ -220,6 +220,19 @@ func Convert_v1beta1_Device_To_resource_Device(in *resourcev1beta1.Device, out * out.BindingConditions = basic.BindingConditions out.BindingFailureConditions = basic.BindingFailureConditions out.AllowMultipleAllocations = in.Basic.AllowMultipleAllocations + + if basic.NodeAllocatableResourceMappings != nil { + out.NodeAllocatableResourceMappings = make(map[corev1.ResourceName]resource.NodeAllocatableResourceMapping) + for key, value := range basic.NodeAllocatableResourceMappings { + var outVal resource.NodeAllocatableResourceMapping + if err := autoConvert_v1beta1_NodeAllocatableResourceMapping_To_resource_NodeAllocatableResourceMapping(&value, &outVal, s); err != nil { + return err + } + out.NodeAllocatableResourceMappings[key] = outVal + } + } else { + out.NodeAllocatableResourceMappings = nil + } } return nil } @@ -269,6 +282,18 @@ func Convert_resource_Device_To_v1beta1_Device(in *resource.Device, out *resourc out.Basic.BindingConditions = in.BindingConditions out.Basic.BindingFailureConditions = in.BindingFailureConditions out.Basic.AllowMultipleAllocations = in.AllowMultipleAllocations + if in.NodeAllocatableResourceMappings != nil { + out.Basic.NodeAllocatableResourceMappings = make(map[corev1.ResourceName]resourcev1beta1.NodeAllocatableResourceMapping) + for key, value := range in.NodeAllocatableResourceMappings { + var outVal resourcev1beta1.NodeAllocatableResourceMapping + if err := autoConvert_resource_NodeAllocatableResourceMapping_To_v1beta1_NodeAllocatableResourceMapping(&value, &outVal, s); err != nil { + return err + } + out.Basic.NodeAllocatableResourceMappings[key] = outVal + } + } else { + out.Basic.NodeAllocatableResourceMappings = nil + } return nil } diff --git a/pkg/apis/resource/v1beta1/conversion_test.go b/pkg/apis/resource/v1beta1/conversion_test.go index 09224375604..c027b0bc754 100644 --- a/pkg/apis/resource/v1beta1/conversion_test.go +++ b/pkg/apis/resource/v1beta1/conversion_test.go @@ -22,13 +22,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" resourcev1beta1 "k8s.io/api/resource/v1beta1" apiresource "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/apis/resource" + "k8s.io/utils/ptr" ) func TestConversion(t *testing.T) { + nodeName := "test-node" testcases := []struct { name string in runtime.Object @@ -317,6 +321,112 @@ func TestConversion(t *testing.T) { }, }, }, + { + name: "ResourceSlice v1beta1 to internal with node allocatable resource mappings", + in: &resourcev1beta1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: resourcev1beta1.ResourceSliceSpec{ + NodeName: nodeName, + Driver: "test-driver", + Pool: resourcev1beta1.ResourcePool{ + Name: "test-pool", + ResourceSliceCount: 1, + }, + Devices: []resourcev1beta1.Device{ + { + Name: "test-device", + Basic: &resourcev1beta1.BasicDevice{ + Attributes: map[resourcev1beta1.QualifiedName]resourcev1beta1.DeviceAttribute{ + "cpu_per_instance": {IntValue: ptr.To[int64](2)}, + }, + NodeAllocatableResourceMappings: map[v1.ResourceName]resourcev1beta1.NodeAllocatableResourceMapping{ + "cpu": { + AllocationMultiplier: ptr.To(apiresource.MustParse("1")), + }, + }, + }, + }, + }, + }, + }, + out: &resource.ResourceSlice{}, + expectOut: &resource.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: resource.ResourceSliceSpec{ + NodeName: &nodeName, + Driver: "test-driver", + Pool: resource.ResourcePool{ + Name: "test-pool", + ResourceSliceCount: 1, + }, + Devices: []resource.Device{ + { + Name: "test-device", + Attributes: map[resource.QualifiedName]resource.DeviceAttribute{ + "cpu_per_instance": {IntValue: ptr.To[int64](2)}, + }, + NodeAllocatableResourceMappings: map[v1.ResourceName]resource.NodeAllocatableResourceMapping{ + "cpu": { + AllocationMultiplier: ptr.To(apiresource.MustParse("1")), + }, + }}, + }, + }, + }, + }, + { + name: "ResourceSlice internal to v1beta1 with node allocatable resource mappings", + in: &resource.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: resource.ResourceSliceSpec{ + NodeName: &nodeName, + Driver: "test-driver", + Pool: resource.ResourcePool{ + Name: "test-pool", + ResourceSliceCount: 1, + }, + Devices: []resource.Device{ + { + Name: "test-device", + Attributes: map[resource.QualifiedName]resource.DeviceAttribute{ + "cpu_per_instance": {IntValue: ptr.To[int64](2)}, + }, + NodeAllocatableResourceMappings: map[v1.ResourceName]resource.NodeAllocatableResourceMapping{ + "cpu": { + AllocationMultiplier: ptr.To(apiresource.MustParse("1")), + }, + }}, + }, + }, + }, + out: &resourcev1beta1.ResourceSlice{}, + expectOut: &resourcev1beta1.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{Name: "test-slice"}, + Spec: resourcev1beta1.ResourceSliceSpec{ + NodeName: nodeName, + Driver: "test-driver", + Pool: resourcev1beta1.ResourcePool{ + Name: "test-pool", + ResourceSliceCount: 1, + }, + Devices: []resourcev1beta1.Device{ + { + Name: "test-device", + Basic: &resourcev1beta1.BasicDevice{ + Attributes: map[resourcev1beta1.QualifiedName]resourcev1beta1.DeviceAttribute{ + "cpu_per_instance": {IntValue: ptr.To[int64](2)}, + }, + NodeAllocatableResourceMappings: map[v1.ResourceName]resourcev1beta1.NodeAllocatableResourceMapping{ + "cpu": { + AllocationMultiplier: ptr.To(apiresource.MustParse("1")), + }, + }, + }, + }, + }, + }, + }, + }, } scheme := runtime.NewScheme() @@ -350,5 +460,4 @@ func TestConversion(t *testing.T) { } }) } - } diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index a3ad1513705..4d86c482265 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -865,6 +865,38 @@ func validateDevice(device resource.Device, oldDevice *resource.Device, fldPath } allErrs = append(allErrs, validateDeviceBindingParameters(device.BindingConditions, device.BindingFailureConditions, fldPath)...) + allErrs = append(allErrs, validateNodeAllocatableResourceMappings(device.NodeAllocatableResourceMappings, device.Capacity, fldPath.Child("nodeAllocatableResourceMappings"))...) + + return allErrs +} + +func validateNodeAllocatableResourceMappings(mappings map[corev1.ResourceName]resource.NodeAllocatableResourceMapping, capacities map[resource.QualifiedName]resource.DeviceCapacity, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + for resourceName, mapping := range mappings { + keyPath := fldPath.Key(string(resourceName)) + if !v1helper.IsNativeResource(resourceName) { + allErrs = append(allErrs, field.Invalid(keyPath, resourceName, "must be a node allocatable resource name")) + } + + if mapping.AllocationMultiplier == nil && mapping.CapacityKey == nil { + allErrs = append(allErrs, field.Invalid(keyPath, "", "at least one of allocationMultiplier or capacityKey must be set")) + } else { + if mapping.AllocationMultiplier != nil { + if mapping.AllocationMultiplier.Sign() <= 0 { + allErrs = append(allErrs, field.Invalid(keyPath.Child("allocationMultiplier"), mapping.AllocationMultiplier.String(), "must be positive")) + } + } + if mapping.CapacityKey != nil { + if *mapping.CapacityKey == "" { + allErrs = append(allErrs, field.Invalid(keyPath.Child("capacityKey"), "", "capacityKey must not be an empty string")) + } else if capacities == nil { + allErrs = append(allErrs, field.NotFound(keyPath.Child("capacityKey"), *mapping.CapacityKey)) + } else if _, exists := capacities[*mapping.CapacityKey]; !exists { + allErrs = append(allErrs, field.NotFound(keyPath.Child("capacityKey"), *mapping.CapacityKey)) + } + } + } + } return allErrs } diff --git a/pkg/apis/resource/validation/validation_resourceslice_test.go b/pkg/apis/resource/validation/validation_resourceslice_test.go index b9160a356df..d863c4a8b9f 100644 --- a/pkg/apis/resource/validation/validation_resourceslice_test.go +++ b/pkg/apis/resource/validation/validation_resourceslice_test.go @@ -21,6 +21,7 @@ import ( "strings" "testing" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" @@ -77,9 +78,9 @@ func testResourceSlice(name, nodeName, driverName string, numDevices int) *resou }, }, } - for i := 0; i < numDevices; i++ { + for d := range numDevices { device := resourceapi.Device{ - Name: fmt.Sprintf("device-%d", i), + Name: fmt.Sprintf("device-%d", d), Attributes: testAttributes(), Capacity: testCapacity(), } @@ -110,6 +111,47 @@ func testResourceSliceWithBindingConditions(name, nodeName, driverName string, n return slice } +func testNodeAllocatableResourceCapacity() map[resourceapi.QualifiedName]resourceapi.DeviceCapacity { + return map[resourceapi.QualifiedName]resourceapi.DeviceCapacity{ + "dra.example.com/cpu": {Value: resource.MustParse("100")}, + "dra.example.com/memory": {Value: resource.MustParse("100Gi")}, + } +} + +func testResourceSliceWithNodeAllocatableResources(name, nodeName, driverName string, numDevices int) *resourceapi.ResourceSlice { + slice := &resourceapi.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: resourceapi.ResourceSliceSpec{ + NodeName: &nodeName, + Driver: driverName, + Pool: resourceapi.ResourcePool{ + Name: nodeName, + ResourceSliceCount: 1, + }, + }, + } + for d := range numDevices { + device := resourceapi.Device{ + Name: fmt.Sprintf("device-%d", d), + Attributes: testAttributes(), + Capacity: testNodeAllocatableResourceCapacity(), + NodeAllocatableResourceMappings: map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: { + AllocationMultiplier: ptr.To(resource.MustParse("1")), + CapacityKey: ptr.To[resourceapi.QualifiedName]("dra.example.com/cpu"), + }, + "memory": { + CapacityKey: ptr.To[resourceapi.QualifiedName]("dra.example.com/cpu"), + }, + }, + } + slice.Spec.Devices = append(slice.Spec.Devices, device) + } + return slice +} + func TestValidateResourceSlice(t *testing.T) { goodName := "foo" badName := "!@#$%^" @@ -118,9 +160,10 @@ func TestValidateResourceSlice(t *testing.T) { badValue := "spaces not allowed" scenarios := map[string]struct { - slice *resourceapi.ResourceSlice - wantFailures field.ErrorList - consumableCapacityFeatureGate bool + slice *resourceapi.ResourceSlice + wantFailures field.ErrorList + consumableCapacityFeatureGate bool + enableDRANodeAllocatableResourcesFeatureGate bool }{ "good": { slice: testResourceSlice(goodName, goodName, driverName, resourceapi.ResourceSliceMaxDevices), @@ -993,11 +1036,136 @@ func TestValidateResourceSlice(t *testing.T) { wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices").Index(0).Child("bindingFailureConditions").Index(0), "condition1", "bindingFailureConditions must not overlap with bindingConditions")}, slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2"}, []string{"condition1", "condition3"}), }, + "correct-node-allocatable-resource-mappings": { + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "node-allocatable-resource-mappings-both-quantityfrom": { + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: { + CapacityKey: ptr.To[resourceapi.QualifiedName]("dra.example.com/cpu"), + AllocationMultiplier: ptr.To(resource.MustParse("1")), + }, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "bad-node-allocatable-resource-mappings-no-quantityfrom": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key(string(v1.ResourceCPU)), "", "at least one of allocationMultiplier or capacityKey must be set"), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: {}, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "bad-node-allocatable-resource-mappings-capacityKey-empty": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key(string(v1.ResourceCPU)).Child("capacityKey"), "", "capacityKey must not be an empty string"), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: { + CapacityKey: ptr.To[resourceapi.QualifiedName](""), + }, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "bad-node-allocatable-resource-mappings-capacity-key-not-found": { + wantFailures: field.ErrorList{ + field.NotFound(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key(string(v1.ResourceCPU)).Child("capacityKey"), resourceapi.QualifiedName("nonexistent")), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: { + CapacityKey: ptr.To[resourceapi.QualifiedName]("nonexistent"), + }, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "bad-node-allocatable-resource-mappings-invalid-allocation-multiplier-negative": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key(string(v1.ResourceCPU)).Child("allocationMultiplier"), "-1", "must be positive"), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: {AllocationMultiplier: ptr.To(resource.MustParse("-1"))}, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "bad-node-allocatable-resource-mappings-invalid-allocation-multiplier-zero": { + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key(string(v1.ResourceCPU)).Child("allocationMultiplier"), "0", "must be positive"), + }, + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSlice(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceCPU: {AllocationMultiplier: ptr.To(resource.MustParse("0"))}, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + }, + "capacity key not in spec.devices[].capacity": { + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceMemory: { + CapacityKey: ptr.To[resourceapi.QualifiedName]("dra.example.com/hugepages"), + AllocationMultiplier: ptr.To(resource.MustParse("1")), + }, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + wantFailures: field.ErrorList{ + field.NotFound(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key(string(v1.ResourceMemory)).Child("capacityKey"), resourceapi.QualifiedName("dra.example.com/hugepages")), + }, + }, + "mapped resource is not a node allocatable resource": { + slice: func() *resourceapi.ResourceSlice { + slice := testResourceSliceWithNodeAllocatableResources(goodName, goodName, driverName, 1) + slice.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resourceapi.NodeAllocatableResourceMapping{ + v1.ResourceName("dra.example.com/gpu"): { + CapacityKey: ptr.To[resourceapi.QualifiedName]("dra.example.com/cpu"), + AllocationMultiplier: ptr.To(resource.MustParse("1")), + }, + } + return slice + }(), + enableDRANodeAllocatableResourcesFeatureGate: true, + wantFailures: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("nodeAllocatableResourceMappings").Key("dra.example.com/gpu"), v1.ResourceName("dra.example.com/gpu"), "must be a node allocatable resource name"), + }, + }, } for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DRAConsumableCapacity, scenario.consumableCapacityFeatureGate) + featuregatetesting.SetFeatureGatesDuringTest(t, utilfeature.DefaultFeatureGate, featuregatetesting.FeatureOverrides{ + features.DRANodeAllocatableResources: scenario.enableDRANodeAllocatableResourcesFeatureGate, + features.DRAConsumableCapacity: scenario.consumableCapacityFeatureGate, + }) errs := ValidateResourceSlice(scenario.slice) assertFailures(t, scenario.wantFailures, errs) }) diff --git a/pkg/registry/resource/resourceslice/strategy.go b/pkg/registry/resource/resourceslice/strategy.go index 54f054b4dd3..12dc2567edb 100644 --- a/pkg/registry/resource/resourceslice/strategy.go +++ b/pkg/registry/resource/resourceslice/strategy.go @@ -196,6 +196,7 @@ func dropDisabledFields(newSlice, oldSlice *resource.ResourceSlice) { dropDisabledDRAPartitionableDevicesFields(newSlice, oldSlice) dropDisabledDRADeviceBindingConditionsFields(newSlice, oldSlice) dropDisabledDRAConsumableCapacityFields(newSlice, oldSlice) + dropDisabledDRANodeAllocatableResourcesFields(newSlice, oldSlice) } func dropDisabledDRADeviceTaintsFields(newSlice, oldSlice *resource.ResourceSlice) { @@ -324,3 +325,26 @@ func dropDisabledDRAConsumableCapacityFields(newSlice, oldSlice *resource.Resour } } } + +func dropDisabledDRANodeAllocatableResourcesFields(newSlice, oldSlice *resource.ResourceSlice) { + if utilfeature.DefaultFeatureGate.Enabled(features.DRANodeAllocatableResources) || draNodeAllocatableResourcesFeatureInUse(oldSlice) { + return + } + + for i := range newSlice.Spec.Devices { + newSlice.Spec.Devices[i].NodeAllocatableResourceMappings = nil + } +} + +func draNodeAllocatableResourcesFeatureInUse(slice *resource.ResourceSlice) bool { + if slice == nil { + return false + } + + for _, device := range slice.Spec.Devices { + if len(device.NodeAllocatableResourceMappings) > 0 { + return true + } + } + return false +} diff --git a/pkg/registry/resource/resourceslice/strategy_test.go b/pkg/registry/resource/resourceslice/strategy_test.go index 9ae889d5f1a..28fc56c2773 100644 --- a/pkg/registry/resource/resourceslice/strategy_test.go +++ b/pkg/registry/resource/resourceslice/strategy_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" k8sresource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" @@ -187,6 +188,17 @@ var sliceWithConsumableCapacity = func() *resource.ResourceSlice { return obj }() +var sliceWithNodeAllocatableResources = func() *resource.ResourceSlice { + obj := slice.DeepCopy() + instanceQuantity := k8sresource.MustParse("1") + obj.Spec.Devices[0].NodeAllocatableResourceMappings = map[v1.ResourceName]resource.NodeAllocatableResourceMapping{ + v1.ResourceCPU: { + AllocationMultiplier: &instanceQuantity, + }, + } + return obj +}() + func TestResourceSliceStrategy(t *testing.T) { if Strategy.NamespaceScoped() { t.Errorf("ResourceSlice must not be namespace scoped") @@ -199,14 +211,15 @@ func TestResourceSliceStrategy(t *testing.T) { func TestResourceSliceStrategyCreate(t *testing.T) { ctx := genericapirequest.NewDefaultContext() testCases := map[string]struct { - obj *resource.ResourceSlice - deviceTaints bool - partitionableDevices bool - bindingConditions bool - deviceStatus bool - consumableCapacity bool - expectedValidationError bool - expectObj *resource.ResourceSlice + obj *resource.ResourceSlice + deviceTaints bool + partitionableDevices bool + bindingConditions bool + deviceStatus bool + consumableCapacity bool + draNodeAllocatableResources bool + expectedValidationError bool + expectObj *resource.ResourceSlice }{ "simple": { obj: slice, @@ -351,6 +364,24 @@ func TestResourceSliceStrategyCreate(t *testing.T) { return obj }(), }, + "keep-fields-node-allocatable-dra-claims": { + obj: sliceWithNodeAllocatableResources, + draNodeAllocatableResources: true, + expectObj: func() *resource.ResourceSlice { + obj := sliceWithNodeAllocatableResources.DeepCopy() + obj.Generation = 1 + return obj + }(), + }, + "drop-fields-node-allocatable-dra-claims-disabled-feature": { + obj: sliceWithNodeAllocatableResources, + draNodeAllocatableResources: false, + expectObj: func() *resource.ResourceSlice { + obj := slice.DeepCopy() + obj.Generation = 1 + return obj + }(), + }, } for name, tc := range testCases { @@ -361,6 +392,7 @@ func TestResourceSliceStrategyCreate(t *testing.T) { features.DRADeviceBindingConditions: tc.bindingConditions, features.DRAResourceClaimDeviceStatus: tc.deviceStatus, features.DRAConsumableCapacity: tc.consumableCapacity, + features.DRANodeAllocatableResources: tc.draNodeAllocatableResources, }) obj := tc.obj.DeepCopy() diff --git a/staging/src/k8s.io/dynamic-resource-allocation/api/v1beta1/conversion.go b/staging/src/k8s.io/dynamic-resource-allocation/api/v1beta1/conversion.go index 2289c7993ae..5fea1fdae00 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/api/v1beta1/conversion.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/api/v1beta1/conversion.go @@ -188,6 +188,18 @@ func Convert_v1beta1_Device_To_v1_Device(in *resourcev1beta1.Device, out *resour out.BindingConditions = basic.BindingConditions out.BindingFailureConditions = basic.BindingFailureConditions out.AllowMultipleAllocations = basic.AllowMultipleAllocations + if basic.NodeAllocatableResourceMappings != nil { + out.NodeAllocatableResourceMappings = make(map[corev1.ResourceName]resourceapi.NodeAllocatableResourceMapping) + for key, value := range basic.NodeAllocatableResourceMappings { + var outVal resourceapi.NodeAllocatableResourceMapping + if err := autoConvert_v1beta1_NodeAllocatableResourceMapping_To_v1_NodeAllocatableResourceMapping(&value, &outVal, s); err != nil { + return err + } + out.NodeAllocatableResourceMappings[key] = outVal + } + } else { + out.NodeAllocatableResourceMappings = nil + } } return nil } @@ -237,6 +249,19 @@ func Convert_v1_Device_To_v1beta1_Device(in *resourceapi.Device, out *resourcev1 out.Basic.BindingConditions = in.BindingConditions out.Basic.BindingFailureConditions = in.BindingFailureConditions out.Basic.AllowMultipleAllocations = in.AllowMultipleAllocations + if in.NodeAllocatableResourceMappings != nil { + out.Basic.NodeAllocatableResourceMappings = make(map[corev1.ResourceName]resourcev1beta1.NodeAllocatableResourceMapping) + for key, value := range in.NodeAllocatableResourceMappings { + var outVal resourcev1beta1.NodeAllocatableResourceMapping + if err := autoConvert_v1_NodeAllocatableResourceMapping_To_v1beta1_NodeAllocatableResourceMapping(&value, &outVal, s); err != nil { + return err + } + out.Basic.NodeAllocatableResourceMappings[key] = outVal + } + } else { + out.Basic.NodeAllocatableResourceMappings = nil + } + return nil }