From 33d8e739f5e7936d998047cf683b71415829b381 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 17 Jul 2024 14:20:16 +0200 Subject: [PATCH] kubelet: enhance podresources tests The manual deep comparison code is hard to maintain (would need to be updated in https://github.com/kubernetes/kubernetes/pull/125488) and error prone. In fact, one test case failed when doing a full automatic comparison with cmp.Diff because it wasn't setting allMemory. --- .../apis/podresources/server_v1_test.go | 202 ++---------------- 1 file changed, 14 insertions(+), 188 deletions(-) diff --git a/pkg/kubelet/apis/podresources/server_v1_test.go b/pkg/kubelet/apis/podresources/server_v1_test.go index aaa560fc4f04c..2bc69a000041d 100644 --- a/pkg/kubelet/apis/podresources/server_v1_test.go +++ b/pkg/kubelet/apis/podresources/server_v1_test.go @@ -19,10 +19,11 @@ package podresources import ( "context" "fmt" - "reflect" - "sort" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -239,8 +240,8 @@ func TestListPodResourcesV1(t *testing.T) { if err != nil { t.Errorf("want err = %v, got %q", nil, err) } - if !equalListResponse(tc.expectedResponse, resp) { - t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String()) + if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" { + t.Fatal(diff) } }) } @@ -541,8 +542,8 @@ func TestListPodResourcesWithInitContainersV1(t *testing.T) { if err != nil { t.Errorf("want err = %v, got %q", nil, err) } - if !equalListResponse(tc.expectedResponse, resp) { - t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String()) + if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" { + t.Fatal(diff) } }) } @@ -678,6 +679,7 @@ func TestAllocatableResources(t *testing.T) { desc: "no devices, no CPUs, all memory", allCPUs: []int64{}, allDevices: []*podresourcesapi.ContainerDevices{}, + allMemory: allMemory, expectedAllocatableResourcesResponse: &podresourcesapi.AllocatableResourcesResponse{ Memory: allMemory, }, @@ -831,8 +833,8 @@ func TestAllocatableResources(t *testing.T) { t.Errorf("want err = %v, got %q", nil, err) } - if !equalAllocatableResourcesResponse(tc.expectedAllocatableResourcesResponse, resp) { - t.Errorf("want resp = %s, got %s", tc.expectedAllocatableResourcesResponse.String(), resp.String()) + if diff := cmp.Diff(tc.expectedAllocatableResourcesResponse, resp, cmpopts.EquateEmpty()); diff != "" { + t.Fatal(diff) } }) } @@ -1006,8 +1008,8 @@ func TestGetPodResourcesV1(t *testing.T) { if err != tc.err { t.Errorf("want exit = %v, got %v", tc.err, err) } else { - if !equalGetResponse(tc.expectedResponse, resp) { - t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String()) + if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" { + t.Fatal(diff) } } } @@ -1297,185 +1299,9 @@ func TestGetPodResourcesWithInitContainersV1(t *testing.T) { if err != nil { t.Errorf("want err = %v, got %q", nil, err) } - if !equalGetResponse(tc.expectedResponse, resp) { - t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String()) + if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" { + t.Fatal(diff) } }) } } - -func equalListResponse(respA, respB *podresourcesapi.ListPodResourcesResponse) bool { - if len(respA.PodResources) != len(respB.PodResources) { - return false - } - for idx := 0; idx < len(respA.PodResources); idx++ { - podResA := respA.PodResources[idx] - podResB := respB.PodResources[idx] - if podResA.Name != podResB.Name { - return false - } - if podResA.Namespace != podResB.Namespace { - return false - } - if len(podResA.Containers) != len(podResB.Containers) { - return false - } - for jdx := 0; jdx < len(podResA.Containers); jdx++ { - cntA := podResA.Containers[jdx] - cntB := podResB.Containers[jdx] - - if cntA.Name != cntB.Name { - return false - } - if !equalInt64s(cntA.CpuIds, cntB.CpuIds) { - return false - } - - if !equalContainerDevices(cntA.Devices, cntB.Devices) { - return false - } - - if !equalDynamicResources(cntA.DynamicResources, cntB.DynamicResources) { - return false - } - } - } - return true -} - -func equalDynamicResources(draResA, draResB []*podresourcesapi.DynamicResource) bool { - if len(draResA) != len(draResB) { - return false - } - - for idx := 0; idx < len(draResA); idx++ { - cntDraResA := draResA[idx] - cntDraResB := draResB[idx] - - if cntDraResA.ClassName != cntDraResB.ClassName { - return false - } - if cntDraResA.ClaimName != cntDraResB.ClaimName { - return false - } - if cntDraResA.ClaimNamespace != cntDraResB.ClaimNamespace { - return false - } - if len(cntDraResA.ClaimResources) != len(cntDraResB.ClaimResources) { - return false - } - for i := 0; i < len(cntDraResA.ClaimResources); i++ { - claimResA := cntDraResA.ClaimResources[i] - claimResB := cntDraResB.ClaimResources[i] - if len(claimResA.CDIDevices) != len(claimResB.CDIDevices) { - return false - } - for y := 0; y < len(claimResA.CDIDevices); y++ { - cdiDeviceA := claimResA.CDIDevices[y] - cdiDeviceB := claimResB.CDIDevices[y] - if cdiDeviceA.Name != cdiDeviceB.Name { - return false - } - } - } - } - - return true -} - -func equalContainerDevices(devA, devB []*podresourcesapi.ContainerDevices) bool { - if len(devA) != len(devB) { - return false - } - - for idx := 0; idx < len(devA); idx++ { - cntDevA := devA[idx] - cntDevB := devB[idx] - - if cntDevA.ResourceName != cntDevB.ResourceName { - return false - } - if !equalTopology(cntDevA.Topology, cntDevB.Topology) { - return false - } - if !equalStrings(cntDevA.DeviceIds, cntDevB.DeviceIds) { - return false - } - } - - return true -} - -func equalInt64s(a, b []int64) bool { - if len(a) != len(b) { - return false - } - aCopy := append([]int64{}, a...) - sort.Slice(aCopy, func(i, j int) bool { return aCopy[i] < aCopy[j] }) - bCopy := append([]int64{}, b...) - sort.Slice(bCopy, func(i, j int) bool { return bCopy[i] < bCopy[j] }) - return reflect.DeepEqual(aCopy, bCopy) -} - -func equalStrings(a, b []string) bool { - if len(a) != len(b) { - return false - } - aCopy := append([]string{}, a...) - sort.Strings(aCopy) - bCopy := append([]string{}, b...) - sort.Strings(bCopy) - return reflect.DeepEqual(aCopy, bCopy) -} - -func equalTopology(a, b *podresourcesapi.TopologyInfo) bool { - if a == nil && b != nil { - return false - } - if a != nil && b == nil { - return false - } - return reflect.DeepEqual(a, b) -} - -func equalAllocatableResourcesResponse(respA, respB *podresourcesapi.AllocatableResourcesResponse) bool { - if !equalInt64s(respA.CpuIds, respB.CpuIds) { - return false - } - return equalContainerDevices(respA.Devices, respB.Devices) -} - -func equalGetResponse(ResA, ResB *podresourcesapi.GetPodResourcesResponse) bool { - podResA := ResA.PodResources - podResB := ResB.PodResources - if podResA.Name != podResB.Name { - return false - } - if podResA.Namespace != podResB.Namespace { - return false - } - if len(podResA.Containers) != len(podResB.Containers) { - return false - } - for jdx := 0; jdx < len(podResA.Containers); jdx++ { - cntA := podResA.Containers[jdx] - cntB := podResB.Containers[jdx] - - if cntA.Name != cntB.Name { - return false - } - if !equalInt64s(cntA.CpuIds, cntB.CpuIds) { - return false - } - - if !equalContainerDevices(cntA.Devices, cntB.Devices) { - return false - } - - if !equalDynamicResources(cntA.DynamicResources, cntB.DynamicResources) { - return false - } - - } - return true -}