From 4f91a69f2b08ecbd2f43e9c3cdc0ea5dd32fd061 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 27 May 2025 16:27:37 +0200 Subject: [PATCH] DRA integration: move and extend device status test This moves the enabled/disabled test into the common test/integration/dra which simplifies the code a bit and amortizes the cost of starting the apiserver because several different tests can use the same instance, running in parallel. While at it, setting the status via SSA also gets tested. --- test/integration/dra/dra_test.go | 200 ++++++++++++++++ .../feature_enable_disable_test.go | 223 ------------------ test/integration/resourceclaim/main_test.go | 27 --- 3 files changed, 200 insertions(+), 250 deletions(-) delete mode 100644 test/integration/resourceclaim/feature_enable_disable_test.go delete mode 100644 test/integration/resourceclaim/main_test.go diff --git a/test/integration/dra/dra_test.go b/test/integration/dra/dra_test.go index 8a0bc352162..9852c04954d 100644 --- a/test/integration/dra/dra_test.go +++ b/test/integration/dra/dra_test.go @@ -17,7 +17,9 @@ limitations under the License. package dra import ( + "bytes" "context" + "encoding/json" "errors" "fmt" "regexp" @@ -41,8 +43,10 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilfeature "k8s.io/apiserver/pkg/util/feature" + resourceapiac "k8s.io/client-go/applyconfigurations/resource/v1beta1" "k8s.io/client-go/informers" "k8s.io/component-base/featuregate" featuregatetesting "k8s.io/component-base/featuregate/testing" @@ -192,6 +196,28 @@ func TestDRA(t *testing.T) { tCtx.Run("APIDisabled", testAPIDisabled) }, }, + "GA": { + // TODO (https://github.com/kubernetes/kubernetes/issues/131903): remove enabling the beta when promoting to GA. + apis: map[schema.GroupVersion]bool{ + resourceapi.SchemeGroupVersion: true, + resourcev1beta2.SchemeGroupVersion: true, + }, + features: map[featuregate.Feature]bool{ + features.DynamicResourceAllocation: true, + // TODO: replace specific list with AllBeta once DRA is not beta. + features.DRAResourceClaimDeviceStatus: false, + // featuregate.Feature("AllBeta"): false, + }, + f: func(tCtx ktesting.TContext) { + tCtx.Run("AdminAccess", func(tCtx ktesting.TContext) { testAdminAccess(tCtx, false) }) + tCtx.Run("PrioritizedList", func(tCtx ktesting.TContext) { testPrioritizedList(tCtx, false) }) + tCtx.Run("Pod", func(tCtx ktesting.TContext) { testPod(tCtx, true) }) + tCtx.Run("PublishResourceSlices", func(tCtx ktesting.TContext) { + testPublishResourceSlices(tCtx, features.DRADeviceTaints, features.DRAPartitionableDevices) + }) + tCtx.Run("ResourceClaimDeviceStatus", func(tCtx ktesting.TContext) { testResourceClaimDeviceStatus(tCtx, false) }) + }, + }, "core": { apis: map[schema.GroupVersion]bool{ resourceapi.SchemeGroupVersion: true, @@ -205,6 +231,7 @@ func TestDRA(t *testing.T) { tCtx.Run("PublishResourceSlices", func(tCtx ktesting.TContext) { testPublishResourceSlices(tCtx, features.DRADeviceTaints, features.DRAPartitionableDevices) }) + tCtx.Run("ResourceClaimDeviceStatus", func(tCtx ktesting.TContext) { testResourceClaimDeviceStatus(tCtx, true) }) }, }, "v1beta1": { @@ -250,6 +277,7 @@ func TestDRA(t *testing.T) { tCtx.Run("Convert", testConvert) tCtx.Run("PrioritizedList", func(tCtx ktesting.TContext) { testPrioritizedList(tCtx, true) }) tCtx.Run("PublishResourceSlices", func(tCtx ktesting.TContext) { testPublishResourceSlices(tCtx) }) + tCtx.Run("ResourceClaimDeviceStatus", func(tCtx ktesting.TContext) { testResourceClaimDeviceStatus(tCtx, true) }) tCtx.Run("MaxResourceSlice", testMaxResourceSlice) }, }, @@ -668,6 +696,178 @@ func testPublishResourceSlices(tCtx ktesting.TContext, disabledFeatures ...featu } +// testResourceClaimDeviceStatus creates a ResourceClaim with an invalid device (not allocated device) +// and checks that the object is not validated (feature enabled) resp. accepted without the field (disabled). +// +// When enabled, it tries server-side-apply (SSA) with different clients. This is what DRA drivers should be using. +func testResourceClaimDeviceStatus(tCtx ktesting.TContext, enabled bool) { + namespace := createTestNamespace(tCtx, nil) + + claim := &resourceapi.ResourceClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimName, + }, + Spec: resourceapi.ResourceClaimSpec{ + Devices: resourceapi.DeviceClaim{ + Requests: []resourceapi.DeviceRequest{ + { + Name: "foo", + DeviceClassName: "foo", + }, + }, + }, + }, + } + + claim, err := tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).Create(tCtx, claim, metav1.CreateOptions{}) + tCtx.ExpectNoError(err, "create ResourceClaim") + + deviceStatus := []resourceapi.AllocatedDeviceStatus{{ + Driver: "one", + Pool: "global", + Device: "my-device", + Data: &runtime.RawExtension{ + Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), + }, + NetworkData: &resourceapi.NetworkDeviceData{ + InterfaceName: "net-1", + IPs: []string{ + "10.9.8.0/24", + "2001:db8::/64", + }, + HardwareAddress: "ea:9f:cb:40:b1:7b", + }, + }} + claim.Status.Devices = deviceStatus + updatedClaim, err := tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).UpdateStatus(tCtx, claim, metav1.UpdateOptions{}) + if !enabled { + tCtx.ExpectNoError(err, "updating the status with an invalid AllocatedDeviceStatus should have worked because the field should have been dropped") + require.Empty(tCtx, updatedClaim.Status.Devices, "field should have been dropped") + return + } + + // Tests for enabled feature follow. + + if err == nil { + tCtx.Fatal("updating the status with an invalid AllocatedDeviceStatus should have failed and didn't") + } + + // Add an allocation result. + claim.Status.Allocation = &resourceapi.AllocationResult{ + Devices: resourceapi.DeviceAllocationResult{ + Results: []resourceapi.DeviceRequestAllocationResult{ + { + Request: "foo", + Driver: "one", + Pool: "global", + Device: "my-device", + }, + { + Request: "foo", + Driver: "two", + Pool: "global", + Device: "another-device", + }, + { + Request: "foo", + Driver: "three", + Pool: "global", + Device: "my-device", + }, + }, + }, + } + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).UpdateStatus(tCtx, claim, metav1.UpdateOptions{}) + tCtx.ExpectNoError(err, "add allocation result") + + // Now adding the device status should work. + claim.Status.Devices = deviceStatus + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).UpdateStatus(tCtx, claim, metav1.UpdateOptions{}) + tCtx.ExpectNoError(err, "add device status") + require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after adding device status") + + // Strip the RawExtension. SSA re-encodes it, which causes negligble differences that nonetheless break assert.Equal. + claim.Status.Devices[0].Data = nil + deviceStatus[0].Data = nil + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).UpdateStatus(tCtx, claim, metav1.UpdateOptions{}) + tCtx.ExpectNoError(err, "add device status") + require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after stripping RawExtension") + + // Exercise SSA. + deviceStatusAC := resourceapiac.AllocatedDeviceStatus(). + WithDriver("two"). + WithPool("global"). + WithDevice("another-device"). + WithNetworkData(resourceapiac.NetworkDeviceData().WithInterfaceName("net-2")) + deviceStatus = append(deviceStatus, resourceapi.AllocatedDeviceStatus{ + Driver: "two", + Pool: "global", + Device: "another-device", + NetworkData: &resourceapi.NetworkDeviceData{ + InterfaceName: "net-2", + }, + }) + claimAC := resourceapiac.ResourceClaim(claim.Name, claim.Namespace). + WithStatus(resourceapiac.ResourceClaimStatus().WithDevices(deviceStatusAC)) + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).ApplyStatus(tCtx, claimAC, metav1.ApplyOptions{ + Force: true, + FieldManager: "manager-1", + }) + tCtx.ExpectNoError(err, "apply device status two") + require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after applying device status two") + + deviceStatusAC = resourceapiac.AllocatedDeviceStatus(). + WithDriver("three"). + WithPool("global"). + WithDevice("my-device"). + WithNetworkData(resourceapiac.NetworkDeviceData().WithInterfaceName("net-3")) + deviceStatus = append(deviceStatus, resourceapi.AllocatedDeviceStatus{ + Driver: "three", + Pool: "global", + Device: "my-device", + NetworkData: &resourceapi.NetworkDeviceData{ + InterfaceName: "net-3", + }, + }) + claimAC = resourceapiac.ResourceClaim(claim.Name, claim.Namespace). + WithStatus(resourceapiac.ResourceClaimStatus().WithDevices(deviceStatusAC)) + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).ApplyStatus(tCtx, claimAC, metav1.ApplyOptions{ + Force: true, + FieldManager: "manager-2", + }) + tCtx.ExpectNoError(err, "apply device status three") + require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after applying device status three") + var buffer bytes.Buffer + encoder := json.NewEncoder(&buffer) + encoder.SetIndent(" ", " ") + tCtx.ExpectNoError(encoder.Encode(claim)) + tCtx.Logf("Final ResourceClaim:\n%s", buffer.String()) + + // Update one entry, remove the other. + deviceStatusAC = resourceapiac.AllocatedDeviceStatus(). + WithDriver("two"). + WithPool("global"). + WithDevice("another-device"). + WithNetworkData(resourceapiac.NetworkDeviceData().WithInterfaceName("yet-another-net")) + deviceStatus[1].NetworkData.InterfaceName = "yet-another-net" + claimAC = resourceapiac.ResourceClaim(claim.Name, claim.Namespace). + WithStatus(resourceapiac.ResourceClaimStatus().WithDevices(deviceStatusAC)) + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).ApplyStatus(tCtx, claimAC, metav1.ApplyOptions{ + Force: true, + FieldManager: "manager-1", + }) + tCtx.ExpectNoError(err, "update device status two") + require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after updating device status two") + claimAC = resourceapiac.ResourceClaim(claim.Name, claim.Namespace) + deviceStatus = deviceStatus[0:2] + claim, err = tCtx.Client().ResourceV1beta1().ResourceClaims(namespace).ApplyStatus(tCtx, claimAC, metav1.ApplyOptions{ + Force: true, + FieldManager: "manager-2", + }) + tCtx.ExpectNoError(err, "remove device status three") + require.Equal(tCtx, deviceStatus, claim.Status.Devices, "after removing device status three") +} + // testMaxResourceSlice creates a ResourceSlice that is as large as possible // and prints some information about it. func testMaxResourceSlice(tCtx ktesting.TContext) { diff --git a/test/integration/resourceclaim/feature_enable_disable_test.go b/test/integration/resourceclaim/feature_enable_disable_test.go deleted file mode 100644 index 844a6e17af2..00000000000 --- a/test/integration/resourceclaim/feature_enable_disable_test.go +++ /dev/null @@ -1,223 +0,0 @@ -/* -Copyright 2024 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resourceclaim - -import ( - "context" - "fmt" - "testing" - - "k8s.io/api/resource/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - clientset "k8s.io/client-go/kubernetes" - kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" - "k8s.io/kubernetes/pkg/features" - "k8s.io/kubernetes/test/integration/framework" -) - -// TestEnableDisableDRAResourceClaimDeviceStatus first test the feature gate disabled -// by creating a ResourceClaim with an invalid device (not allocated device) and checks -// the object is not validated. -// Then the feature gate is created, and an attempt to create similar invalid ResourceClaim -// is done with no success. -func TestEnableDisableDRAResourceClaimDeviceStatus(t *testing.T) { - // start etcd instance - etcdOptions := framework.SharedEtcd() - apiServerOptions := kubeapiservertesting.NewDefaultTestServerOptions() - // apiserver with the feature disabled - server1 := kubeapiservertesting.StartTestServerOrDie(t, apiServerOptions, - []string{ - fmt.Sprintf("--runtime-config=%s=true", v1beta1.SchemeGroupVersion), - fmt.Sprintf("--feature-gates=%s=true,%s=false", features.DynamicResourceAllocation, features.DRAResourceClaimDeviceStatus), - }, - etcdOptions) - client1, err := clientset.NewForConfig(server1.ClientConfig) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - ns := framework.CreateNamespaceOrDie(client1, "test-enable-dra-resourceclaim-device-status", t) - - rcDisabledName := "test-enable-dra-resourceclaim-device-status-rc-disabled" - rcDisabled := &v1beta1.ResourceClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: rcDisabledName, - }, - Spec: v1beta1.ResourceClaimSpec{ - Devices: v1beta1.DeviceClaim{ - Requests: []v1beta1.DeviceRequest{ - { - Name: "foo", - DeviceClassName: "foo", - Count: 1, - AllocationMode: v1beta1.DeviceAllocationModeExactCount, - }, - }, - }, - }, - } - - if _, err := client1.ResourceV1beta1().ResourceClaims(ns.Name).Create(context.TODO(), rcDisabled, metav1.CreateOptions{}); err != nil { - t.Fatal(err) - } - - rcDisabled.Status = v1beta1.ResourceClaimStatus{ - Devices: []v1beta1.AllocatedDeviceStatus{ - { - Driver: "foo", - Pool: "foo", - Device: "foo", - Data: &runtime.RawExtension{ - Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), - }, - NetworkData: &v1beta1.NetworkDeviceData{ - InterfaceName: "net-1", - IPs: []string{ - "10.9.8.0/24", - "2001:db8::/64", - }, - HardwareAddress: "ea:9f:cb:40:b1:7b", - }, - }, - }, - } - if _, err := client1.ResourceV1beta1().ResourceClaims(ns.Name).UpdateStatus(context.TODO(), rcDisabled, metav1.UpdateOptions{}); err != nil { - t.Fatal(err) - } - - rcDisabled, err = client1.ResourceV1beta1().ResourceClaims(ns.Name).Get(context.TODO(), rcDisabledName, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - // No devices as the Kubernetes api-server dropped these fields since the feature is disabled. - if len(rcDisabled.Status.Devices) != 0 { - t.Fatalf("expected 0 Device in status got %d", len(rcDisabled.Status.Devices)) - } - - // shutdown apiserver with the feature disabled - server1.TearDownFn() - - // apiserver with the feature enabled - server2 := kubeapiservertesting.StartTestServerOrDie(t, apiServerOptions, - []string{ - fmt.Sprintf("--runtime-config=%s=true", v1beta1.SchemeGroupVersion), - fmt.Sprintf("--feature-gates=%s=true,%s=true", features.DynamicResourceAllocation, features.DRAResourceClaimDeviceStatus), - }, - etcdOptions) - client2, err := clientset.NewForConfig(server2.ClientConfig) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - - rcEnabledName := "test-enable-dra-resourceclaim-device-status-rc-enabled" - rcEnabled := &v1beta1.ResourceClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: rcEnabledName, - }, - Spec: v1beta1.ResourceClaimSpec{ - Devices: v1beta1.DeviceClaim{ - Requests: []v1beta1.DeviceRequest{ - { - Name: "bar", - DeviceClassName: "bar", - Count: 1, - AllocationMode: v1beta1.DeviceAllocationModeExactCount, - }, - }, - }, - }, - } - - if _, err := client2.ResourceV1beta1().ResourceClaims(ns.Name).Create(context.TODO(), rcEnabled, metav1.CreateOptions{}); err != nil { - t.Fatal(err) - } - - // Tests the validation is enabled. - // validation will refuse this update as the device is not allocated. - rcEnabled.Status = v1beta1.ResourceClaimStatus{ - Devices: []v1beta1.AllocatedDeviceStatus{ - { - Driver: "bar", - Pool: "bar", - Device: "bar", - Data: &runtime.RawExtension{ - Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), - }, - NetworkData: &v1beta1.NetworkDeviceData{ - InterfaceName: "net-1", - IPs: []string{ - "10.9.8.0/24", - "2001:db8::/64", - }, - HardwareAddress: "ea:9f:cb:40:b1:7b", - }, - }, - }, - } - if _, err := client2.ResourceV1beta1().ResourceClaims(ns.Name).UpdateStatus(context.TODO(), rcEnabled, metav1.UpdateOptions{}); err == nil { - t.Fatalf("Expected error (must be an allocated device in the claim)") - } - - rcEnabled.Status = v1beta1.ResourceClaimStatus{ - Allocation: &v1beta1.AllocationResult{ - Devices: v1beta1.DeviceAllocationResult{ - Results: []v1beta1.DeviceRequestAllocationResult{ - { - Request: "bar", - Driver: "bar", - Pool: "bar", - Device: "bar", - }, - }, - }, - }, - Devices: []v1beta1.AllocatedDeviceStatus{ - { - Driver: "bar", - Pool: "bar", - Device: "bar", - Data: &runtime.RawExtension{ - Raw: []byte(`{"kind": "foo", "apiVersion": "dra.example.com/v1"}`), - }, - NetworkData: &v1beta1.NetworkDeviceData{ - InterfaceName: "net-1", - IPs: []string{ - "10.9.8.0/24", - "2001:db8::/64", - }, - HardwareAddress: "ea:9f:cb:40:b1:7b", - }, - }, - }, - } - if _, err := client2.ResourceV1beta1().ResourceClaims(ns.Name).UpdateStatus(context.TODO(), rcEnabled, metav1.UpdateOptions{}); err != nil { - t.Fatal(err) - } - - // Tests the field is enabled. - rcEnabled, err = client2.ResourceV1beta1().ResourceClaims(ns.Name).Get(context.TODO(), rcEnabledName, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - if len(rcEnabled.Status.Devices) != 1 { - t.Fatalf("expected 1 Device in status got %d", len(rcEnabled.Status.Devices)) - } - - // shutdown apiserver with the feature enabled - server2.TearDownFn() -} diff --git a/test/integration/resourceclaim/main_test.go b/test/integration/resourceclaim/main_test.go deleted file mode 100644 index 7b48de60741..00000000000 --- a/test/integration/resourceclaim/main_test.go +++ /dev/null @@ -1,27 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resourceclaim - -import ( - "testing" - - "k8s.io/kubernetes/test/integration/framework" -) - -func TestMain(m *testing.M) { - framework.EtcdMain(m.Run) -}