From 5cfaf4744893397b27998d0ef0795f50bf53116d Mon Sep 17 00:00:00 2001 From: Sergey Kanzhelev Date: Fri, 25 Oct 2024 23:03:30 +0000 Subject: [PATCH] update allocated resources status validation to account for claims --- pkg/apis/core/validation/validation.go | 36 +++++-- pkg/apis/core/validation/validation_test.go | 104 +++++++++++++++++++- 2 files changed, 132 insertions(+), 8 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 01d12a036d3..3623b9d3145 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -8260,17 +8260,39 @@ func validateContainerStatusAllocatedResourcesStatus(containerStatuses []core.Co // ignore missing container, see https://github.com/kubernetes/kubernetes/issues/124915 if containerFound { found := false + var errorStr string - // get container resources from the spec - containerResources := container.Resources - for resourceName := range containerResources.Requests { - if resourceName == allocatedResource.Name { - found = true - break + if strings.HasPrefix(string(allocatedResource.Name), "claim:") { + // assume it is a claim name + + errorStr = "must match one of the container's resource claims in a format 'claim:/' or 'claim:' if request is empty" + + for _, c := range container.Resources.Claims { + name := "claim:" + c.Name + if c.Request != "" { + name += "/" + c.Request + } + + if name == string(allocatedResource.Name) { + found = true + break + } + } + + } else { + // assume it is a resource name + + errorStr = "must match one of the container's resource requests" + + for resourceName := range container.Resources.Requests { + if resourceName == allocatedResource.Name { + found = true + break + } } } if !found { - allErrors = append(allErrors, field.Invalid(fldPath.Index(i).Child("allocatedResourcesStatus").Index(j).Child("name"), allocatedResource.Name, "must match one of the container's resource requirements")) + allErrors = append(allErrors, field.Invalid(fldPath.Index(i).Child("allocatedResourcesStatus").Index(j).Child("name"), allocatedResource.Name, errorStr)) } } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index a4e9325e8d1..9c2b655f0c2 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -24772,7 +24772,109 @@ func TestValidateContainerStatusAllocatedResourcesStatus(t *testing.T) { }, }, wantFieldErrors: field.ErrorList{ - field.Invalid(fldPath.Index(0).Child("allocatedResourcesStatus").Index(1).Child("name"), core.ResourceName("test.device/test2"), "must match one of the container's resource requirements"), + field.Invalid(fldPath.Index(0).Child("allocatedResourcesStatus").Index(1).Child("name"), core.ResourceName("test.device/test2"), "must match one of the container's resource requests"), + }, + }, + + "allow claims and request that are in spec": { + containers: []core.Container{ + { + Name: "container-1", + Resources: core.ResourceRequirements{ + Claims: []core.ResourceClaim{ + { + Name: "claim.name", + Request: "request.name", + }, + }, + }, + }, + }, + containerStatuses: []core.ContainerStatus{ + { + Name: "container-1", + AllocatedResourcesStatus: []core.ResourceStatus{ + { + Name: "claim:claim.name/request.name", + Resources: []core.ResourceHealth{ + { + ResourceID: "driver/pool/device-name", + Health: core.ResourceHealthStatusHealthy, + }, + }, + }, + }, + }, + }, + wantFieldErrors: field.ErrorList{}, + }, + + "allow claims that are in spec without the request": { + containers: []core.Container{ + { + Name: "container-1", + Resources: core.ResourceRequirements{ + Claims: []core.ResourceClaim{ + { + Name: "claim.name", + }, + }, + }, + }, + }, + containerStatuses: []core.ContainerStatus{ + { + Name: "container-1", + AllocatedResourcesStatus: []core.ResourceStatus{ + { + Name: "claim:claim.name", + Resources: []core.ResourceHealth{ + { + ResourceID: "driver/pool/device-name", + Health: core.ResourceHealthStatusHealthy, + }, + }, + }, + }, + }, + }, + wantFieldErrors: field.ErrorList{}, + }, + + "don't allow claims that are not in spec": { + containers: []core.Container{ + { + Name: "container-1", + Resources: core.ResourceRequirements{ + Claims: []core.ResourceClaim{ + { + Name: "other-claim.name", + }, + }, + Requests: core.ResourceList{ + "claim.name": resource.MustParse("1"), + }, + }, + }, + }, + containerStatuses: []core.ContainerStatus{ + { + Name: "container-1", + AllocatedResourcesStatus: []core.ResourceStatus{ + { + Name: "claim:claim.name", + Resources: []core.ResourceHealth{ + { + ResourceID: "driver/pool/device-name", + Health: core.ResourceHealthStatusHealthy, + }, + }, + }, + }, + }, + }, + wantFieldErrors: field.ErrorList{ + field.Invalid(fldPath.Index(0).Child("allocatedResourcesStatus").Index(0).Child("name"), core.ResourceName("claim:claim.name"), "must match one of the container's resource claims in a format 'claim:/' or 'claim:' if request is empty"), }, },