Skip to content

Commit 987ad39

Browse files
KobayashiD27KobayashiD27
committed
KEP-5007 DRA Device Binding Conditions: API Related code Update
1 parent 34c3b09 commit 987ad39

File tree

7 files changed

+556
-2
lines changed

7 files changed

+556
-2
lines changed

‎pkg/apis/resource/validation/validation.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ func validateDeviceRequestAllocationResult(result resource.DeviceRequestAllocati
462462
allErrs = append(allErrs, validateDriverName(result.Driver, fldPath.Child("driver"))...)
463463
allErrs = append(allErrs, validatePoolName(result.Pool, fldPath.Child("pool"))...)
464464
allErrs = append(allErrs, validateDeviceName(result.Device, fldPath.Child("device"))...)
465+
allErrs = append(allErrs, validateDeviceBindingParameters(result.BindingConditions, result.BindingFailureConditions, fldPath)...)
465466
return allErrs
466467
}
467468

@@ -788,6 +789,8 @@ func validateDevice(device resource.Device, fldPath *field.Path, sharedCounterTo
788789
} else if (perDeviceNodeSelection == nil || !*perDeviceNodeSelection) && (device.NodeName != nil || device.NodeSelector != nil || device.AllNodes != nil) {
789790
allErrs = append(allErrs, field.Invalid(fldPath, nil, "`nodeName`, `nodeSelector` and `allNodes` can only be set if `perDeviceNodeSelection` is set to true in the ResourceSlice spec"))
790791
}
792+
793+
allErrs = append(allErrs, validateDeviceBindingParameters(device.BindingConditions, device.BindingFailureConditions, fldPath)...)
791794
return allErrs
792795
}
793796

@@ -1192,3 +1195,42 @@ func validateLabelValue(value string, fldPath *field.Path) field.ErrorList {
11921195

11931196
return allErrs
11941197
}
1198+
1199+
func validateDeviceBindingParameters(bindingConditions, bindingFailureConditions []string, fldPath *field.Path) field.ErrorList {
1200+
var allErrs field.ErrorList
1201+
allErrs = append(allErrs, validateSlice(bindingConditions, resource.BindingConditionsMaxSize,
1202+
metav1validation.ValidateLabelName, fldPath.Child("bindingConditions"))...)
1203+
1204+
allErrs = append(allErrs, validateSlice(bindingFailureConditions, resource.BindingFailureConditionsMaxSize,
1205+
metav1validation.ValidateLabelName, fldPath.Child("bindingFailureConditions"))...)
1206+
1207+
// ensure BindingConditions and BindingFailureConditions contain no duplicate items and do not overlap with each other
1208+
conditionsSet := sets.New[string]()
1209+
for i, condition := range bindingConditions {
1210+
if conditionsSet.Has(condition) {
1211+
allErrs = append(allErrs, field.Duplicate(fldPath.Child("bindingConditions").Index(i), condition))
1212+
} else {
1213+
conditionsSet.Insert(condition)
1214+
}
1215+
}
1216+
failureConditionsSet := sets.New[string]()
1217+
for i, condition := range bindingFailureConditions {
1218+
if failureConditionsSet.Has(condition) {
1219+
allErrs = append(allErrs, field.Duplicate(fldPath.Child("bindingFailureConditions").Index(i), condition))
1220+
} else {
1221+
failureConditionsSet.Insert(condition)
1222+
}
1223+
if sets.New(bindingConditions...).Has(condition) {
1224+
allErrs = append(allErrs, field.Invalid(fldPath.Child("bindingFailureConditions").Index(i), condition, "bindingFailureConditions must not overlap with bindingConditions"))
1225+
}
1226+
}
1227+
1228+
if len(bindingConditions) == 0 && len(bindingFailureConditions) > 0 {
1229+
allErrs = append(allErrs, field.Invalid(fldPath.Child("bindingConditions"), bindingConditions, "bindingConditions are required to use bindingFailureConditions"))
1230+
}
1231+
if len(bindingFailureConditions) == 0 && len(bindingConditions) > 0 {
1232+
allErrs = append(allErrs, field.Invalid(fldPath.Child("bindingFailureConditions"), bindingFailureConditions, "bindingFailureConditions are required to use bindingConditions"))
1233+
}
1234+
1235+
return allErrs
1236+
}

‎pkg/apis/resource/validation/validation_resourceclaim_test.go‎

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ var (
9595
},
9696
},
9797
}
98-
validClaim = testClaim(goodName, goodNS, validClaimSpec)
98+
validClaim = testClaim(goodName, goodNS, validClaimSpec)
99+
conditionValidationErrMessage = "name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"
99100
)
100101

101102
func TestValidateClaim(t *testing.T) {
@@ -1701,6 +1702,145 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
17011702
},
17021703
prioritizedListFeatureGate: false,
17031704
},
1705+
"good-binding-conditions": {
1706+
oldClaim: validClaim,
1707+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1708+
claim.Status.Allocation = &resource.AllocationResult{
1709+
Devices: resource.DeviceAllocationResult{
1710+
Results: []resource.DeviceRequestAllocationResult{{
1711+
Request: goodName,
1712+
Driver: goodName,
1713+
Pool: goodName,
1714+
Device: goodName,
1715+
AdminAccess: ptr.To(false),
1716+
BindingConditions: []string{"example.com/condition1", "condition2", "condition3", "condition4"},
1717+
BindingFailureConditions: []string{"example.com/condition5", "condition6", "condition7", "condition8"},
1718+
}},
1719+
},
1720+
}
1721+
return claim
1722+
},
1723+
},
1724+
"too-many-binding-conditions": {
1725+
wantFailures: field.ErrorList{field.TooMany(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("bindingConditions"), 5, 4)},
1726+
oldClaim: validClaim,
1727+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1728+
claim.Status.Allocation = &resource.AllocationResult{
1729+
Devices: resource.DeviceAllocationResult{
1730+
Results: []resource.DeviceRequestAllocationResult{{
1731+
Request: goodName,
1732+
Driver: goodName,
1733+
Pool: goodName,
1734+
Device: goodName,
1735+
AdminAccess: ptr.To(false),
1736+
BindingConditions: []string{"condition1", "condition2", "condition3", "condition4", "condition5"},
1737+
BindingFailureConditions: []string{"condition6", "condition7"},
1738+
}},
1739+
},
1740+
}
1741+
return claim
1742+
},
1743+
},
1744+
"too-many-binding-failure-conditions": {
1745+
wantFailures: field.ErrorList{field.TooMany(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("bindingFailureConditions"), 5, 4)},
1746+
oldClaim: validClaim,
1747+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1748+
claim.Status.Allocation = &resource.AllocationResult{
1749+
Devices: resource.DeviceAllocationResult{
1750+
Results: []resource.DeviceRequestAllocationResult{{
1751+
Request: goodName,
1752+
Driver: goodName,
1753+
Pool: goodName,
1754+
Device: goodName,
1755+
AdminAccess: ptr.To(false),
1756+
BindingConditions: []string{"condition1", "condition2"},
1757+
BindingFailureConditions: []string{"condition3", "condition4", "condition5", "condition6", "condition7"},
1758+
}},
1759+
},
1760+
}
1761+
return claim
1762+
},
1763+
},
1764+
"invalid-binding-conditions": {
1765+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("bindingConditions").Index(1), "condition2!", conditionValidationErrMessage)},
1766+
oldClaim: validClaim,
1767+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1768+
claim.Status.Allocation = &resource.AllocationResult{
1769+
Devices: resource.DeviceAllocationResult{
1770+
Results: []resource.DeviceRequestAllocationResult{{
1771+
Request: goodName,
1772+
Driver: goodName,
1773+
Pool: goodName,
1774+
Device: goodName,
1775+
AdminAccess: ptr.To(false),
1776+
BindingConditions: []string{"condition1", "condition2!"},
1777+
BindingFailureConditions: []string{"condition3", "condition4"},
1778+
}},
1779+
},
1780+
}
1781+
return claim
1782+
},
1783+
},
1784+
"invalid-binding-failure-conditions": {
1785+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("bindingFailureConditions").Index(1), "condition4!", conditionValidationErrMessage)},
1786+
oldClaim: validClaim,
1787+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1788+
claim.Status.Allocation = &resource.AllocationResult{
1789+
Devices: resource.DeviceAllocationResult{
1790+
Results: []resource.DeviceRequestAllocationResult{{
1791+
Request: goodName,
1792+
Driver: goodName,
1793+
Pool: goodName,
1794+
Device: goodName,
1795+
AdminAccess: ptr.To(false),
1796+
BindingConditions: []string{"condition1", "condition2"},
1797+
BindingFailureConditions: []string{"condition3", "condition4!"},
1798+
}},
1799+
},
1800+
}
1801+
return claim
1802+
},
1803+
},
1804+
"lacking-binding-conditions-claim-has-binding-failure-conditions": {
1805+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("bindingConditions"), []string(nil), "bindingConditions are required to use bindingFailureConditions")},
1806+
oldClaim: validClaim,
1807+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1808+
claim.Status.Allocation = &resource.AllocationResult{
1809+
Devices: resource.DeviceAllocationResult{
1810+
Results: []resource.DeviceRequestAllocationResult{{
1811+
Request: goodName,
1812+
Driver: goodName,
1813+
Pool: goodName,
1814+
Device: goodName,
1815+
AdminAccess: ptr.To(false),
1816+
BindingConditions: nil,
1817+
BindingFailureConditions: []string{"condition3", "condition4"},
1818+
}},
1819+
},
1820+
}
1821+
return claim
1822+
},
1823+
},
1824+
"lacking-binding-failure-conditions-claim-has-binding-conditions": {
1825+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("status", "allocation", "devices", "results").Index(0).Child("bindingFailureConditions"), []string(nil), "bindingFailureConditions are required to use bindingConditions")},
1826+
oldClaim: validClaim,
1827+
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
1828+
claim.Status.Allocation = &resource.AllocationResult{
1829+
Devices: resource.DeviceAllocationResult{
1830+
Results: []resource.DeviceRequestAllocationResult{{
1831+
Request: goodName,
1832+
Driver: goodName,
1833+
Pool: goodName,
1834+
Device: goodName,
1835+
AdminAccess: ptr.To(false),
1836+
BindingConditions: []string{"condition1", "condition2"},
1837+
BindingFailureConditions: nil,
1838+
}},
1839+
},
1840+
}
1841+
return claim
1842+
},
1843+
},
17041844
}
17051845

17061846
for name, scenario := range scenarios {

‎pkg/apis/resource/validation/validation_resourceslice_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@ func testResourceSlice(name, nodeName, driverName string, numDevices int) *resou
7777
return slice
7878
}
7979

80+
func testResourceSliceWithBindingConditions(name, nodeName, driverName string, numDevices int, bindingConditions, bindingFailureConditions []string) *resourceapi.ResourceSlice {
81+
slice := testResourceSlice(name, nodeName, driverName, numDevices)
82+
for i := range slice.Spec.Devices {
83+
slice.Spec.Devices[i].BindingConditions = bindingConditions
84+
slice.Spec.Devices[i].BindingFailureConditions = bindingFailureConditions
85+
}
86+
return slice
87+
}
88+
8089
func TestValidateResourceSlice(t *testing.T) {
8190
goodName := "foo"
8291
badName := "!@#$%^"
@@ -779,6 +788,45 @@ func TestValidateResourceSlice(t *testing.T) {
779788
return slice
780789
}(),
781790
},
791+
"good-binding-conditions": {
792+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"example.com/condition1", "condition2"}, []string{"example.com/condition3", "condition4"}),
793+
},
794+
"too-many-binding-conditions": {
795+
wantFailures: field.ErrorList{field.TooMany(field.NewPath("spec", "devices").Index(0).Child("bindingConditions"), resourceapi.BindingConditionsMaxSize+1, resourceapi.BindingConditionsMaxSize)},
796+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2", "condition3", "condition4", "condition5"}, []string{"condition6", "condition7"}),
797+
},
798+
"too-many-binding-failure-conditions": {
799+
wantFailures: field.ErrorList{field.TooMany(field.NewPath("spec", "devices").Index(0).Child("bindingFailureConditions"), resourceapi.BindingConditionsMaxSize+1, resourceapi.BindingConditionsMaxSize)},
800+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2"}, []string{"condition3", "condition4", "condition5", "condition6", "condition7"}),
801+
},
802+
"invalid-binding-conditions": {
803+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices").Index(0).Child("bindingConditions").Index(1), "condition2!", conditionValidationErrMessage)},
804+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2!"}, []string{"condition3", "condition4"}),
805+
},
806+
"invalid-binding-failure-conditions": {
807+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices").Index(0).Child("bindingFailureConditions").Index(0), "condition3!", conditionValidationErrMessage)},
808+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2"}, []string{"condition3!", "condition4"}),
809+
},
810+
"invalid-slice-has-binding-failure-but-no-binding-conditions": {
811+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices").Index(0).Child("bindingConditions"), []string(nil), "bindingConditions are required to use bindingFailureConditions")},
812+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, nil, []string{"condition1", "condition2"}),
813+
},
814+
"invalid-slice-has-binding-conditions-but-no-binding-failure-conditions": {
815+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices").Index(0).Child("bindingFailureConditions"), []string(nil), "bindingFailureConditions are required to use bindingConditions")},
816+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2"}, nil),
817+
},
818+
"duplicate-binding-conditions": {
819+
wantFailures: field.ErrorList{field.Duplicate(field.NewPath("spec", "devices").Index(0).Child("bindingConditions").Index(1), "condition1")},
820+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition1"}, []string{"condition2", "condition3"}),
821+
},
822+
"duplicate-binding-failure-conditions": {
823+
wantFailures: field.ErrorList{field.Duplicate(field.NewPath("spec", "devices").Index(0).Child("bindingFailureConditions").Index(1), "condition3")},
824+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2"}, []string{"condition3", "condition3"}),
825+
},
826+
"overlapping-binding-conditions": {
827+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "devices").Index(0).Child("bindingFailureConditions").Index(0), "condition1", "bindingFailureConditions must not overlap with bindingConditions")},
828+
slice: testResourceSliceWithBindingConditions(goodName, goodName, driverName, 1, []string{"condition1", "condition2"}, []string{"condition1", "condition3"}),
829+
},
782830
}
783831

784832
for name, scenario := range scenarios {

‎pkg/registry/resource/resourceclaim/strategy.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ func dropDisabledFields(newClaim, oldClaim *resource.ResourceClaim) {
220220
dropDisabledDRAPrioritizedListFields(newClaim, oldClaim)
221221
dropDisabledDRAAdminAccessFields(newClaim, oldClaim)
222222
dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim)
223+
dropDisabledDRADeviceBindingConditionsFields(newClaim, oldClaim)
223224
}
224225

225226
func dropDisabledDRAPrioritizedListFields(newClaim, oldClaim *resource.ResourceClaim) {
@@ -351,3 +352,39 @@ func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) {
351352
}
352353

353354
// TODO: add tests after partitionable devices is merged (code conflict!)
355+
356+
// dropDisabledDRADeviceBindingConditionsFields removes fields which are covered by a feature gate.
357+
func dropDisabledDRADeviceBindingConditionsFields(newClaim, oldClaim *resource.ResourceClaim) {
358+
if utilfeature.DefaultFeatureGate.Enabled(features.DRADeviceBindingConditions) && utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) ||
359+
draBindingConditionsFeatureInUse(oldClaim) {
360+
return
361+
}
362+
363+
if newClaim.Status.Allocation == nil {
364+
return
365+
}
366+
newClaim.Status.Allocation.AllocationTimestamp = nil
367+
368+
for i := range newClaim.Status.Allocation.Devices.Results {
369+
newClaim.Status.Allocation.Devices.Results[i].BindingConditions = nil
370+
newClaim.Status.Allocation.Devices.Results[i].BindingFailureConditions = nil
371+
}
372+
}
373+
374+
func draBindingConditionsFeatureInUse(claim *resource.ResourceClaim) bool {
375+
if claim == nil || claim.Status.Allocation == nil {
376+
return false
377+
}
378+
379+
if claim.Status.Allocation.AllocationTimestamp != nil {
380+
return true
381+
}
382+
383+
for _, result := range claim.Status.Allocation.Devices.Results {
384+
if len(result.BindingConditions) != 0 || len(result.BindingFailureConditions) != 0 {
385+
return true
386+
}
387+
}
388+
389+
return false
390+
}

0 commit comments

Comments
 (0)