github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 14:37:00 +00:00
Code Issues Projects Releases Wiki Activity

[KEP-4817] Fixes based on review

Browse Source 
* Rename HWAddress to HardwareAddress
* Fix condition validation
* Remove feature gate validation
* Fix drop field on disabled feature gate

Signed-off-by: Lionel Jouin <lionel.jouin@est.tech>
This commit is contained in:
Lionel Jouin 2024-11-04 10:03:29 +01:00
parent 5df47a64d3
commit a062f91106
8 changed files with 120 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/apis/resource/types.go
View File

@ -1056,8 +1056,8 @@ type NetworkDeviceData struct {
// +listType=atomic
Addresses []string
// HWAddress represents the hardware address (e.g. MAC Address) of the device's network interface.
// HardwareAddress represents the hardware address (e.g. MAC Address) of the device's network interface.
//
// +optional
HWAddress string
HardwareAddress string
}

36
pkg/apis/resource/validation/validation.go
View File

@ -25,7 +25,7 @@ import (
apiequality "k8s.io/apimachinery/pkg/api/equality"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
@ -33,12 +33,10 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/cel"
"k8s.io/apiserver/pkg/cel/environment"
utilfeature "k8s.io/apiserver/pkg/util/feature"
dracel "k8s.io/dynamic-resource-allocation/cel"
"k8s.io/dynamic-resource-allocation/structured"
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/resource"
"k8s.io/kubernetes/pkg/features"
)
var (
@ -268,20 +266,18 @@ func validateResourceClaimStatusUpdate(status, oldStatus *resource.ResourceClaim
func(consumer resource.ResourceClaimConsumerReference) (types.UID, string) { return consumer.UID, "uid" },
fldPath.Child("reservedFor"))...)
if utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) {
var allocatedDevices sets.Set[structured.DeviceID]
if status.Allocation != nil {
allocatedDevices = gatherAllocatedDevices(&status.Allocation.Devices)
}
allErrs = append(allErrs, validateSet(status.Devices, -1,
func(device resource.AllocatedDeviceStatus, fldPath *field.Path) field.ErrorList {
return validateDeviceStatus(device, fldPath, allocatedDevices)
},
func(device resource.AllocatedDeviceStatus) (structured.DeviceID, string) {
return structured.DeviceID{Driver: device.Driver, Pool: device.Pool, Device: device.Device}, "deviceID"
},
fldPath.Child("devices"))...)
var allocatedDevices sets.Set[structured.DeviceID]
if status.Allocation != nil {
allocatedDevices = gatherAllocatedDevices(&status.Allocation.Devices)
}
allErrs = append(allErrs, validateSet(status.Devices, -1,
func(device resource.AllocatedDeviceStatus, fldPath *field.Path) field.ErrorList {
return validateDeviceStatus(device, fldPath, allocatedDevices)
},
func(device resource.AllocatedDeviceStatus) (structured.DeviceID, string) {
return structured.DeviceID{Driver: device.Driver, Pool: device.Pool, Device: device.Device}, "deviceID"
},
fldPath.Child("devices"))...)
// Now check for invariants that must be valid for a ResourceClaim.
if len(status.ReservedFor) > 0 {
@ -748,11 +744,7 @@ func validateDeviceStatus(device resource.AllocatedDeviceStatus, fldPath *field.
if !allocatedDevices.Has(deviceID) {
allErrs = append(allErrs, field.Invalid(fldPath, deviceID, "must be an allocated device in the claim"))
}
allErrs = append(allErrs, validateSlice(device.Conditions, -1,
func(condition metav1.Condition, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
return allErrs
}, fldPath.Child("conditions"))...)
allErrs = append(allErrs, metav1validation.ValidateConditions(device.Conditions, fldPath.Child("conditions"))...)
if len(device.Data.Raw) > 0 { // Data is an optional field.
allErrs = append(allErrs, validateRawExtension(device.Data, fldPath.Child("data"))...)
}
@ -760,9 +752,9 @@ func validateDeviceStatus(device resource.AllocatedDeviceStatus, fldPath *field.
return allErrs
}
// validateRawExtension validates RawExtension as in https://github.com/kubernetes/kubernetes/pull/125549/
func validateRawExtension(rawExtension runtime.RawExtension, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
// Validation of RawExtension as in https://github.com/kubernetes/kubernetes/pull/125549/
var v any
if len(rawExtension.Raw) == 0 {
allErrs = append(allErrs, field.Required(fldPath, ""))

33
pkg/apis/resource/validation/validation_resourceclaim_test.go
View File

@ -995,8 +995,11 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
Device: goodName,
Conditions: []metav1.Condition{
{
Type: "test",
Status: metav1.ConditionTrue,
Type: "test",
Status: metav1.ConditionTrue,
Reason: "test_reason",
LastTransitionTime: metav1.Now(),
ObservedGeneration: 0,
},
},
Data: runtime.RawExtension{
@ -1008,7 +1011,7 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
"10.9.8.0/24",
"2001:db8::/64",
},
HWAddress: "ea:9f:cb:40:b1:7b",
HardwareAddress: "ea:9f:cb:40:b1:7b",
},
},
}
@ -1098,8 +1101,10 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
deviceStatusFeatureGate: true,
},
"invalid-device-status-duplicate-disabled-feature-gate": {
wantFailures: nil,
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
wantFailures: field.ErrorList{
field.Duplicate(field.NewPath("status", "devices").Index(1).Child("deviceID"), structured.DeviceID{Driver: goodName, Pool: goodName, Device: goodName}),
},
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
claim.Status.Devices = []resource.AllocatedDeviceStatus{
{
@ -1118,8 +1123,10 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
deviceStatusFeatureGate: false,
},
"invalid-network-device-status-disabled-feature-gate": {
wantFailures: nil,
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
wantFailures: field.ErrorList{
field.Invalid(field.NewPath("status", "devices").Index(0).Child("networkData", "addresses").Index(0), "300.9.8.0/24", "must be a valid CIDR value, (e.g. 10.9.8.0/24 or 2001:db8::/64)"),
},
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
claim.Status.Devices = []resource.AllocatedDeviceStatus{
{
@ -1138,8 +1145,10 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
deviceStatusFeatureGate: false,
},
"invalid-data-device-status-disabled-feature-gate": {
wantFailures: nil,
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
wantFailures: field.ErrorList{
field.Invalid(field.NewPath("status", "devices").Index(0).Child("data"), "<value omitted>", "error parsing data: invalid character 'o' in literal false (expecting 'a')"),
},
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
claim.Status.Devices = []resource.AllocatedDeviceStatus{
{
@ -1156,8 +1165,10 @@ func TestValidateClaimStatusUpdate(t *testing.T) {
deviceStatusFeatureGate: false,
},
"invalid-device-status-no-device-disabled-feature-gate": {
wantFailures: nil,
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
wantFailures: field.ErrorList{
field.Invalid(field.NewPath("status", "devices").Index(0), structured.DeviceID{Driver: "b", Pool: "a", Device: "r"}, "must be an allocated device in the claim"),
},
oldClaim: func() *resource.ResourceClaim { return validAllocatedClaim }(),
update: func(claim *resource.ResourceClaim) *resource.ResourceClaim {
claim.Status.Devices = []resource.AllocatedDeviceStatus{
{

24
pkg/registry/resource/resourceclaim/strategy.go
View File

@ -237,7 +237,9 @@ func draAdminAccessFeatureInUse(claim *resource.ResourceClaim) bool {
}
func dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim *resource.ResourceClaim) {
if !utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) {
isDRAResourceClaimDeviceStatusInUse := (oldClaim != nil && len(oldClaim.Status.Devices) > 0)
// drop resourceClaim.Status.Devices field if feature gate is not enabled and it was not in use
if !utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) && !isDRAResourceClaimDeviceStatusInUse {
newClaim.Status.Devices = nil
}
}
@ -245,7 +247,8 @@ func dropDisabledDRAResourceClaimDeviceStatusFields(newClaim, oldClaim *resource
// dropDeallocatedStatusDevices removes the status.devices that were allocated
// in the oldClaim and that have been removed in the newClaim.
func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) {
if !utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) {
isDRAResourceClaimDeviceStatusInUse := (oldClaim != nil && len(oldClaim.Status.Devices) > 0)
if !utilfeature.DefaultFeatureGate.Enabled(features.DRAResourceClaimDeviceStatus) && !isDRAResourceClaimDeviceStatusInUse {
return
}
@ -268,18 +271,19 @@ func dropDeallocatedStatusDevices(newClaim, oldClaim *resource.ResourceClaim) {
}
// Remove from newClaim.Status.Devices.
for i := len(newClaim.Status.Devices) - 1; i >= 0; i-- {
n := 0
for _, device := range newClaim.Status.Devices {
deviceID := structured.DeviceID{
Driver: newClaim.Status.Devices[i].Driver,
Pool: newClaim.Status.Devices[i].Pool,
Device: newClaim.Status.Devices[i].Device,
Driver: device.Driver,
Pool: device.Pool,
Device: device.Device,
}
// Device was in the oldClaim.Status.Allocation.Devices but is no longer in the
// newClaim.Status.Allocation.Devices so it must be removed from the newClaim.Status.Devices.
if deallocatedDevices.Has(deviceID) {
newClaim.Status.Devices = append(newClaim.Status.Devices[:i], newClaim.Status.Devices[i+1:]...)
if !deallocatedDevices.Has(deviceID) {
newClaim.Status.Devices[n] = device
n++
}
}
newClaim.Status.Devices = newClaim.Status.Devices[:n]
if len(newClaim.Status.Devices) == 0 {
newClaim.Status.Devices = nil

55
pkg/registry/resource/resourceclaim/strategy_test.go
View File

@ -133,10 +133,12 @@ var objWithAdminAccessStatus = &resource.ResourceClaim{
},
}
var testRequest = "test-request"
var testDriver = "test-driver"
var testPool = "test-pool"
var testDevice = "test-device"
const (
testRequest = "test-request"
testDriver = "test-driver"
testPool = "test-pool"
testDevice = "test-device"
)
func TestStrategy(t *testing.T) {
if !Strategy.NamespaceScoped() {
@ -375,6 +377,30 @@ func TestStatusStrategyUpdate(t *testing.T) {
return obj
}(),
},
"keep-fields-devices-status-disable-feature-gate": {
oldObj: func() *resource.ResourceClaim {
obj := obj.DeepCopy()
addSpecDevicesRequest(obj, testRequest)
addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
addStatusDevices(obj, testDriver, testPool, testDevice)
return obj
}(),
newObj: func() *resource.ResourceClaim { // Status is added
obj := obj.DeepCopy()
addSpecDevicesRequest(obj, testRequest)
addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
addStatusDevices(obj, testDriver, testPool, testDevice)
return obj
}(),
deviceStatusFeatureGate: false,
expectObj: func() *resource.ResourceClaim { // Status is still there (as the status was set in the old object)
obj := obj.DeepCopy()
addSpecDevicesRequest(obj, testRequest)
addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
addStatusDevices(obj, testDriver, testPool, testDevice)
return obj
}(),
},
"keep-fields-devices-status": {
oldObj: func() *resource.ResourceClaim {
obj := obj.DeepCopy()
@ -419,6 +445,27 @@ func TestStatusStrategyUpdate(t *testing.T) {
return obj
}(),
},
"drop-status-deallocated-device-disable-feature-gate": {
oldObj: func() *resource.ResourceClaim {
obj := obj.DeepCopy()
addSpecDevicesRequest(obj, testRequest)
addStatusAllocationDevicesResults(obj, testDriver, testPool, testDevice, testRequest)
addStatusDevices(obj, testDriver, testPool, testDevice)
return obj
}(),
newObj: func() *resource.ResourceClaim { // device is deallocated
obj := obj.DeepCopy()
addSpecDevicesRequest(obj, testRequest)
addStatusDevices(obj, testDriver, testPool, testDevice)
return obj
}(),
deviceStatusFeatureGate: false,
expectObj: func() *resource.ResourceClaim { // Status is no longer there
obj := obj.DeepCopy()
addSpecDevicesRequest(obj, testRequest)
return obj
}(),
},
}
for name, tc := range testcases {

4
staging/src/k8s.io/api/resource/v1alpha3/types.go
View File

@ -1067,8 +1067,8 @@ type NetworkDeviceData struct {
// +listType=atomic
Addresses []string `json:"addresses,omitempty" protobuf:"bytes,2,opt,name=addresses"`
// HWAddress represents the hardware address (e.g. MAC Address) of the device's network interface.
// HardwareAddress represents the hardware address (e.g. MAC Address) of the device's network interface.
//
// +optional
HWAddress string `json:"hwAddress,omitempty" protobuf:"bytes,3,opt,name=hwAddress"`
HardwareAddress string `json:"hardwareAddress,omitempty" protobuf:"bytes,3,opt,name=hardwareAddress"`
}

20
test/e2e/dra/dra.go
View File

@ -437,12 +437,12 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation,
Driver: allocatedResourceClaim.Status.Allocation.Devices.Results[0].Driver,
Pool: allocatedResourceClaim.Status.Allocation.Devices.Results[0].Pool,
Device: allocatedResourceClaim.Status.Allocation.Devices.Results[0].Device,
Conditions: []metav1.Condition{{Type: "a", Status: "b", Message: "c", Reason: "d"}},
Conditions: []metav1.Condition{{Type: "a", Status: "True", Message: "c", Reason: "d", LastTransitionTime: metav1.NewTime(time.Now().Truncate(time.Second))}},
Data: runtime.RawExtension{Raw: []byte(`{"foo":"bar"}`)},
NetworkData: &resourceapi.NetworkDeviceData{
InterfaceName: "inf1",
Addresses: []string{"10.9.8.0/24", "2001:db8::/64"},
HWAddress: "bc:1c:b6:3e:b8:25",
InterfaceName: "inf1",
Addresses: []string{"10.9.8.0/24", "2001:db8::/64"},
HardwareAddress: "bc:1c:b6:3e:b8:25",
},
})
// Updates the ResourceClaim from the driver on the same node as the pod.
@ -456,12 +456,12 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation,
Driver: allocatedResourceClaim.Status.Allocation.Devices.Results[0].Driver,
Pool: allocatedResourceClaim.Status.Allocation.Devices.Results[0].Pool,
Device: allocatedResourceClaim.Status.Allocation.Devices.Results[0].Device,
Conditions: []metav1.Condition{{Type: "e", Status: "f", Message: "g", Reason: "h"}},
Conditions: []metav1.Condition{{Type: "e", Status: "True", Message: "g", Reason: "h", LastTransitionTime: metav1.NewTime(time.Now().Truncate(time.Second))}},
Data: runtime.RawExtension{Raw: []byte(`{"bar":"foo"}`)},
NetworkData: &resourceapi.NetworkDeviceData{
InterfaceName: "inf2",
Addresses: []string{"10.9.8.1/24", "2001:db8::1/64"},
HWAddress: "bc:1c:b6:3e:b8:26",
InterfaceName: "inf2",
Addresses: []string{"10.9.8.1/24", "2001:db8::1/64"},
HardwareAddress: "bc:1c:b6:3e:b8:26",
},
}
updatedResourceClaim2, err := driver.Nodes[scheduledPod.Spec.NodeName].ExamplePlugin.UpdateStatus(ctx, updatedResourceClaim)
@ -469,6 +469,10 @@ var _ = framework.SIGDescribe("node")("DRA", feature.DynamicResourceAllocation,
gomega.Expect(updatedResourceClaim2).ToNot(gomega.BeNil())
gomega.Expect(updatedResourceClaim2.Status.Devices).To(gomega.Equal(updatedResourceClaim.Status.Devices))
getResourceClaim, err := b.f.ClientSet.ResourceV1alpha3().ResourceClaims(b.f.Namespace.Name).Get(ctx, claim.Name, metav1.GetOptions{})
framework.ExpectNoError(err)
gomega.Expect(getResourceClaim).ToNot(gomega.BeNil())
gomega.Expect(getResourceClaim.Status.Devices).To(gomega.Equal(updatedResourceClaim.Status.Devices))
})
}

6
test/integration/resourceclaim/feature_enable_disable_test.go
View File

@ -90,7 +90,7 @@ func TestEnableDisableDRAResourceClaimDeviceStatus(t *testing.T) {
"10.9.8.0/24",
"2001:db8::/64",
},
HWAddress: "ea:9f:cb:40:b1:7b",
HardwareAddress: "ea:9f:cb:40:b1:7b",
},
},
},
@ -162,7 +162,7 @@ func TestEnableDisableDRAResourceClaimDeviceStatus(t *testing.T) {
"10.9.8.0/24",
"2001:db8::/64",
},
HWAddress: "ea:9f:cb:40:b1:7b",
HardwareAddress: "ea:9f:cb:40:b1:7b",
},
},
},
@ -198,7 +198,7 @@ func TestEnableDisableDRAResourceClaimDeviceStatus(t *testing.T) {
"10.9.8.0/24",
"2001:db8::/64",
},
HWAddress: "ea:9f:cb:40:b1:7b",
HardwareAddress: "ea:9f:cb:40:b1:7b",
},
},
},