Skip to content

Commit f7ffeb4

Browse files
MenD32MenD32
committed
feat: Support DRA Admin Access
Signed-off-by: MenD32 <[email protected]> tests: added utilization test for adminaccess resourceclaims Signed-off-by: MenD32 <[email protected]> fix: ran gofmt Signed-off-by: MenD32 <[email protected]> fix: false positives on SanitizedPodResourceClaims Signed-off-by: MenD32 <[email protected]> fix: non admin results get removed from resourceclaim when requests is empty Signed-off-by: MenD32 <[email protected]> feat: Support DRA Admin Access Signed-off-by: MenD32 <[email protected]> refactor: moved device filtering to groupAllocatedDevices Signed-off-by: MenD32 <[email protected]> tests: added test of combined admin access and non-admin access utilization calculation Signed-off-by: MenD32 <[email protected]> tests: added test for admin access request sanitization Signed-off-by: MenD32 <[email protected]> feat: Support DRA Admin Access Signed-off-by: MenD32 <[email protected]> tests: added utilization test for adminaccess resourceclaims Signed-off-by: MenD32 <[email protected]> fix: ran gofmt Signed-off-by: MenD32 <[email protected]> fix: false positives on SanitizedPodResourceClaims Signed-off-by: MenD32 <[email protected]> fix: non admin results get removed from resourceclaim when requests is empty Signed-off-by: MenD32 <[email protected]> feat: Support DRA Admin Access Signed-off-by: MenD32 <[email protected]> refactor: moved device filtering to groupAllocatedDevices Signed-off-by: MenD32 <[email protected]> fix: removed ClaimWithoutAdminAccessRequests sanitization Signed-off-by: MenD32 <[email protected]> chore: migrated to v1 Signed-off-by: MenD32 <[email protected]> fix: changed methodology to using ptr.Deref Signed-off-by: MenD32 <[email protected]> fix: changed methodology to using ptr.Deref Signed-off-by: MenD32 <[email protected]> removed allocation status check from getDeviceResultRequest Signed-off-by: MenD32 <[email protected]> fix: added handling logic for daemonsets with non-adminAccess resource claims (and also style fixes) Signed-off-by: MenD32 <[email protected]> tests: changed duplicated test name Signed-off-by: MenD32 <[email protected]> style: changed variable name to hasRegularAllocations Signed-off-by: MenD32 <[email protected]>
1 parent ffcbfee commit f7ffeb4

File tree

4 files changed

+235
-22
lines changed

4 files changed

+235
-22
lines changed

‎cluster-autoscaler/simulator/dynamicresources/utils/sanitize.go‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
"k8s.io/apimachinery/pkg/util/uuid"
2626
"k8s.io/dynamic-resource-allocation/resourceclaim"
27+
"k8s.io/utils/ptr"
2728
"k8s.io/utils/set"
2829
)
2930

@@ -54,11 +55,13 @@ func SanitizedNodeResourceSlices(nodeLocalSlices []*resourceapi.ResourceSlice, n
5455
// - ResourceClaims not owned by oldOwner are returned unchanged in the result. They are shared claims not bound to the
5556
// lifecycle of the duplicated pod, so they shouldn't be duplicated.
5657
// - Works for unallocated claims (e.g. if the pod being duplicated isn't scheduled).
58+
// - Works for claims with only AdminAccess allocations, which can be safely duplicated without sanitization since they
59+
// don't reserve devices.
5760
// - Works for claims allocated on a single node that is being duplicated (e.g. if the pod being duplicated is a scheduled DS pod).
5861
// The name of the old node and its pools have to be provided in this case. Such allocated claims are pointed to newNodeName,
59-
// and nameSuffix is appended to all pool names in allocation results, to match the pool names of the new, duplicated node.
60-
// - Returns an error if any of the allocated claims is not node-local on oldNodeName. Such allocations can't be sanitized, the only
61-
// option is to clear the allocation and run scheduler filters&reserve to get a new allocation when duplicating a pod.
62+
// and nameSuffix is appended to all pool names in non-AdminAccess allocation results, to match the pool names of the new, duplicated node.
63+
// - Returns an error if any of the allocated non-AdminAccess claims is not node-local on oldNodeName. Such allocations can't be sanitized,
64+
// the only option is to clear the allocation and run scheduler filters&reserve to get a new allocation when duplicating a pod.
6265
func SanitizedPodResourceClaims(newOwner, oldOwner *v1.Pod, claims []*resourceapi.ResourceClaim, nameSuffix, newNodeName, oldNodeName string, oldNodePoolNames set.Set[string]) ([]*resourceapi.ResourceClaim, error) {
6366
var result []*resourceapi.ResourceClaim
6467
for _, claim := range claims {
@@ -81,14 +84,17 @@ func SanitizedPodResourceClaims(newOwner, oldOwner *v1.Pod, claims []*resourceap
8184
continue
8285
}
8386

84-
if singleNodeSelector := nodeSelectorSingleNode(claimCopy.Status.Allocation.NodeSelector); singleNodeSelector == "" || singleNodeSelector != oldNodeName {
85-
// This claim most likely has an allocation available on more than a single Node. We can't sanitize it, and it shouldn't be duplicated as we'd
86-
// have multiple claims allocating the same devices.
87-
return nil, fmt.Errorf("claim %s/%s is allocated, but not node-local on %s - can't be sanitized", claim.Namespace, claim.Name, oldNodeName)
88-
}
89-
9087
var sanitizedAllocations []resourceapi.DeviceRequestAllocationResult
88+
hasRegularAllocations := false
9189
for _, devAlloc := range claim.Status.Allocation.Devices.Results {
90+
if ptr.Deref(devAlloc.AdminAccess, false) {
91+
// AdminAccess Device allocations don't actually reserve the Device, and the same Device can be allocated for multiple AdminAccess requests.
92+
// When we just copy the allocation without sanitizing, we introduce multiple allocations for the same DRA Device.
93+
// But this is fine for AdminAccess allocations, so in this case we can just copy it.
94+
sanitizedAllocations = append(sanitizedAllocations, devAlloc)
95+
continue
96+
}
97+
hasRegularAllocations = true
9298
// It's possible to have both node-local and global allocations in a single resource claim. Make sure that all allocations were node-local on the old node.
9399
if !oldNodePoolNames.Has(devAlloc.Pool) {
94100
return nil, fmt.Errorf("claim %s/%s has an allocation %s, from a pool that isn't node-local on %s - can't be sanitized", claim.Namespace, claim.Name, devAlloc.Request, oldNodeName)
@@ -97,6 +103,13 @@ func SanitizedPodResourceClaims(newOwner, oldOwner *v1.Pod, claims []*resourceap
97103
sanitizedAllocations = append(sanitizedAllocations, devAlloc)
98104
}
99105

106+
// if all allocations were AdminAccess, we're done checking for node-locality, doesn't matter since it doesn't reserve anything.
107+
if singleNodeSelector := nodeSelectorSingleNode(claimCopy.Status.Allocation.NodeSelector); hasRegularAllocations && (singleNodeSelector == "" || singleNodeSelector != oldNodeName) {
108+
// This claim most likely has an allocation available on more than a single Node. We can't sanitize it, and it shouldn't be duplicated as we'd
109+
// have multiple claims allocating the same devices.
110+
return nil, fmt.Errorf("claim %s/%s is allocated, but not node-local on %s - can't be sanitized", claim.Namespace, claim.Name, oldNodeName)
111+
}
112+
100113
claimCopy.Status.Allocation.Devices.Results = sanitizedAllocations
101114
claimCopy.Status.Allocation.NodeSelector = createNodeSelectorSingleNode(newNodeName)
102115
claimCopy.Status.ReservedFor = []resourceapi.ResourceClaimConsumerReference{PodClaimConsumerReference(newOwner)}

‎cluster-autoscaler/simulator/dynamicresources/utils/sanitize_test.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
resourceapi "k8s.io/api/resource/v1"
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/autoscaler/cluster-autoscaler/utils/test"
29+
"k8s.io/utils/ptr"
2930
"k8s.io/utils/set"
3031
)
3132

@@ -322,6 +323,86 @@ func TestSanitizedPodResourceClaims(t *testing.T) {
322323
},
323324
},
324325
}
326+
singleNodeAllocationWithAdminAccessRequest := &resourceapi.AllocationResult{
327+
NodeSelector: &apiv1.NodeSelector{NodeSelectorTerms: []apiv1.NodeSelectorTerm{{
328+
MatchFields: []apiv1.NodeSelectorRequirement{
329+
{Key: "metadata.name", Operator: apiv1.NodeSelectorOpIn, Values: []string{"testNode"}},
330+
}},
331+
}},
332+
Devices: resourceapi.DeviceAllocationResult{
333+
Results: []resourceapi.DeviceRequestAllocationResult{
334+
{Request: "request1", Driver: "driver.foo.com", Pool: "testNodePool1", Device: "device1"},
335+
{Request: "request2", Driver: "driver.foo.com", Pool: "testNodePool1", Device: "device2", AdminAccess: ptr.To(false)},
336+
{Request: "request3", Driver: "driver.foo.com", Pool: "testNodePool2", Device: "device1", AdminAccess: ptr.To(true)},
337+
},
338+
},
339+
}
340+
singleNodeAllocationWithAdminAccessRequestSanitized := &resourceapi.AllocationResult{
341+
NodeSelector: &apiv1.NodeSelector{NodeSelectorTerms: []apiv1.NodeSelectorTerm{{
342+
MatchFields: []apiv1.NodeSelectorRequirement{
343+
{Key: "metadata.name", Operator: apiv1.NodeSelectorOpIn, Values: []string{"newNode"}},
344+
}},
345+
}},
346+
Devices: resourceapi.DeviceAllocationResult{
347+
Results: []resourceapi.DeviceRequestAllocationResult{
348+
{Request: "request1", Driver: "driver.foo.com", Pool: "testNodePool1-abc", Device: "device1"},
349+
{Request: "request2", Driver: "driver.foo.com", Pool: "testNodePool1-abc", Device: "device2", AdminAccess: ptr.To(false)},
350+
{Request: "request3", Driver: "driver.foo.com", Pool: "testNodePool2", Device: "device1", AdminAccess: ptr.To(true)},
351+
},
352+
},
353+
}
354+
singleNodeAllocationOnlyAdminAccess := &resourceapi.AllocationResult{
355+
NodeSelector: &apiv1.NodeSelector{NodeSelectorTerms: []apiv1.NodeSelectorTerm{{
356+
MatchFields: []apiv1.NodeSelectorRequirement{
357+
{Key: "metadata.name", Operator: apiv1.NodeSelectorOpIn, Values: []string{"testNode"}},
358+
}},
359+
}},
360+
Devices: resourceapi.DeviceAllocationResult{
361+
Results: []resourceapi.DeviceRequestAllocationResult{
362+
{Request: "request1", Driver: "driver.foo.com", Pool: "testNodePool1", Device: "device1", AdminAccess: ptr.To(true)},
363+
{Request: "request2", Driver: "driver.foo.com", Pool: "testNodePool2", Device: "device2", AdminAccess: ptr.To(true)},
364+
},
365+
},
366+
}
367+
singleNodeAllocationOnlyAdminAccessSanitized := &resourceapi.AllocationResult{
368+
NodeSelector: &apiv1.NodeSelector{NodeSelectorTerms: []apiv1.NodeSelectorTerm{{
369+
MatchFields: []apiv1.NodeSelectorRequirement{
370+
{Key: "metadata.name", Operator: apiv1.NodeSelectorOpIn, Values: []string{"newNode"}},
371+
}},
372+
}},
373+
Devices: resourceapi.DeviceAllocationResult{
374+
Results: []resourceapi.DeviceRequestAllocationResult{
375+
{Request: "request1", Driver: "driver.foo.com", Pool: "testNodePool1", Device: "device1", AdminAccess: ptr.To(true)},
376+
{Request: "request2", Driver: "driver.foo.com", Pool: "testNodePool2", Device: "device2", AdminAccess: ptr.To(true)},
377+
},
378+
},
379+
}
380+
multipleNodesAllocationOnlyAdminAccess := &resourceapi.AllocationResult{
381+
NodeSelector: &apiv1.NodeSelector{NodeSelectorTerms: []apiv1.NodeSelectorTerm{{
382+
MatchExpressions: []apiv1.NodeSelectorRequirement{
383+
{Key: "someLabel", Operator: apiv1.NodeSelectorOpIn, Values: []string{"val1", "val2"}},
384+
}},
385+
}},
386+
Devices: resourceapi.DeviceAllocationResult{
387+
Results: []resourceapi.DeviceRequestAllocationResult{
388+
{Request: "request1", Driver: "driver.foo.com", Pool: "sharedPool1", Device: "device1", AdminAccess: ptr.To(true)},
389+
{Request: "request2", Driver: "driver.foo.com", Pool: "sharedPool1", Device: "device2", AdminAccess: ptr.To(true)},
390+
},
391+
},
392+
}
393+
multipleNodesAllocationOnlyAdminAccessSanitized := &resourceapi.AllocationResult{
394+
NodeSelector: &apiv1.NodeSelector{NodeSelectorTerms: []apiv1.NodeSelectorTerm{{
395+
MatchFields: []apiv1.NodeSelectorRequirement{
396+
{Key: "metadata.name", Operator: apiv1.NodeSelectorOpIn, Values: []string{"newNode"}},
397+
}},
398+
}},
399+
Devices: resourceapi.DeviceAllocationResult{
400+
Results: []resourceapi.DeviceRequestAllocationResult{
401+
{Request: "request1", Driver: "driver.foo.com", Pool: "sharedPool1", Device: "device1", AdminAccess: ptr.To(true)},
402+
{Request: "request2", Driver: "driver.foo.com", Pool: "sharedPool1", Device: "device2", AdminAccess: ptr.To(true)},
403+
},
404+
},
405+
}
325406

326407
for _, tc := range []struct {
327408
testName string
@@ -416,6 +497,46 @@ func TestSanitizedPodResourceClaims(t *testing.T) {
416497
TestClaimWithPodReservations(TestClaimWithAllocation(newOwnerClaim2, singleNodeAllocationSanitized), newOwner),
417498
},
418499
},
500+
{
501+
testName: "adminAccess pod-owned claims are sanitized",
502+
claims: []*resourceapi.ResourceClaim{
503+
TestClaimWithPodReservations(TestClaimWithAllocation(oldOwnerClaim1, singleNodeAllocationWithAdminAccessRequest), oldOwner),
504+
TestClaimWithPodReservations(TestClaimWithAllocation(oldOwnerClaim2, singleNodeAllocationWithAdminAccessRequest), oldOwner),
505+
},
506+
oldNodeName: "testNode",
507+
newNodeName: "newNode",
508+
oldNodePoolNames: set.New("testNodePool1", "testNodePool2"),
509+
wantClaims: []*resourceapi.ResourceClaim{
510+
TestClaimWithPodReservations(TestClaimWithAllocation(newOwnerClaim1, singleNodeAllocationWithAdminAccessRequestSanitized), newOwner),
511+
TestClaimWithPodReservations(TestClaimWithAllocation(newOwnerClaim2, singleNodeAllocationWithAdminAccessRequestSanitized), newOwner),
512+
},
513+
},
514+
{
515+
testName: "pod with multiple claims, one without AdminAccess and one with only AdminAccess",
516+
claims: []*resourceapi.ResourceClaim{
517+
TestClaimWithPodReservations(TestClaimWithAllocation(oldOwnerClaim1, singleNodeAllocation), oldOwner),
518+
TestClaimWithPodReservations(TestClaimWithAllocation(oldOwnerClaim2, singleNodeAllocationOnlyAdminAccess), oldOwner),
519+
},
520+
oldNodeName: "testNode",
521+
newNodeName: "newNode",
522+
oldNodePoolNames: set.New("testNodePool1", "testNodePool2"),
523+
wantClaims: []*resourceapi.ResourceClaim{
524+
TestClaimWithPodReservations(TestClaimWithAllocation(newOwnerClaim1, singleNodeAllocationSanitized), newOwner),
525+
TestClaimWithPodReservations(TestClaimWithAllocation(newOwnerClaim2, singleNodeAllocationOnlyAdminAccessSanitized), newOwner),
526+
},
527+
},
528+
{
529+
testName: "claim available on multiple nodes with all AdminAccess allocations can be sanitized",
530+
claims: []*resourceapi.ResourceClaim{
531+
TestClaimWithPodReservations(TestClaimWithAllocation(oldOwnerClaim1, multipleNodesAllocationOnlyAdminAccess), oldOwner),
532+
},
533+
oldNodeName: "testNode",
534+
newNodeName: "newNode",
535+
oldNodePoolNames: set.New("testNodePool1", "testNodePool2"),
536+
wantClaims: []*resourceapi.ResourceClaim{
537+
TestClaimWithPodReservations(TestClaimWithAllocation(newOwnerClaim1, multipleNodesAllocationOnlyAdminAccessSanitized), newOwner),
538+
},
539+
},
419540
} {
420541
t.Run(tc.testName, func(t *testing.T) {
421542
claims, err := SanitizedPodResourceClaims(newOwner, oldOwner, tc.claims, nameSuffix, tc.newNodeName, tc.oldNodeName, tc.oldNodePoolNames)

‎cluster-autoscaler/simulator/dynamicresources/utils/utilization.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ import (
2222
v1 "k8s.io/api/core/v1"
2323
resourceapi "k8s.io/api/resource/v1"
2424
"k8s.io/autoscaler/cluster-autoscaler/simulator/framework"
25+
"k8s.io/utils/ptr"
2526
)
2627

2728
// CalculateDynamicResourceUtilization calculates a map of ResourceSlice pool utilization grouped by the driver and pool. Returns
2829
// an error if the NodeInfo doesn't have all ResourceSlices from a pool.
2930
func CalculateDynamicResourceUtilization(nodeInfo *framework.NodeInfo) (map[string]map[string]float64, error) {
3031
result := map[string]map[string]float64{}
3132
claims := nodeInfo.ResourceClaims()
32-
allocatedDevices, err := groupAllocatedDevices(claims)
33+
allocatedDevices, err := groupAllocatedReservedDevices(claims)
3334
if err != nil {
3435
return nil, err
3536
}
@@ -99,9 +100,10 @@ func getAllDevices(slices []*resourceapi.ResourceSlice) []resourceapi.Device {
99100
return devices
100101
}
101102

102-
// groupAllocatedDevices groups the devices from claim allocations by their driver and pool. Returns an error
103-
// if any of the claims isn't allocated.
104-
func groupAllocatedDevices(claims []*resourceapi.ResourceClaim) (map[string]map[string][]string, error) {
103+
// groupAllocatedReservedDevices groups reserved devices from claim allocations by their driver and pool.
104+
// If a deviced will not exclusively reserved (i.e. AdminAccess), it will not be included in the result.
105+
// Returns an error if any of the claims isn't allocated.
106+
func groupAllocatedReservedDevices(claims []*resourceapi.ResourceClaim) (map[string]map[string][]string, error) {
105107
result := map[string]map[string][]string{}
106108
for _, claim := range claims {
107109
alloc := claim.Status.Allocation
@@ -110,6 +112,11 @@ func groupAllocatedDevices(claims []*resourceapi.ResourceClaim) (map[string]map[
110112
}
111113

112114
for _, deviceAlloc := range alloc.Devices.Results {
115+
if ptr.Deref(deviceAlloc.AdminAccess, false) {
116+
// Admin access Device allocations don't actually reserve the Device, so they shouldn't be counted towards utilization.
117+
continue
118+
}
119+
113120
if result[deviceAlloc.Driver] == nil {
114121
result[deviceAlloc.Driver] = map[string][]string{}
115122
}

0 commit comments

Comments
 (0)