From 3883e050fc13f0510e23def09234585ffc67f0ba Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Sat, 12 Apr 2025 17:02:36 -0500 Subject: [PATCH 1/6] Use shared test fixtures for ResourceSlice Tracker test cases This scaffolds almost all of the objects used in tests outside of the test cases themselves to make the test cases a bit easier to grok at a glance. These changes don't perfectly preserve the old test cases bit-for-bit, but the same meaningful chunks of functionality should still be covered. The other main benefit is that the checks against emitted events are now verifying against entire objects instead of being more sparsely tested. --- .../resourceslice/tracker/tracker_test.go | 1532 +++++------------ 1 file changed, 412 insertions(+), 1120 deletions(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go index 1e5feca7cea..331eb7016b6 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go @@ -125,9 +125,189 @@ func inputEventGeneratorForTest(ctx context.Context, t *testing.T, tracker *Trac } } -func TestListPatchedResourceSlices(t *testing.T) { - now, _ := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") +var ( + now, _ = time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") + driver1 = "driver1.example.com" + driver2 = "driver2.example.com" + pool1 = "pool-1" + pool2 = "pool-2" + device0Name = "device-0" + device1Name = "device-1" + device2Name = "device-2" + deviceClass1 = &resourceapi.DeviceClass{ + ObjectMeta: metav1.ObjectMeta{Name: "device-class-1"}, + Spec: resourceapi.DeviceClassSpec{ + Selectors: []resourceapi.DeviceSelector{ + { + CEL: &resourceapi.CELDeviceSelector{ + Expression: `device.driver == "` + driver1 + `"`, + }, + }, + }, + }, + } + + sliceWithDevices = func(slice *resourceapi.ResourceSlice, devices []resourceapi.Device) *resourceapi.ResourceSlice { + slice = slice.DeepCopy() + slice.Spec.Devices = devices + return slice + } + slice1NoDevices = &resourceapi.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: "s1", + }, + Spec: resourceapi.ResourceSliceSpec{ + Driver: driver1, + Pool: resourceapi.ResourcePool{ + Name: pool1, + }, + }, + } + slice2NoDevices = &resourceapi.ResourceSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: "s2", + }, + Spec: resourceapi.ResourceSliceSpec{ + Driver: driver2, + Pool: resourceapi.ResourcePool{ + Name: pool2, + }, + }, + } + unchangedSlice = &resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "no-change"}} + + deviceWithName = func(device resourceapi.Device, name string) resourceapi.Device { + device.Name = name + return device + } + deviceWithTaints = func(device resourceapi.Device, taints []resourceapi.DeviceTaint) resourceapi.Device { + if device.Basic != nil { + device.Basic = device.Basic.DeepCopy() + device.Basic.Taints = taints + } + return device + } + emptyDevice = resourceapi.Device{Basic: &resourceapi.BasicDevice{}} + device0 = deviceWithName(emptyDevice, device0Name) + device1 = deviceWithName(emptyDevice, device1Name) + device2 = deviceWithName(emptyDevice, device2Name) + + deviceTaint1 = resourceapi.DeviceTaint{ + Key: "example.com/taint", + Value: "tainted", + Effect: resourceapi.DeviceTaintEffectNoExecute, + TimeAdded: &metav1.Time{Time: now}, + } + deviceTaint2 = resourceapi.DeviceTaint{ + Key: "example.com/taint2", + Value: "tainted2", + Effect: resourceapi.DeviceTaintEffectNoExecute, + TimeAdded: &metav1.Time{Time: now}, + } + deviceTaints = []resourceapi.DeviceTaint{deviceTaint1} + device1Tainted = deviceWithTaints(device1, deviceTaints) + device2Tainted = deviceWithTaints(device2, deviceTaints) + devices = []resourceapi.Device{device1} + threeDevices = []resourceapi.Device{ + device0, + device1, + device2, + } + threeDevicesOneTainted = []resourceapi.Device{ + device0, + device1Tainted, + device2, + } + devices2 = []resourceapi.Device{device2} + taintedDevices = []resourceapi.Device{device1Tainted} + taintedDevices2 = []resourceapi.Device{device2Tainted} + + existingDeviceTaints = []resourceapi.DeviceTaint{deviceTaint2} + existingDevice1Tainted = deviceWithTaints(device1, existingDeviceTaints) + existingTaintedDevices = []resourceapi.Device{existingDevice1Tainted} + mergedDeviceTaints = []resourceapi.DeviceTaint{deviceTaint2, deviceTaint1} + mergedDevice1Tainted = deviceWithTaints(device1, mergedDeviceTaints) + mergedTaintedDevices = []resourceapi.Device{mergedDevice1Tainted} + + slice1 = sliceWithDevices(slice1NoDevices, devices) + slice1Tainted = sliceWithDevices(slice1, taintedDevices) + slice1AlreadyTainted = sliceWithDevices(slice1, existingTaintedDevices) + slice1MergedTaints = sliceWithDevices(slice1, mergedTaintedDevices) + slice2 = sliceWithDevices(slice2NoDevices, devices2) + slice2Tainted = sliceWithDevices(slice2, taintedDevices2) + + alphaDeviceTaint = func(taint resourceapi.DeviceTaint) resourcealphaapi.DeviceTaint { + return resourcealphaapi.DeviceTaint{ + Key: taint.Key, + Value: taint.Value, + Effect: resourcealphaapi.DeviceTaintEffect(taint.Effect), + TimeAdded: taint.TimeAdded, + } + } + taintAllDevicesRule = &resourcealphaapi.DeviceTaintRule{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rule", + }, + Spec: resourcealphaapi.DeviceTaintRuleSpec{ + Taint: alphaDeviceTaint(deviceTaint1), + }, + } + taintPoolDevicesRule = func(rule *resourcealphaapi.DeviceTaintRule, pool string) *resourcealphaapi.DeviceTaintRule { + rule = rule.DeepCopy() + rule.Spec.DeviceSelector = &resourcealphaapi.DeviceTaintSelector{ + Pool: &pool, + } + return rule + } + taintDriverDevicesRule = func(rule *resourcealphaapi.DeviceTaintRule, driver string) *resourcealphaapi.DeviceTaintRule { + rule = rule.DeepCopy() + rule.Spec.DeviceSelector = &resourcealphaapi.DeviceTaintSelector{ + Driver: &driver, + } + return rule + } + taintNamedDevicesRule = func(rule *resourcealphaapi.DeviceTaintRule, name string) *resourcealphaapi.DeviceTaintRule { + rule = rule.DeepCopy() + rule.Spec.DeviceSelector = &resourcealphaapi.DeviceTaintSelector{ + Device: &name, + } + return rule + } + taintCELSelectedDevicesRule = func(rule *resourcealphaapi.DeviceTaintRule, exprs ...string) *resourcealphaapi.DeviceTaintRule { + rule = rule.DeepCopy() + var selectors []resourcealphaapi.DeviceSelector + for _, expr := range exprs { + selectors = append(selectors, resourcealphaapi.DeviceSelector{ + CEL: &resourcealphaapi.CELDeviceSelector{ + Expression: expr, + }, + }) + } + rule.Spec.DeviceSelector = &resourcealphaapi.DeviceTaintSelector{ + Selectors: selectors, + } + return rule + } + taintDeviceClassRule = func(rule *resourcealphaapi.DeviceTaintRule, deviceClassName string) *resourcealphaapi.DeviceTaintRule { + rule = rule.DeepCopy() + rule.Spec.DeviceSelector = &resourcealphaapi.DeviceTaintSelector{ + DeviceClassName: &deviceClassName, + } + return rule + } + taintPool1DevicesRule = taintPoolDevicesRule(taintAllDevicesRule, pool1) + taintPool2DevicesRule = taintPoolDevicesRule(taintAllDevicesRule, pool2) + taintDriver1DevicesRule = taintDriverDevicesRule(taintAllDevicesRule, driver1) + taintDevice1Rule = taintNamedDevicesRule(taintAllDevicesRule, device1Name) + taintDriver1DevicesCELRule = taintCELSelectedDevicesRule(taintAllDevicesRule, `device.driver == "`+driver1+`"`) + taintNoDevicesCELRule = taintCELSelectedDevicesRule(taintAllDevicesRule, `true`, `false`, `true`) + taintNoDevicesCELRuntimeErrorRule = taintCELSelectedDevicesRule(taintAllDevicesRule, `device.attributes["test.example.com"].deviceAttr`) + taintNoDevicesInvalidCELRule = taintCELSelectedDevicesRule(taintAllDevicesRule, `invalid`) + taintDeviceClass1Rule = taintDeviceClassRule(taintAllDevicesRule, deviceClass1.Name) +) + +func TestListPatchedResourceSlices(t *testing.T) { tests := map[string]struct { deviceTaintsDisabled bool inputEvents func(event inputEventGenerator) @@ -138,918 +318,277 @@ func TestListPatchedResourceSlices(t *testing.T) { }{ "add-slices-no-patches": { inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "s1"}}) - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "s2"}}) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - {ObjectMeta: metav1.ObjectMeta{Name: "s1"}}, - {ObjectMeta: metav1.ObjectMeta{Name: "s2"}}, + slice1, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "s1", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "s2", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "update-slices-no-patches": { inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "s1", - }, - Spec: resourceapi.ResourceSliceSpec{ - // no devices - Devices: nil, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "s2", - }, - Spec: resourceapi.ResourceSliceSpec{ - // no devices - Devices: nil, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "no-change"}}) + event.addResourceSlice(slice1NoDevices) + event.addResourceSlice(slice2NoDevices) + event.addResourceSlice(unchangedSlice) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "s1", - }, - Spec: resourceapi.ResourceSliceSpec{ - // devices! - Devices: []resourceapi.Device{ - {Basic: &resourceapi.BasicDevice{}}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "s2", - }, - Spec: resourceapi.ResourceSliceSpec{ - // devices! - Devices: []resourceapi.Device{ - {Basic: &resourceapi.BasicDevice{}}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "no-change"}}) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) + event.addResourceSlice(unchangedSlice) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "s1", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - {Basic: &resourceapi.BasicDevice{}}, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "s2", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - {Basic: &resourceapi.BasicDevice{}}, - }, - }, - }, - {ObjectMeta: metav1.ObjectMeta{Name: "no-change"}}, + slice1, + slice2, + unchangedSlice, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 5) { - return - } - // The first events are adds. - assert.Equal(t, handlerEventUpdate, events[3].event) - assert.Equal(t, "s1", events[3].newObj.Name) - assert.Equal(t, "s1", events[3].oldObj.Name) - assert.Nil(t, events[3].oldObj.Spec.Devices) - assert.NotNil(t, events[3].newObj.Spec.Devices) - assert.Equal(t, handlerEventUpdate, events[4].event) - assert.Equal(t, "s2", events[4].newObj.Name) - assert.Equal(t, "s2", events[4].oldObj.Name) - assert.Nil(t, events[4].oldObj.Spec.Devices) - assert.NotNil(t, events[4].newObj.Spec.Devices) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1NoDevices}, + {event: handlerEventAdd, newObj: slice2NoDevices}, + {event: handlerEventAdd, newObj: unchangedSlice}, + {event: handlerEventUpdate, oldObj: slice1NoDevices, newObj: slice1}, + {event: handlerEventUpdate, oldObj: slice2NoDevices, newObj: slice2}, + }, + events, + ) }, }, "delete-slices": { inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "s1"}}) - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "s2"}}) - event.addResourceSlice(&resourceapi.ResourceSlice{ObjectMeta: metav1.ObjectMeta{Name: "keep-me"}}) - event.deleteResourceSlice("s1") - event.deleteResourceSlice("s2") + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) + event.addResourceSlice(unchangedSlice) + event.deleteResourceSlice(slice1.Name) + event.deleteResourceSlice(slice2.Name) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - {ObjectMeta: metav1.ObjectMeta{Name: "keep-me"}}, + unchangedSlice, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 5) { - return - } - // The first events are adds. - assert.Equal(t, handlerEventDelete, events[3].event) - assert.Equal(t, "s1", events[3].oldObj.Name) - assert.Equal(t, handlerEventDelete, events[4].event) - assert.Equal(t, "s2", events[4].oldObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2}, + {event: handlerEventAdd, newObj: unchangedSlice}, + {event: handlerEventDelete, oldObj: slice1}, + {event: handlerEventDelete, oldObj: slice2}, + }, + events, + ) }, }, "patch-all-slices": { inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "all-slices", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: nil, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) + event.addResourceSlice(slice1) + event.addDeviceTaintRule(taintAllDevicesRule) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, + slice1Tainted, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) - assert.Equal(t, handlerEventUpdate, events[1].event) - assert.Equal(t, "slice", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventUpdate, oldObj: slice1, newObj: slice1Tainted}, + }, + events, + ) }, }, "update-patch": { inputEvents: func(event inputEventGenerator) { - taintRule := &resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "taintRule", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Pool: ptr.To("pool-1"), - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - } - event.addDeviceTaintRule(taintRule.DeepCopy()) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice-1", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool-1", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice-2", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool-2", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - taintRule.Spec.DeviceSelector.Pool = ptr.To("pool-2") - event.addDeviceTaintRule(taintRule) + event.addDeviceTaintRule(taintPool1DevicesRule) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) + event.addDeviceTaintRule(taintPool2DevicesRule) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice-1", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool-1", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice-2", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool-2", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, + slice1, + slice2Tainted, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 4) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice-1", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "slice-2", events[1].newObj.Name) - - assert.Equal(t, handlerEventUpdate, events[2].event) - assert.Equal(t, handlerEventUpdate, events[3].event) - assert.ElementsMatch(t, []string{"slice-1", "slice-2"}, []string{events[2].newObj.Name, events[3].newObj.Name}) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events[:2], + ) + // The remaining events may come in any order + assert.ElementsMatch( + t, + []handlerEvent{ + {event: handlerEventUpdate, oldObj: slice1Tainted, newObj: slice1}, + {event: handlerEventUpdate, oldObj: slice2, newObj: slice2Tainted}, + }, + events[2:], + ) }, }, "merge-taints": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "merge", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: nil, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint2", - Value: "tainted2", - Effect: resourceapi.DeviceTaintEffectNoSchedule, - }}, - }, - }, - }, - }, - }) + event.addDeviceTaintRule(taintAllDevicesRule) + event.addResourceSlice(slice1AlreadyTainted) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{ - { - Key: "example.com/taint2", - Value: "tainted2", - Effect: resourceapi.DeviceTaintEffectNoSchedule, - }, - { - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }, - }, - }, - }, - }, + slice1MergedTaints, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 1) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1MergedTaints}, + }, + events, + ) }, }, "add-taint-for-driver": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "driver", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Driver: ptr.To("test.example.com"), - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceTaintRule(taintDriver1DevicesRule) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1Tainted, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "wrong-driver", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "add-taint-for-pool": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pool", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Pool: ptr.To("pool"), - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-pool", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "other", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceTaintRule(taintPool1DevicesRule) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-pool", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "other", - }, - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1Tainted, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "wrong-pool", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "add-taint-for-device": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "device", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Device: ptr.To("device"), - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool", - }, - Devices: []resourceapi.Device{ - { - Name: "device", - Basic: &resourceapi.BasicDevice{}, - }, - { - Name: "wrong-device", - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceTaintRule(taintDevice1Rule) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Pool: resourceapi.ResourcePool{ - Name: "pool", - }, - Devices: []resourceapi.Device{ - { - Name: "device", - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - { - Name: "wrong-device", - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1Tainted, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 1) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "add-attribute-for-selector": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "selector", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Selectors: []resourcealphaapi.DeviceSelector{ - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `device.driver == "test.example.com"`, - }, - }, - }, - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceTaintRule(taintDriver1DevicesCELRule) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1Tainted, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "wrong-driver", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "selector-does-not-match": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "selector", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Selectors: []resourcealphaapi.DeviceSelector{ - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `true`, - }, - }, - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `false`, - }, - }, - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `true`, - }, - }, - }, - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceTaintRule(taintNoDevicesCELRule) + event.addResourceSlice(slice1) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 1) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + }, + events, + ) }, }, "runtime-CEL-errors-skip-devices": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "selector", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Selectors: []resourcealphaapi.DeviceSelector{ - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `device.attributes["test.example.com"].deviceAttr`, - }, - }, - }, - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }) + event.addDeviceTaintRule(taintNoDevicesCELRuntimeErrorRule) + event.addResourceSlice(slice1) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, + slice1, }, expectEvents: func(t *assert.CollectT, events *v1.EventList) { if !assert.Len(t, events.Items, 1) { return } - assert.Equal(t, "selector", events.Items[0].InvolvedObject.Name) + assert.Equal(t, taintNoDevicesCELRuntimeErrorRule.Name, events.Items[0].InvolvedObject.Name) assert.Equal(t, "CELRuntimeError", events.Items[0].Reason) }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 1) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + }, + events, + ) }, }, "invalid-CEL-expression-throws-error": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "selector", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Selectors: []resourcealphaapi.DeviceSelector{ - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `invalid`, - }, - }, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceTaintRule(taintNoDevicesInvalidCELRule) + event.addResourceSlice(slice1) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{}, expectUnhandledErrors: func(t *testing.T, errs []error) { @@ -1061,335 +600,88 @@ func TestListPatchedResourceSlices(t *testing.T) { }, "add-taint-for-device-class": { inputEvents: func(event inputEventGenerator) { - event.addDeviceClass(&resourceapi.DeviceClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "class.example.com", - }, - Spec: resourceapi.DeviceClassSpec{ - Selectors: []resourceapi.DeviceSelector{ - { - CEL: &resourceapi.CELDeviceSelector{ - Expression: `device.driver == "test.example.com"`, - }, - }, - }, - }, - }) - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "device-class", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - DeviceClassName: ptr.To("class.example.com"), - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceClass(deviceClass1) + event.addDeviceTaintRule(taintDeviceClass1Rule) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1Tainted, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "wrong-driver", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "filter-all-criteria": { inputEvents: func(event inputEventGenerator) { - event.addDeviceClass(&resourceapi.DeviceClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "class.example.com", - }, - Spec: resourceapi.DeviceClassSpec{ - Selectors: []resourceapi.DeviceSelector{ - { - CEL: &resourceapi.CELDeviceSelector{ - Expression: `device.driver == "test.example.com"`, - }, - }, - }, - }, - }) - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "all-criteria", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - DeviceClassName: ptr.To("class.example.com"), - Driver: ptr.To("test.example.com"), - Pool: ptr.To("pool"), - Device: ptr.To("device"), - Selectors: []resourcealphaapi.DeviceSelector{ - { - CEL: &resourcealphaapi.CELDeviceSelector{ - Expression: `true`, - }, - }, - }, - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Pool: resourceapi.ResourcePool{ - Name: "pool", - }, - Devices: []resourceapi.Device{ - { - Name: "device", - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) - event.addResourceSlice(&resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }) + event.addDeviceClass(deviceClass1) + event.addDeviceTaintRule( + taintDeviceClassRule( + taintDriverDevicesRule( + taintPoolDevicesRule( + taintNamedDevicesRule( + taintCELSelectedDevicesRule( + taintAllDevicesRule, + `true`, + ), + device1Name, + ), + pool1, + ), + driver1, + ), + deviceClass1.Name, + ), + ) + event.addResourceSlice(slice1) + event.addResourceSlice(slice2) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "slice", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "test.example.com", - Pool: resourceapi.ResourcePool{ - Name: "pool", - }, - Devices: []resourceapi.Device{ - { - Name: "device", - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "wrong-driver", - }, - Spec: resourceapi.ResourceSliceSpec{ - Driver: "wrong.example.com", - Devices: []resourceapi.Device{ - { - Basic: &resourceapi.BasicDevice{}, - }, - }, - }, - }, + slice1Tainted, + slice2, }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 2) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "slice", events[0].newObj.Name) - assert.Equal(t, handlerEventAdd, events[1].event) - assert.Equal(t, "wrong-driver", events[1].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, + }, + events, + ) }, }, "update-patched-slice": { inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(&resourcealphaapi.DeviceTaintRule{ - ObjectMeta: metav1.ObjectMeta{ - Name: "all-slices", - }, - Spec: resourcealphaapi.DeviceTaintRuleSpec{ - DeviceSelector: &resourcealphaapi.DeviceTaintSelector{ - Device: ptr.To("device-1"), - }, - Taint: resourcealphaapi.DeviceTaint{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourcealphaapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }, - }, - }) - oneDevice := []resourceapi.Device{ - {Name: "device-1", Basic: &resourceapi.BasicDevice{}}, - } - threeDevices := []resourceapi.Device{ - {Name: "device-0", Basic: &resourceapi.BasicDevice{}}, - {Name: "device-1", Basic: &resourceapi.BasicDevice{}}, - {Name: "device-2", Basic: &resourceapi.BasicDevice{}}, - } - devicesAdded := &resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "devices-added", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: oneDevice, - }, - } - devicesRemoved := &resourceapi.ResourceSlice{ - ObjectMeta: metav1.ObjectMeta{ - Name: "devices-removed", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: threeDevices, - }, - } - event.addResourceSlice(devicesAdded.DeepCopy()) - devicesAdded.Spec.Devices = threeDevices - event.addResourceSlice(devicesAdded) - event.addResourceSlice(devicesRemoved.DeepCopy()) - devicesRemoved.Spec.Devices = oneDevice - event.addResourceSlice(devicesRemoved) + event.addDeviceTaintRule(taintDevice1Rule) + event.addResourceSlice(slice1) + event.addResourceSlice(sliceWithDevices(slice1, threeDevices)) + event.addResourceSlice(sliceWithDevices(slice2, threeDevices)) + event.addResourceSlice(sliceWithDevices(slice2, devices)) }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "devices-added", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - {Name: "device-0", Basic: &resourceapi.BasicDevice{}}, - { - Name: "device-1", - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - {Name: "device-2", Basic: &resourceapi.BasicDevice{}}, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "devices-removed", - }, - Spec: resourceapi.ResourceSliceSpec{ - Devices: []resourceapi.Device{ - { - Name: "device-1", - Basic: &resourceapi.BasicDevice{ - Taints: []resourceapi.DeviceTaint{{ - Key: "example.com/taint", - Value: "tainted", - Effect: resourceapi.DeviceTaintEffectNoExecute, - TimeAdded: &metav1.Time{Time: now}, - }}, - }, - }, - }, - }, - }, + sliceWithDevices(slice1, threeDevicesOneTainted), + sliceWithDevices(slice2, taintedDevices), }, expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - if !assert.Len(t, events, 4) { - return - } - assert.Equal(t, handlerEventAdd, events[0].event) - assert.Equal(t, "devices-added", events[0].newObj.Name) - assert.Equal(t, handlerEventUpdate, events[1].event) - assert.Equal(t, "devices-added", events[1].newObj.Name) - assert.Equal(t, handlerEventAdd, events[2].event) - assert.Equal(t, "devices-removed", events[2].newObj.Name) - assert.Equal(t, handlerEventUpdate, events[3].event) - assert.Equal(t, "devices-removed", events[3].newObj.Name) + assert.Equal( + t, + []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventUpdate, oldObj: slice1Tainted, newObj: sliceWithDevices(slice1, threeDevicesOneTainted)}, + {event: handlerEventAdd, newObj: sliceWithDevices(slice2, threeDevicesOneTainted)}, + {event: handlerEventUpdate, oldObj: sliceWithDevices(slice2, threeDevicesOneTainted), newObj: sliceWithDevices(slice2, taintedDevices)}, + }, + events, + ) }, }, } From 45b0ddff12c3ce849e34ae93e08f0636f744f70d Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Sat, 12 Apr 2025 17:57:56 -0500 Subject: [PATCH 2/6] make input events a slice --- .../resourceslice/tracker/tracker_test.go | 310 ++++++++++-------- 1 file changed, 169 insertions(+), 141 deletions(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go index 331eb7016b6..89a369c2336 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go @@ -54,77 +54,104 @@ type handlerEvent struct { newObj *resourceapi.ResourceSlice } -type inputEventGenerator struct { - addResourceSlice func(slice *resourceapi.ResourceSlice) - deleteResourceSlice func(name string) - addDeviceTaintRule func(taintRule *resourcealphaapi.DeviceTaintRule) - deleteDeviceTaintRule func(name string) - addDeviceClass func(class *resourceapi.DeviceClass) - deleteDeviceClass func(name string) +func add[T any](obj *T) [2]*T { + return [2]*T{nil, obj} } -func inputEventGeneratorForTest(ctx context.Context, t *testing.T, tracker *Tracker) inputEventGenerator { - return inputEventGenerator{ - addResourceSlice: func(slice *resourceapi.ResourceSlice) { - oldObj, exists, err := tracker.resourceSlices.GetIndexer().Get(slice) - require.NoError(t, err) - err = tracker.resourceSlices.GetIndexer().Add(slice) - require.NoError(t, err) - if !exists { - tracker.resourceSliceAdd(ctx)(slice) - } else if !apiequality.Semantic.DeepEqual(oldObj, slice) { - tracker.resourceSliceUpdate(ctx)(oldObj, slice) - } - }, - deleteResourceSlice: func(name string) { - oldObj, exists, err := tracker.resourceSlices.GetIndexer().GetByKey(name) - require.NoError(t, err) - require.True(t, exists, "deleting resource slice that was never created") - err = tracker.resourceSlices.GetIndexer().Delete(oldObj) - require.NoError(t, err) - tracker.resourceSliceDelete(ctx)(oldObj) - }, - addDeviceTaintRule: func(taintRule *resourcealphaapi.DeviceTaintRule) { - oldObj, exists, err := tracker.deviceTaints.GetIndexer().Get(taintRule) - require.NoError(t, err) - err = tracker.deviceTaints.GetIndexer().Add(taintRule) - require.NoError(t, err) - if !exists { - tracker.deviceTaintAdd(ctx)(taintRule) - } else if !apiequality.Semantic.DeepEqual(oldObj, taintRule) { - tracker.deviceTaintUpdate(ctx)(oldObj, taintRule) - } - }, - deleteDeviceTaintRule: func(name string) { - oldObj, exists, err := tracker.deviceTaints.GetIndexer().GetByKey(name) - require.NoError(t, err) - require.True(t, exists, "deleting DeviceTaintRule that was never created") - err = tracker.deviceTaints.GetIndexer().Delete(oldObj) - require.NoError(t, err) - tracker.deviceTaintDelete(ctx)(oldObj) - }, - addDeviceClass: func(class *resourceapi.DeviceClass) { - oldObj, exists, err := tracker.deviceClasses.GetIndexer().Get(class) - require.NoError(t, err) - err = tracker.deviceClasses.GetIndexer().Add(class) - require.NoError(t, err) - if !exists { - tracker.deviceClassAdd(ctx)(class) - } else if !apiequality.Semantic.DeepEqual(oldObj, class) { - tracker.deviceClassUpdate(ctx)(oldObj, class) - } - }, - deleteDeviceClass: func(name string) { - oldObj, exists, err := tracker.deviceClasses.GetIndexer().GetByKey(name) - require.NoError(t, err) - require.True(t, exists, "deleting device class that was never created") - err = tracker.deviceClasses.GetIndexer().Delete(oldObj) - require.NoError(t, err) - tracker.deviceClassDelete(ctx)(oldObj) - }, +func remove[T any](obj *T) [2]*T { + return [2]*T{obj, nil} +} + +func update[T any](oldObj, newObj *T) [2]*T { + return [2]*T{oldObj, newObj} +} + +func runInputEvents(tCtx *testContext, events []any) { + for _, event := range events { + applyEventPair(tCtx, event) } } +func applyEventPair(tCtx *testContext, event any) { + switch pair := event.(type) { + case [2]*resourceapi.ResourceSlice: + slice := pair[0] + if pair[1] != nil { + slice = pair[1] + } + require.NotNil(tCtx, slice) + + oldObj, exists, err := tCtx.resourceSlices.GetIndexer().Get(slice) + require.NoError(tCtx, err) + if pair[1] == nil { + require.True(tCtx, exists, "device taint rule that was never created cannot be deleted") + err = tCtx.resourceSlices.GetIndexer().Delete(oldObj) + require.NoError(tCtx, err) + tCtx.resourceSliceDelete(tCtx.Context)(oldObj) + } else { + err = tCtx.resourceSlices.GetIndexer().Add(slice) + require.NoError(tCtx, err) + if !exists { + tCtx.resourceSliceAdd(tCtx.Context)(slice) + } else if !apiequality.Semantic.DeepEqual(oldObj, slice) { + tCtx.resourceSliceUpdate(tCtx.Context)(oldObj, slice) + } + } + case [2]*resourcealphaapi.DeviceTaintRule: + taint := pair[0] + if pair[1] != nil { + taint = pair[1] + } + require.NotNil(tCtx, taint) + + oldObj, exists, err := tCtx.deviceTaints.GetIndexer().Get(taint) + require.NoError(tCtx, err) + if pair[1] == nil { + require.True(tCtx, exists, "device taint rule that was never created cannot be deleted") + err = tCtx.deviceTaints.GetIndexer().Delete(oldObj) + require.NoError(tCtx, err) + tCtx.deviceTaintDelete(tCtx.Context)(oldObj) + } else { + err = tCtx.deviceTaints.GetIndexer().Add(taint) + require.NoError(tCtx, err) + if !exists { + tCtx.deviceTaintAdd(tCtx.Context)(taint) + } else if !apiequality.Semantic.DeepEqual(oldObj, taint) { + tCtx.deviceTaintUpdate(tCtx.Context)(oldObj, taint) + } + } + case [2]*resourceapi.DeviceClass: + class := pair[0] + if pair[1] != nil { + class = pair[1] + } + require.NotNil(tCtx, class) + + oldObj, exists, err := tCtx.deviceClasses.GetIndexer().Get(class) + require.NoError(tCtx, err) + if pair[1] == nil { + require.True(tCtx, exists, "device class that was never created cannot be deleted") + err = tCtx.deviceClasses.GetIndexer().Delete(oldObj) + require.NoError(tCtx, err) + tCtx.deviceClassDelete(tCtx.Context)(oldObj) + } else { + err = tCtx.deviceClasses.GetIndexer().Add(class) + require.NoError(tCtx, err) + if !exists { + tCtx.deviceClassAdd(tCtx.Context)(class) + } else if !apiequality.Semantic.DeepEqual(oldObj, class) { + tCtx.deviceClassUpdate(tCtx.Context)(oldObj, class) + } + } + } +} + +type testContext struct { + *testing.T + context.Context + *Tracker +} + var ( now, _ = time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") driver1 = "driver1.example.com" @@ -309,17 +336,16 @@ var ( func TestListPatchedResourceSlices(t *testing.T) { tests := map[string]struct { - deviceTaintsDisabled bool - inputEvents func(event inputEventGenerator) + events []any expectedPatchedSlices []*resourceapi.ResourceSlice expectHandlerEvents func(t *testing.T, events []handlerEvent) expectEvents func(t *assert.CollectT, events *v1.EventList) expectUnhandledErrors func(t *testing.T, errs []error) }{ "add-slices-no-patches": { - inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + events: []any{ + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -337,14 +363,12 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "update-slices-no-patches": { - inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(slice1NoDevices) - event.addResourceSlice(slice2NoDevices) - event.addResourceSlice(unchangedSlice) - - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) - event.addResourceSlice(unchangedSlice) + events: []any{ + add(slice1NoDevices), + add(slice2NoDevices), + add(unchangedSlice), + update(slice1NoDevices, slice1), + update(slice2NoDevices, slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -366,12 +390,12 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "delete-slices": { - inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) - event.addResourceSlice(unchangedSlice) - event.deleteResourceSlice(slice1.Name) - event.deleteResourceSlice(slice2.Name) + events: []any{ + add(slice1), + add(slice2), + add(unchangedSlice), + remove(slice1), + remove(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ unchangedSlice, @@ -391,9 +415,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "patch-all-slices": { - inputEvents: func(event inputEventGenerator) { - event.addResourceSlice(slice1) - event.addDeviceTaintRule(taintAllDevicesRule) + events: []any{ + add(slice1), + add(taintAllDevicesRule), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -410,11 +434,11 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "update-patch": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintPool1DevicesRule) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) - event.addDeviceTaintRule(taintPool2DevicesRule) + events: []any{ + add(taintPool1DevicesRule), + add(slice1), + add(slice2), + update(taintPool1DevicesRule, taintPool2DevicesRule), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -441,9 +465,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "merge-taints": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintAllDevicesRule) - event.addResourceSlice(slice1AlreadyTainted) + events: []any{ + add(taintAllDevicesRule), + add(slice1AlreadyTainted), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1MergedTaints, @@ -459,10 +483,10 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "add-taint-for-driver": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintDriver1DevicesRule) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + events: []any{ + add(taintDriver1DevicesRule), + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -480,10 +504,10 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "add-taint-for-pool": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintPool1DevicesRule) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + events: []any{ + add(taintPool1DevicesRule), + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -501,10 +525,10 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "add-taint-for-device": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintDevice1Rule) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + events: []any{ + add(taintDevice1Rule), + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -522,10 +546,10 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "add-attribute-for-selector": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintDriver1DevicesCELRule) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + events: []any{ + add(taintDriver1DevicesCELRule), + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -543,9 +567,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "selector-does-not-match": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintNoDevicesCELRule) - event.addResourceSlice(slice1) + events: []any{ + add(taintNoDevicesCELRule), + add(slice1), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -561,9 +585,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "runtime-CEL-errors-skip-devices": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintNoDevicesCELRuntimeErrorRule) - event.addResourceSlice(slice1) + events: []any{ + add(taintNoDevicesCELRuntimeErrorRule), + add(slice1), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -586,9 +610,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "invalid-CEL-expression-throws-error": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintNoDevicesInvalidCELRule) - event.addResourceSlice(slice1) + events: []any{ + add(taintNoDevicesInvalidCELRule), + add(slice1), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{}, expectUnhandledErrors: func(t *testing.T, errs []error) { @@ -599,11 +623,11 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "add-taint-for-device-class": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceClass(deviceClass1) - event.addDeviceTaintRule(taintDeviceClass1Rule) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + events: []any{ + add(deviceClass1), + add(taintDeviceClass1Rule), + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -621,9 +645,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "filter-all-criteria": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceClass(deviceClass1) - event.addDeviceTaintRule( + events: []any{ + add(deviceClass1), + add( taintDeviceClassRule( taintDriverDevicesRule( taintPoolDevicesRule( @@ -640,9 +664,9 @@ func TestListPatchedResourceSlices(t *testing.T) { ), deviceClass1.Name, ), - ) - event.addResourceSlice(slice1) - event.addResourceSlice(slice2) + ), + add(slice1), + add(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, @@ -660,12 +684,12 @@ func TestListPatchedResourceSlices(t *testing.T) { }, }, "update-patched-slice": { - inputEvents: func(event inputEventGenerator) { - event.addDeviceTaintRule(taintDevice1Rule) - event.addResourceSlice(slice1) - event.addResourceSlice(sliceWithDevices(slice1, threeDevices)) - event.addResourceSlice(sliceWithDevices(slice2, threeDevices)) - event.addResourceSlice(sliceWithDevices(slice2, devices)) + events: []any{ + add(taintDevice1Rule), + add(slice1), + update(slice1, sliceWithDevices(slice1, threeDevices)), + add(sliceWithDevices(slice2, threeDevices)), + update(sliceWithDevices(slice2, threeDevices), sliceWithDevices(slice2, devices)), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ sliceWithDevices(slice1, threeDevicesOneTainted), @@ -707,7 +731,7 @@ func TestListPatchedResourceSlices(t *testing.T) { } opts := Options{ - EnableDeviceTaints: !test.deviceTaintsDisabled, + EnableDeviceTaints: true, SliceInformer: informerFactory.Resource().V1beta1().ResourceSlices(), TaintInformer: informerFactory.Resource().V1alpha3().DeviceTaintRules(), ClassInformer: informerFactory.Resource().V1beta1().DeviceClasses(), @@ -721,10 +745,14 @@ func TestListPatchedResourceSlices(t *testing.T) { } _, _ = tracker.AddEventHandler(handler) - if test.inputEvents != nil { - test.inputEvents(inputEventGeneratorForTest(ctx, t, tracker)) + tCtx := &testContext{ + T: t, + Context: ctx, + Tracker: tracker, } + runInputEvents(tCtx, test.events) + expectHandlerEvents := test.expectHandlerEvents if expectHandlerEvents == nil { expectHandlerEvents = func(t *testing.T, events []handlerEvent) { From 29b499e2ba492d6ae602ef35ec938365e7f75a47 Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Sun, 13 Apr 2025 12:42:28 -0500 Subject: [PATCH 3/6] simplify applyEventPair --- .../resourceslice/tracker/tracker_test.go | 94 +++++++------------ 1 file changed, 36 insertions(+), 58 deletions(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go index 89a369c2336..2c2b7e5a81d 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go @@ -29,7 +29,6 @@ import ( v1 "k8s.io/api/core/v1" resourcealphaapi "k8s.io/api/resource/v1alpha3" resourceapi "k8s.io/api/resource/v1beta1" - apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" @@ -75,73 +74,52 @@ func runInputEvents(tCtx *testContext, events []any) { func applyEventPair(tCtx *testContext, event any) { switch pair := event.(type) { case [2]*resourceapi.ResourceSlice: - slice := pair[0] - if pair[1] != nil { - slice = pair[1] - } - require.NotNil(tCtx, slice) - - oldObj, exists, err := tCtx.resourceSlices.GetIndexer().Get(slice) - require.NoError(tCtx, err) - if pair[1] == nil { - require.True(tCtx, exists, "device taint rule that was never created cannot be deleted") - err = tCtx.resourceSlices.GetIndexer().Delete(oldObj) + store := tCtx.resourceSlices.GetStore() + switch { + case pair[0] != nil && pair[1] != nil: + err := store.Update(pair[1]) require.NoError(tCtx, err) - tCtx.resourceSliceDelete(tCtx.Context)(oldObj) - } else { - err = tCtx.resourceSlices.GetIndexer().Add(slice) + tCtx.resourceSliceUpdate(tCtx.Context)(pair[0], pair[1]) + case pair[0] != nil: + err := store.Delete(pair[0]) require.NoError(tCtx, err) - if !exists { - tCtx.resourceSliceAdd(tCtx.Context)(slice) - } else if !apiequality.Semantic.DeepEqual(oldObj, slice) { - tCtx.resourceSliceUpdate(tCtx.Context)(oldObj, slice) - } + tCtx.resourceSliceDelete(tCtx.Context)(pair[0]) + default: + err := store.Add(pair[1]) + require.NoError(tCtx, err) + tCtx.resourceSliceAdd(tCtx.Context)(pair[1]) } case [2]*resourcealphaapi.DeviceTaintRule: - taint := pair[0] - if pair[1] != nil { - taint = pair[1] - } - require.NotNil(tCtx, taint) - - oldObj, exists, err := tCtx.deviceTaints.GetIndexer().Get(taint) - require.NoError(tCtx, err) - if pair[1] == nil { - require.True(tCtx, exists, "device taint rule that was never created cannot be deleted") - err = tCtx.deviceTaints.GetIndexer().Delete(oldObj) + store := tCtx.deviceTaints.GetStore() + switch { + case pair[0] != nil && pair[1] != nil: + err := store.Update(pair[1]) require.NoError(tCtx, err) - tCtx.deviceTaintDelete(tCtx.Context)(oldObj) - } else { - err = tCtx.deviceTaints.GetIndexer().Add(taint) + tCtx.deviceTaintUpdate(tCtx.Context)(pair[0], pair[1]) + case pair[0] != nil: + err := store.Delete(pair[0]) require.NoError(tCtx, err) - if !exists { - tCtx.deviceTaintAdd(tCtx.Context)(taint) - } else if !apiequality.Semantic.DeepEqual(oldObj, taint) { - tCtx.deviceTaintUpdate(tCtx.Context)(oldObj, taint) - } + tCtx.deviceTaintDelete(tCtx.Context)(pair[0]) + default: + err := store.Add(pair[1]) + require.NoError(tCtx, err) + tCtx.deviceTaintAdd(tCtx.Context)(pair[1]) } case [2]*resourceapi.DeviceClass: - class := pair[0] - if pair[1] != nil { - class = pair[1] - } - require.NotNil(tCtx, class) - - oldObj, exists, err := tCtx.deviceClasses.GetIndexer().Get(class) - require.NoError(tCtx, err) - if pair[1] == nil { - require.True(tCtx, exists, "device class that was never created cannot be deleted") - err = tCtx.deviceClasses.GetIndexer().Delete(oldObj) + store := tCtx.deviceClasses.GetStore() + switch { + case pair[0] != nil && pair[1] != nil: + err := store.Update(pair[1]) require.NoError(tCtx, err) - tCtx.deviceClassDelete(tCtx.Context)(oldObj) - } else { - err = tCtx.deviceClasses.GetIndexer().Add(class) + tCtx.deviceClassUpdate(tCtx.Context)(pair[0], pair[1]) + case pair[0] != nil: + err := store.Delete(pair[0]) require.NoError(tCtx, err) - if !exists { - tCtx.deviceClassAdd(tCtx.Context)(class) - } else if !apiequality.Semantic.DeepEqual(oldObj, class) { - tCtx.deviceClassUpdate(tCtx.Context)(oldObj, class) - } + tCtx.deviceClassDelete(tCtx.Context)(pair[0]) + default: + err := store.Add(pair[1]) + require.NoError(tCtx, err) + tCtx.deviceClassAdd(tCtx.Context)(pair[1]) } } } From 614b1e901702fea309175318e51dd4f4ffe8e982 Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Sat, 12 Apr 2025 21:14:57 -0500 Subject: [PATCH 4/6] Run all permutations of events in ResourceSliceTracker tests This change steals the idea from the device taint eviction controller tests that (at)pohly originally wrote to run each test case for the ResourceSlice tracker with every possible permutation of events, which should all produce the same output: a list of ResourceSlices with DeviceTaintRules applied. The set of emitted events by the tracker is sensitive to changes in the sequence of input events, so those are only verified against the original sequence of input events. This increases the number of test cases run from 16 to 107. --- .../resourceslice/tracker/tracker_test.go | 292 +++++++++++------- 1 file changed, 185 insertions(+), 107 deletions(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go index 2c2b7e5a81d..ae39c467207 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go @@ -19,8 +19,10 @@ package tracker import ( stdcmp "cmp" "context" + "fmt" "slices" "strconv" + "strings" "testing" "time" @@ -67,7 +69,14 @@ func update[T any](oldObj, newObj *T) [2]*T { func runInputEvents(tCtx *testContext, events []any) { for _, event := range events { - applyEventPair(tCtx, event) + switch event := event.(type) { + case []any: + for _, event := range event { + applyEventPair(tCtx, event) + } + default: + applyEventPair(tCtx, event) + } } } @@ -128,6 +137,7 @@ type testContext struct { *testing.T context.Context *Tracker + *fake.Clientset } var ( @@ -313,13 +323,24 @@ var ( ) func TestListPatchedResourceSlices(t *testing.T) { - tests := map[string]struct { + type test struct { + // events contains pairs of old and new objects which will + // be passed to event handler methods. + // Objects can be slices, device taint rules, and device + // classes. + // [add], [remove], and [update] can be used to produce + // such pairs. + // + // Alternatively, it can also contain a list of such pairs. + // Those will be applied in the order in which they appear + // in each event entry. events []any expectedPatchedSlices []*resourceapi.ResourceSlice expectHandlerEvents func(t *testing.T, events []handlerEvent) expectEvents func(t *assert.CollectT, events *v1.EventList) expectUnhandledErrors func(t *testing.T, errs []error) - }{ + } + tests := map[string]test{ "add-slices-no-patches": { events: []any{ add(slice1), @@ -342,11 +363,15 @@ func TestListPatchedResourceSlices(t *testing.T) { }, "update-slices-no-patches": { events: []any{ - add(slice1NoDevices), - add(slice2NoDevices), + []any{ + add(slice1NoDevices), + update(slice1NoDevices, slice1), + }, + []any{ + add(slice2NoDevices), + update(slice2NoDevices, slice2), + }, add(unchangedSlice), - update(slice1NoDevices, slice1), - update(slice2NoDevices, slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -358,10 +383,10 @@ func TestListPatchedResourceSlices(t *testing.T) { t, []handlerEvent{ {event: handlerEventAdd, newObj: slice1NoDevices}, - {event: handlerEventAdd, newObj: slice2NoDevices}, - {event: handlerEventAdd, newObj: unchangedSlice}, {event: handlerEventUpdate, oldObj: slice1NoDevices, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2NoDevices}, {event: handlerEventUpdate, oldObj: slice2NoDevices, newObj: slice2}, + {event: handlerEventAdd, newObj: unchangedSlice}, }, events, ) @@ -369,11 +394,9 @@ func TestListPatchedResourceSlices(t *testing.T) { }, "delete-slices": { events: []any{ - add(slice1), - add(slice2), + []any{add(slice1), remove(slice1)}, + []any{add(slice2), remove(slice2)}, add(unchangedSlice), - remove(slice1), - remove(slice2), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ unchangedSlice, @@ -383,10 +406,10 @@ func TestListPatchedResourceSlices(t *testing.T) { t, []handlerEvent{ {event: handlerEventAdd, newObj: slice1}, - {event: handlerEventAdd, newObj: slice2}, - {event: handlerEventAdd, newObj: unchangedSlice}, {event: handlerEventDelete, oldObj: slice1}, + {event: handlerEventAdd, newObj: slice2}, {event: handlerEventDelete, oldObj: slice2}, + {event: handlerEventAdd, newObj: unchangedSlice}, }, events, ) @@ -413,10 +436,12 @@ func TestListPatchedResourceSlices(t *testing.T) { }, "update-patch": { events: []any{ - add(taintPool1DevicesRule), + []any{ + add(taintPool1DevicesRule), + update(taintPool1DevicesRule, taintPool2DevicesRule), + }, add(slice1), add(slice2), - update(taintPool1DevicesRule, taintPool2DevicesRule), }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, @@ -426,19 +451,10 @@ func TestListPatchedResourceSlices(t *testing.T) { assert.Equal( t, []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2Tainted}, }, - events[:2], - ) - // The remaining events may come in any order - assert.ElementsMatch( - t, - []handlerEvent{ - {event: handlerEventUpdate, oldObj: slice1Tainted, newObj: slice1}, - {event: handlerEventUpdate, oldObj: slice2, newObj: slice2Tainted}, - }, - events[2:], + events, ) }, }, @@ -589,8 +605,10 @@ func TestListPatchedResourceSlices(t *testing.T) { }, "invalid-CEL-expression-throws-error": { events: []any{ - add(taintNoDevicesInvalidCELRule), - add(slice1), + []any{ + add(taintNoDevicesInvalidCELRule), + add(slice1), + }, }, expectedPatchedSlices: []*resourceapi.ResourceSlice{}, expectUnhandledErrors: func(t *testing.T, errs []error) { @@ -664,10 +682,14 @@ func TestListPatchedResourceSlices(t *testing.T) { "update-patched-slice": { events: []any{ add(taintDevice1Rule), - add(slice1), - update(slice1, sliceWithDevices(slice1, threeDevices)), - add(sliceWithDevices(slice2, threeDevices)), - update(sliceWithDevices(slice2, threeDevices), sliceWithDevices(slice2, devices)), + []any{ + add(slice1), + update(slice1, sliceWithDevices(slice1, threeDevices)), + }, + []any{ + add(sliceWithDevices(slice2, threeDevices)), + update(sliceWithDevices(slice2, threeDevices), sliceWithDevices(slice2, devices)), + }, }, expectedPatchedSlices: []*resourceapi.ResourceSlice{ sliceWithDevices(slice1, threeDevicesOneTainted), @@ -688,93 +710,149 @@ func TestListPatchedResourceSlices(t *testing.T) { }, } - for name, test := range tests { - t.Run(name, func(t *testing.T) { - _, ctx := ktesting.NewTestContext(t) + setup := func(t *testing.T) *testContext { + _, ctx := ktesting.NewTestContext(t) - kubeClient := fake.NewSimpleClientset() - informerFactory := informers.NewSharedInformerFactoryWithOptions(kubeClient, 10*time.Minute) + kubeClient := fake.NewSimpleClientset() + informerFactory := informers.NewSharedInformerFactoryWithOptions(kubeClient, 10*time.Minute) - var handlerEvents []handlerEvent - handler := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - handlerEvents = append(handlerEvents, handlerEvent{event: handlerEventAdd, newObj: obj.(*resourceapi.ResourceSlice)}) - }, - UpdateFunc: func(oldObj, newObj interface{}) { - handlerEvents = append(handlerEvents, handlerEvent{event: handlerEventUpdate, oldObj: oldObj.(*resourceapi.ResourceSlice), newObj: newObj.(*resourceapi.ResourceSlice)}) - }, - DeleteFunc: func(obj interface{}) { - handlerEvents = append(handlerEvents, handlerEvent{event: handlerEventDelete, oldObj: obj.(*resourceapi.ResourceSlice)}) - }, - } + opts := Options{ + EnableDeviceTaints: true, + SliceInformer: informerFactory.Resource().V1beta1().ResourceSlices(), + TaintInformer: informerFactory.Resource().V1alpha3().DeviceTaintRules(), + ClassInformer: informerFactory.Resource().V1beta1().DeviceClasses(), + KubeClient: kubeClient, + } + tracker, err := newTracker(ctx, opts) + require.NoError(t, err) - opts := Options{ - EnableDeviceTaints: true, - SliceInformer: informerFactory.Resource().V1beta1().ResourceSlices(), - TaintInformer: informerFactory.Resource().V1alpha3().DeviceTaintRules(), - ClassInformer: informerFactory.Resource().V1beta1().DeviceClasses(), - KubeClient: kubeClient, - } - tracker, err := newTracker(ctx, opts) - require.NoError(t, err) - var unhandledErrors []error - tracker.handleError = func(_ context.Context, err error, _ string, _ ...any) { - unhandledErrors = append(unhandledErrors, err) - } - _, _ = tracker.AddEventHandler(handler) + return &testContext{ + T: t, + Context: ctx, + Tracker: tracker, + Clientset: kubeClient, + } + } - tCtx := &testContext{ - T: t, - Context: ctx, - Tracker: tracker, - } + testHandlers := func(tCtx *testContext, test test, testExpectedEmittedEvents bool) { + var handlerEvents []handlerEvent + handler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + handlerEvents = append(handlerEvents, handlerEvent{event: handlerEventAdd, newObj: obj.(*resourceapi.ResourceSlice)}) + }, + UpdateFunc: func(oldObj, newObj interface{}) { + handlerEvents = append(handlerEvents, handlerEvent{event: handlerEventUpdate, oldObj: oldObj.(*resourceapi.ResourceSlice), newObj: newObj.(*resourceapi.ResourceSlice)}) + }, + DeleteFunc: func(obj interface{}) { + handlerEvents = append(handlerEvents, handlerEvent{event: handlerEventDelete, oldObj: obj.(*resourceapi.ResourceSlice)}) + }, + } + _, _ = tCtx.AddEventHandler(handler) - runInputEvents(tCtx, test.events) + var unhandledErrors []error + tCtx.handleError = func(_ context.Context, err error, _ string, _ ...any) { + unhandledErrors = append(unhandledErrors, err) + } + runInputEvents(tCtx, test.events) + + if testExpectedEmittedEvents { expectHandlerEvents := test.expectHandlerEvents if expectHandlerEvents == nil { expectHandlerEvents = func(t *testing.T, events []handlerEvent) { - assert.Empty(t, events) + assert.Empty(tCtx, events) } } expectHandlerEvents(t, handlerEvents) + } - expectUnhandledErrors := test.expectUnhandledErrors - if expectUnhandledErrors == nil { - expectUnhandledErrors = func(t *testing.T, errs []error) { - assert.Empty(t, errs) + expectUnhandledErrors := test.expectUnhandledErrors + if expectUnhandledErrors == nil { + expectUnhandledErrors = func(t *testing.T, errs []error) { + assert.Empty(t, errs) + } + } + expectUnhandledErrors(tCtx.T, unhandledErrors) + + // Check ResourceSlices + patchedResourceSlices, err := tCtx.ListPatchedResourceSlices() + require.NoError(tCtx, err, "list patched resource slices") + sortResourceSlicesFunc := func(s1, s2 *resourceapi.ResourceSlice) int { + return stdcmp.Compare(s1.Name, s2.Name) + } + slices.SortFunc(test.expectedPatchedSlices, sortResourceSlicesFunc) + slices.SortFunc(patchedResourceSlices, sortResourceSlicesFunc) + assert.Equal(tCtx, test.expectedPatchedSlices, patchedResourceSlices) + expectEvents := test.expectEvents + if expectEvents == nil { + expectEvents = func(t *assert.CollectT, events *v1.EventList) { + assert.Empty(t, events.Items) + } + } + // Events are generated asynchronously. While shutting down the event recorder will flush all + // pending events, it is not possible to determine when exactly that flush is complete. + assert.EventuallyWithT( + tCtx, + func(t *assert.CollectT) { + events, err := tCtx.CoreV1().Events("").List(tCtx.Context, metav1.ListOptions{}) + require.NoError(t, err, "list events") + expectEvents(t, events) + }, + 1*time.Second, + 10*time.Millisecond, + "did not observe expected events", + ) + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + // The exact events that are emitted for a sequence of events is + // highly dependent on the order in which those events are received. + // We punt on determining a set of validation criteria for every + // possible sequence and only check them against the first + // permutation: the order in which the events are defined. + testExpectedEmittedEvents := true + + numEvents := len(tc.events) + if numEvents <= 1 { + // No permutations. + tContext := setup(t) + testHandlers(tContext, tc, testExpectedEmittedEvents) + return + } + + permutation := make([]int, numEvents) + var permutate func(depth int) + permutate = func(depth int) { + if depth >= numEvents { + // Define a sub-test which runs the current permutation of events. + events := make([]any, numEvents) + for i := range numEvents { + events[i] = tc.events[permutation[i]] + } + tc := tc + tc.events = events + name := strings.Trim(fmt.Sprintf("%v", permutation), "[]") + t.Run(name, func(t *testing.T) { + tContext := setup(t) + testHandlers(tContext, tc, testExpectedEmittedEvents) + + testExpectedEmittedEvents = false + }) + return + } + for i := range numEvents { + if slices.Contains(permutation[0:depth], i) { + // Already taken. + continue + } + // Pick it for the current position in permutation, + // continue with next position. + permutation[depth] = i + permutate(depth + 1) } } - expectUnhandledErrors(t, unhandledErrors) - - // Check ResourceSlices - patchedResourceSlices, err := tracker.ListPatchedResourceSlices() - require.NoError(t, err, "list patched resource slices") - sortResourceSlicesFunc := func(s1, s2 *resourceapi.ResourceSlice) int { - return stdcmp.Compare(s1.Name, s2.Name) - } - slices.SortFunc(test.expectedPatchedSlices, sortResourceSlicesFunc) - slices.SortFunc(patchedResourceSlices, sortResourceSlicesFunc) - assert.Equal(t, test.expectedPatchedSlices, patchedResourceSlices) - expectEvents := test.expectEvents - if expectEvents == nil { - expectEvents = func(t *assert.CollectT, events *v1.EventList) { - assert.Empty(t, events.Items) - } - } - // Events are generated asynchronously. While shutting down the event recorder will flush all - // pending events, it is not possible to determine when exactly that flush is complete. - assert.EventuallyWithT( - t, - func(t *assert.CollectT) { - events, err := kubeClient.CoreV1().Events("").List(ctx, metav1.ListOptions{}) - require.NoError(t, err, "list events") - expectEvents(t, events) - }, - 1*time.Second, - 10*time.Millisecond, - "did not observe expected events", - ) + permutate(0) }) } } From 5ebeb8defacddae685f03d2be47b429bd9df68d2 Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Sat, 12 Apr 2025 21:26:30 -0500 Subject: [PATCH 5/6] don't only run events in nested lists consecutively This change allows events in nested lists to have other events placed in between them so long as no event in a nested list occurs after one of its successors in that same nested list. Given a sequence with two nested lists like this: [ [1 2 3], [4 5], ] The prior version of these tests would execute these sequences of events: [1 2 3 4 5] [4 5 1 2 3] Now, these sequences would be executed: [1 2 3 4 5] [1 2 4 3 5] [1 2 4 5 3] [1 4 2 3 5] [1 4 2 5 3] [1 4 5 2 3] [4 1 2 3 5] [4 1 2 5 3] [4 1 5 2 3] [4 5 1 2 3] This increases the total number of test cases run from 107 to 185. --- .../resourceslice/tracker/tracker_test.go | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go index ae39c467207..e5fa49fec3f 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go @@ -333,7 +333,9 @@ func TestListPatchedResourceSlices(t *testing.T) { // // Alternatively, it can also contain a list of such pairs. // Those will be applied in the order in which they appear - // in each event entry. + // in each event entry, but not necessarily in consecutive + // order. Other events may be placed in between as long as + // the order in those nested lists is preserved. events []any expectedPatchedSlices []*resourceapi.ResourceSlice expectHandlerEvents func(t *testing.T, events []handlerEvent) @@ -821,6 +823,28 @@ func TestListPatchedResourceSlices(t *testing.T) { return } + // flatten does one level of flattening of events. It also returns + // another slice of pairs of indices representing ranges which were + // flattened. + flatten := func(events []any) ([]any, [][2]int) { + var ret []any + var ranges [][2]int + for _, e := range events { + switch e := e.(type) { + case []any: + ranges = append(ranges, [2]int{len(ret), len(ret) + len(e)}) + ret = append(ret, e...) + default: + ret = append(ret, e) + } + } + return ret, ranges + } + + var constraints [][2]int + tc.events, constraints = flatten(tc.events) + numEvents = len(tc.events) + permutation := make([]int, numEvents) var permutate func(depth int) permutate = func(depth int) { @@ -841,11 +865,22 @@ func TestListPatchedResourceSlices(t *testing.T) { }) return } + nexti: for i := range numEvents { if slices.Contains(permutation[0:depth], i) { // Already taken. continue } + for _, constraint := range constraints { + if i < constraint[0] || i > constraint[1] { + continue + } + for j := i + 1; j < constraint[1]; j++ { + if slices.Contains(permutation[0:depth], j) { + continue nexti + } + } + } // Pick it for the current position in permutation, // continue with next position. permutation[depth] = i From 405b9e80221d16a3a85d16e50ae0671afa0df939 Mon Sep 17 00:00:00 2001 From: Jon Huhn Date: Sat, 12 Apr 2025 22:11:44 -0500 Subject: [PATCH 6/6] Check expected emitted events with assert.Equal Since the only test case where a single assert.Equal was not used to check the expected events emitted was refactored to make the events permutable, every test case can now use the same simple equality check. --- .../resourceslice/tracker/tracker_test.go | 200 +++++------------- 1 file changed, 52 insertions(+), 148 deletions(-) diff --git a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go index e5fa49fec3f..8e7a35c4b57 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/resourceslice/tracker/tracker_test.go @@ -338,7 +338,7 @@ func TestListPatchedResourceSlices(t *testing.T) { // the order in those nested lists is preserved. events []any expectedPatchedSlices []*resourceapi.ResourceSlice - expectHandlerEvents func(t *testing.T, events []handlerEvent) + expectedHandlerEvents []handlerEvent expectEvents func(t *assert.CollectT, events *v1.EventList) expectUnhandledErrors func(t *testing.T, errs []error) } @@ -352,15 +352,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2}, }, }, "update-slices-no-patches": { @@ -380,18 +374,12 @@ func TestListPatchedResourceSlices(t *testing.T) { slice2, unchangedSlice, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1NoDevices}, - {event: handlerEventUpdate, oldObj: slice1NoDevices, newObj: slice1}, - {event: handlerEventAdd, newObj: slice2NoDevices}, - {event: handlerEventUpdate, oldObj: slice2NoDevices, newObj: slice2}, - {event: handlerEventAdd, newObj: unchangedSlice}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1NoDevices}, + {event: handlerEventUpdate, oldObj: slice1NoDevices, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2NoDevices}, + {event: handlerEventUpdate, oldObj: slice2NoDevices, newObj: slice2}, + {event: handlerEventAdd, newObj: unchangedSlice}, }, }, "delete-slices": { @@ -403,18 +391,12 @@ func TestListPatchedResourceSlices(t *testing.T) { expectedPatchedSlices: []*resourceapi.ResourceSlice{ unchangedSlice, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1}, - {event: handlerEventDelete, oldObj: slice1}, - {event: handlerEventAdd, newObj: slice2}, - {event: handlerEventDelete, oldObj: slice2}, - {event: handlerEventAdd, newObj: unchangedSlice}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventDelete, oldObj: slice1}, + {event: handlerEventAdd, newObj: slice2}, + {event: handlerEventDelete, oldObj: slice2}, + {event: handlerEventAdd, newObj: unchangedSlice}, }, }, "patch-all-slices": { @@ -425,15 +407,9 @@ func TestListPatchedResourceSlices(t *testing.T) { expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1Tainted, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1}, - {event: handlerEventUpdate, oldObj: slice1, newObj: slice1Tainted}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventUpdate, oldObj: slice1, newObj: slice1Tainted}, }, }, "update-patch": { @@ -449,15 +425,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1, slice2Tainted, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1}, - {event: handlerEventAdd, newObj: slice2Tainted}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, + {event: handlerEventAdd, newObj: slice2Tainted}, }, }, "merge-taints": { @@ -468,14 +438,8 @@ func TestListPatchedResourceSlices(t *testing.T) { expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1MergedTaints, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1MergedTaints}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1MergedTaints}, }, }, "add-taint-for-driver": { @@ -488,15 +452,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1Tainted, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, }, }, "add-taint-for-pool": { @@ -509,15 +467,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1Tainted, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, }, }, "add-taint-for-device": { @@ -530,15 +482,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1Tainted, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, }, }, "add-attribute-for-selector": { @@ -551,15 +497,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1Tainted, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, }, }, "selector-does-not-match": { @@ -570,14 +510,8 @@ func TestListPatchedResourceSlices(t *testing.T) { expectedPatchedSlices: []*resourceapi.ResourceSlice{ slice1, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, }, }, "runtime-CEL-errors-skip-devices": { @@ -595,14 +529,8 @@ func TestListPatchedResourceSlices(t *testing.T) { assert.Equal(t, taintNoDevicesCELRuntimeErrorRule.Name, events.Items[0].InvolvedObject.Name) assert.Equal(t, "CELRuntimeError", events.Items[0].Reason) }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1}, }, }, "invalid-CEL-expression-throws-error": { @@ -631,15 +559,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1Tainted, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, }, }, "filter-all-criteria": { @@ -670,15 +592,9 @@ func TestListPatchedResourceSlices(t *testing.T) { slice1Tainted, slice2, }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventAdd, newObj: slice2}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventAdd, newObj: slice2}, }, }, "update-patched-slice": { @@ -697,17 +613,11 @@ func TestListPatchedResourceSlices(t *testing.T) { sliceWithDevices(slice1, threeDevicesOneTainted), sliceWithDevices(slice2, taintedDevices), }, - expectHandlerEvents: func(t *testing.T, events []handlerEvent) { - assert.Equal( - t, - []handlerEvent{ - {event: handlerEventAdd, newObj: slice1Tainted}, - {event: handlerEventUpdate, oldObj: slice1Tainted, newObj: sliceWithDevices(slice1, threeDevicesOneTainted)}, - {event: handlerEventAdd, newObj: sliceWithDevices(slice2, threeDevicesOneTainted)}, - {event: handlerEventUpdate, oldObj: sliceWithDevices(slice2, threeDevicesOneTainted), newObj: sliceWithDevices(slice2, taintedDevices)}, - }, - events, - ) + expectedHandlerEvents: []handlerEvent{ + {event: handlerEventAdd, newObj: slice1Tainted}, + {event: handlerEventUpdate, oldObj: slice1Tainted, newObj: sliceWithDevices(slice1, threeDevicesOneTainted)}, + {event: handlerEventAdd, newObj: sliceWithDevices(slice2, threeDevicesOneTainted)}, + {event: handlerEventUpdate, oldObj: sliceWithDevices(slice2, threeDevicesOneTainted), newObj: sliceWithDevices(slice2, taintedDevices)}, }, }, } @@ -759,13 +669,7 @@ func TestListPatchedResourceSlices(t *testing.T) { runInputEvents(tCtx, test.events) if testExpectedEmittedEvents { - expectHandlerEvents := test.expectHandlerEvents - if expectHandlerEvents == nil { - expectHandlerEvents = func(t *testing.T, events []handlerEvent) { - assert.Empty(tCtx, events) - } - } - expectHandlerEvents(t, handlerEvents) + assert.Equal(tCtx, test.expectedHandlerEvents, handlerEvents) } expectUnhandledErrors := test.expectUnhandledErrors