-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: Support DRA Admin Access #8063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 23 commits
d2d21f4
e7b9ad4
76ca2c9
05dcfc6
a389604
bece2b0
31237c6
e9265d9
371713f
3532049
ce2b874
a99a709
a363619
6dd2644
2cb70c1
43462fa
13f7199
38083af
d846fd2
63c63ee
eee7143
57805ff
fb24953
3a4e76a
141158d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ import ( | |
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/component-helpers/scheduling/corev1" | ||
| "k8s.io/dynamic-resource-allocation/resourceclaim" | ||
| "k8s.io/utils/ptr" | ||
| ) | ||
|
|
||
| // ClaimAllocated returns whether the provided claim is allocated. | ||
|
|
@@ -114,3 +115,37 @@ func PodClaimOwnerReference(pod *apiv1.Pod) metav1.OwnerReference { | |
| func claimConsumerReferenceMatchesPod(pod *apiv1.Pod, ref resourceapi.ResourceClaimConsumerReference) bool { | ||
| return ref.APIGroup == "" && ref.Resource == "pods" && ref.Name == pod.Name && ref.UID == pod.UID | ||
| } | ||
|
|
||
| // ClaimWithoutAdminAccessRequests returns a copy of the claim without admin access requests, and their results. | ||
| func ClaimWithoutAdminAccessRequests(claim *resourceapi.ResourceClaim) *resourceapi.ResourceClaim { | ||
| claimCopy := claim.DeepCopy() | ||
| if claimCopy.Status.Allocation == nil { | ||
| return claimCopy | ||
| } | ||
| deviceRequestAllocationResults := make([]resourceapi.DeviceRequestAllocationResult, 0) | ||
| for _, deviceRequestAllocationResult := range claimCopy.Status.Allocation.Devices.Results { | ||
| // Device requests with AdminAccess don't reserve their allocated resources, and are ignored when scheuling. | ||
| devReq := getDeviceResultRequest(claim, &deviceRequestAllocationResult) | ||
| if ptr.Deref(devReq.Exactly.AdminAccess, false) { | ||
| continue | ||
| } | ||
| deviceRequestAllocationResults = append(deviceRequestAllocationResults, deviceRequestAllocationResult) | ||
| } | ||
| if claimCopy.Status.Allocation != nil { | ||
| claimCopy.Status.Allocation.Devices.Results = deviceRequestAllocationResults | ||
| } | ||
| return claimCopy | ||
| } | ||
|
|
||
| // getDeviceResultRequest returns the DeviceRequest for the provided DeviceRequestAllocationResult in the provided ResourceClaim. If no result is found, nil is returned. | ||
| func getDeviceResultRequest(claim *resourceapi.ResourceClaim, deviceRequestAllocationResult *resourceapi.DeviceRequestAllocationResult) *resourceapi.DeviceRequest { | ||
| if claim.Status.Allocation == nil { | ||
|
||
| return nil | ||
| } | ||
| for _, deviceRequest := range claim.Spec.Devices.Requests { | ||
| if deviceRequest.Name == deviceRequestAllocationResult.Request { | ||
| return &deviceRequest | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,11 @@ func groupAllocatedDevices(claims []*resourceapi.ResourceClaim) (map[string]map[ | |
| } | ||
|
|
||
| for _, deviceAlloc := range alloc.Devices.Results { | ||
| if deviceAlloc.AdminAccess != nil && *deviceAlloc.AdminAccess { | ||
|
||
| // devices with admin access don't count for utilization | ||
| continue | ||
| } | ||
|
|
||
| if result[deviceAlloc.Driver] == nil { | ||
| result[deviceAlloc.Driver] = map[string][]string{} | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ import ( | |
| "k8s.io/apimachinery/pkg/types" | ||
| "k8s.io/autoscaler/cluster-autoscaler/simulator/framework" | ||
| "k8s.io/autoscaler/cluster-autoscaler/utils/test" | ||
| "k8s.io/utils/ptr" | ||
| ) | ||
|
|
||
| func TestDynamicResourceUtilization(t *testing.T) { | ||
|
|
@@ -57,7 +58,7 @@ func TestDynamicResourceUtilization(t *testing.T) { | |
| testName: "single slice, single pool, 6/10 devices used", | ||
| nodeInfo: framework.NewNodeInfo(node, | ||
| testResourceSlices(fooDriver, "pool1", "node", 0, 10, 1), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 1)..., | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 1, nil)..., | ||
| ), | ||
| wantUtilization: map[string]map[string]float64{ | ||
| fooDriver: { | ||
|
|
@@ -76,9 +77,9 @@ func TestDynamicResourceUtilization(t *testing.T) { | |
| testResourceSlices(barDriver, "pool1", "node", 0, 8, 2), | ||
| ), | ||
| mergeLists( | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2), | ||
| testPodsWithClaims(fooDriver, "pool2", "node", 18, 3), | ||
| testPodsWithClaims(barDriver, "pool1", "node", 2, 1), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2, nil), | ||
| testPodsWithClaims(fooDriver, "pool2", "node", 18, 3, nil), | ||
| testPodsWithClaims(barDriver, "pool1", "node", 2, 1, nil), | ||
| )..., | ||
| ), | ||
| wantUtilization: map[string]map[string]float64{ | ||
|
|
@@ -101,7 +102,7 @@ func TestDynamicResourceUtilization(t *testing.T) { | |
| testResourceSlices(fooDriver, "pool1", "node", 1, 20, 2), | ||
| testResourceSlices(fooDriver, "pool1", "node", 2, 30, 2), | ||
| ), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2)..., | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2, nil)..., | ||
| ), | ||
| wantUtilization: map[string]map[string]float64{ | ||
| fooDriver: { | ||
|
|
@@ -119,7 +120,7 @@ func TestDynamicResourceUtilization(t *testing.T) { | |
| testResourceSlices(fooDriver, "pool1", "node", 1, 20, 2), | ||
| testResourceSlices(fooDriver, "pool1", "node", 2, 30, 2)[:14], | ||
| ), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2)..., | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2, nil)..., | ||
| ), | ||
| wantErr: cmpopts.AnyError, | ||
| }, | ||
|
|
@@ -131,7 +132,7 @@ func TestDynamicResourceUtilization(t *testing.T) { | |
| testResourceSlices(fooDriver, "pool1", "node", 1, 20, 2)[:7], | ||
| testResourceSlices(fooDriver, "pool1", "node", 2, 30, 2), | ||
| ), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2)..., | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 2, nil)..., | ||
| ), | ||
| wantUtilization: map[string]map[string]float64{ | ||
| fooDriver: { | ||
|
|
@@ -141,7 +142,41 @@ func TestDynamicResourceUtilization(t *testing.T) { | |
| wantHighestUtilization: 0.2, | ||
| wantHighestUtilizationName: apiv1.ResourceName(fmt.Sprintf("%s/%s", fooDriver, "pool1")), | ||
| }, | ||
| { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a test case with both kinds of claims together?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep |
||
| testName: "AdminAccess ResourceClaim don't count for utilization", | ||
| nodeInfo: framework.NewNodeInfo(node, | ||
| testResourceSlices(fooDriver, "pool1", "node", 0, 10, 1), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 1, ptr.To(true))..., | ||
| ), | ||
| wantUtilization: map[string]map[string]float64{ | ||
| fooDriver: { | ||
| "pool1": 0, | ||
| }, | ||
| }, | ||
| wantHighestUtilization: 0, | ||
| wantHighestUtilizationName: apiv1.ResourceName(fmt.Sprintf("%s/%s", fooDriver, "pool1")), | ||
| }, | ||
| { | ||
| testName: "AdminAccess ResourceClaim don't count for utilization", | ||
| nodeInfo: framework.NewNodeInfo(node, | ||
| testResourceSlices(fooDriver, "pool1", "node", 0, 10, 1), | ||
| mergeLists( | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 1, ptr.To(true)), | ||
| testPodsWithClaims(fooDriver, "pool1", "node", 6, 1, ptr.To(false)), | ||
| )..., | ||
| ), | ||
| wantUtilization: map[string]map[string]float64{ | ||
| fooDriver: { | ||
| "pool1": 0.6, | ||
| }, | ||
| }, | ||
| wantHighestUtilization: 0.6, | ||
| wantHighestUtilizationName: apiv1.ResourceName(fmt.Sprintf("%s/%s", fooDriver, "pool1")), | ||
| }, | ||
| } { | ||
| if tc.testName != "AdminAccess ResourceClaim don't count for utilization" { | ||
| continue | ||
| } | ||
| t.Run(tc.testName, func(t *testing.T) { | ||
| utilization, err := CalculateDynamicResourceUtilization(tc.nodeInfo) | ||
| if diff := cmp.Diff(tc.wantErr, err, cmpopts.EquateErrors()); diff != "" { | ||
|
|
@@ -190,7 +225,7 @@ func testResourceSlices(driverName, poolName, nodeName string, poolGen, deviceCo | |
| return result | ||
| } | ||
|
|
||
| func testPodsWithClaims(driverName, poolName, nodeName string, deviceCount, devicesPerPod int64) []*framework.PodInfo { | ||
| func testPodsWithClaims(driverName, poolName, nodeName string, deviceCount, devicesPerPod int64, adminaccess *bool) []*framework.PodInfo { | ||
| podCount := deviceCount / devicesPerPod | ||
|
|
||
| deviceIndex := 0 | ||
|
|
@@ -201,13 +236,25 @@ func testPodsWithClaims(driverName, poolName, nodeName string, deviceCount, devi | |
| for podDevIndex := range devicesPerPod { | ||
| claimName := fmt.Sprintf("%s-claim-%d", pod.Name, podDevIndex) | ||
| devName := fmt.Sprintf("%s-%s-dev-%d", driverName, poolName, deviceIndex) | ||
| devReqName := fmt.Sprintf("request-%d", podDevIndex) | ||
| devReq := resourceapi.DeviceRequest{ | ||
| Name: devReqName, | ||
| Exactly: &resourceapi.ExactDeviceRequest{ | ||
| AdminAccess: adminaccess, | ||
| }, | ||
| } | ||
| claims = append(claims, &resourceapi.ResourceClaim{ | ||
| ObjectMeta: metav1.ObjectMeta{Name: claimName, UID: types.UID(claimName)}, | ||
| Spec: resourceapi.ResourceClaimSpec{ | ||
| Devices: resourceapi.DeviceClaim{ | ||
| Requests: []resourceapi.DeviceRequest{devReq}, | ||
| }, | ||
| }, | ||
| Status: resourceapi.ResourceClaimStatus{ | ||
| Allocation: &resourceapi.AllocationResult{ | ||
| Devices: resourceapi.DeviceAllocationResult{ | ||
| Results: []resourceapi.DeviceRequestAllocationResult{ | ||
| {Request: fmt.Sprintf("request-%d", podDevIndex), Driver: driverName, Pool: poolName, Device: devName}, | ||
| {Request: devReqName, Driver: driverName, Pool: poolName, Device: devName, AdminAccess: adminaccess}, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L122 already guarantees that
claimCopywon't be nil at this point in the execution flow, so we can simply assigndeviceRequestAllocationResultstoclaimCopy.Status.Allocation.Devices.Resultswithout checking for nil here.