Skip to content

Commit 5fc7032

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#126156 from pohly/kubelet-test-enhancements
kubelet test enhancements
2 parents fa7fcde + 6604ff9 commit 5fc7032

File tree

7 files changed

+70
-201
lines changed

7 files changed

+70
-201
lines changed

‎pkg/kubelet/apis/podresources/server_v1_test.go‎

Lines changed: 14 additions & 188 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ package podresources
1919
import (
2020
"context"
2121
"fmt"
22-
"reflect"
23-
"sort"
2422
"testing"
2523

24+
"github.com/google/go-cmp/cmp"
25+
"github.com/google/go-cmp/cmp/cmpopts"
26+
2627
v1 "k8s.io/api/core/v1"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2829
"k8s.io/apimachinery/pkg/types"
@@ -239,8 +240,8 @@ func TestListPodResourcesV1(t *testing.T) {
239240
if err != nil {
240241
t.Errorf("want err = %v, got %q", nil, err)
241242
}
242-
if !equalListResponse(tc.expectedResponse, resp) {
243-
t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String())
243+
if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" {
244+
t.Fatal(diff)
244245
}
245246
})
246247
}
@@ -541,8 +542,8 @@ func TestListPodResourcesWithInitContainersV1(t *testing.T) {
541542
if err != nil {
542543
t.Errorf("want err = %v, got %q", nil, err)
543544
}
544-
if !equalListResponse(tc.expectedResponse, resp) {
545-
t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String())
545+
if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" {
546+
t.Fatal(diff)
546547
}
547548
})
548549
}
@@ -678,6 +679,7 @@ func TestAllocatableResources(t *testing.T) {
678679
desc: "no devices, no CPUs, all memory",
679680
allCPUs: []int64{},
680681
allDevices: []*podresourcesapi.ContainerDevices{},
682+
allMemory: allMemory,
681683
expectedAllocatableResourcesResponse: &podresourcesapi.AllocatableResourcesResponse{
682684
Memory: allMemory,
683685
},
@@ -831,8 +833,8 @@ func TestAllocatableResources(t *testing.T) {
831833
t.Errorf("want err = %v, got %q", nil, err)
832834
}
833835

834-
if !equalAllocatableResourcesResponse(tc.expectedAllocatableResourcesResponse, resp) {
835-
t.Errorf("want resp = %s, got %s", tc.expectedAllocatableResourcesResponse.String(), resp.String())
836+
if diff := cmp.Diff(tc.expectedAllocatableResourcesResponse, resp, cmpopts.EquateEmpty()); diff != "" {
837+
t.Fatal(diff)
836838
}
837839
})
838840
}
@@ -1006,8 +1008,8 @@ func TestGetPodResourcesV1(t *testing.T) {
10061008
if err != tc.err {
10071009
t.Errorf("want exit = %v, got %v", tc.err, err)
10081010
} else {
1009-
if !equalGetResponse(tc.expectedResponse, resp) {
1010-
t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String())
1011+
if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" {
1012+
t.Fatal(diff)
10111013
}
10121014
}
10131015
}
@@ -1297,185 +1299,9 @@ func TestGetPodResourcesWithInitContainersV1(t *testing.T) {
12971299
if err != nil {
12981300
t.Errorf("want err = %v, got %q", nil, err)
12991301
}
1300-
if !equalGetResponse(tc.expectedResponse, resp) {
1301-
t.Errorf("want resp = %s, got %s", tc.expectedResponse.String(), resp.String())
1302+
if diff := cmp.Diff(tc.expectedResponse, resp, cmpopts.EquateEmpty()); diff != "" {
1303+
t.Fatal(diff)
13021304
}
13031305
})
13041306
}
13051307
}
1306-
1307-
func equalListResponse(respA, respB *podresourcesapi.ListPodResourcesResponse) bool {
1308-
if len(respA.PodResources) != len(respB.PodResources) {
1309-
return false
1310-
}
1311-
for idx := 0; idx < len(respA.PodResources); idx++ {
1312-
podResA := respA.PodResources[idx]
1313-
podResB := respB.PodResources[idx]
1314-
if podResA.Name != podResB.Name {
1315-
return false
1316-
}
1317-
if podResA.Namespace != podResB.Namespace {
1318-
return false
1319-
}
1320-
if len(podResA.Containers) != len(podResB.Containers) {
1321-
return false
1322-
}
1323-
for jdx := 0; jdx < len(podResA.Containers); jdx++ {
1324-
cntA := podResA.Containers[jdx]
1325-
cntB := podResB.Containers[jdx]
1326-
1327-
if cntA.Name != cntB.Name {
1328-
return false
1329-
}
1330-
if !equalInt64s(cntA.CpuIds, cntB.CpuIds) {
1331-
return false
1332-
}
1333-
1334-
if !equalContainerDevices(cntA.Devices, cntB.Devices) {
1335-
return false
1336-
}
1337-
1338-
if !equalDynamicResources(cntA.DynamicResources, cntB.DynamicResources) {
1339-
return false
1340-
}
1341-
}
1342-
}
1343-
return true
1344-
}
1345-
1346-
func equalDynamicResources(draResA, draResB []*podresourcesapi.DynamicResource) bool {
1347-
if len(draResA) != len(draResB) {
1348-
return false
1349-
}
1350-
1351-
for idx := 0; idx < len(draResA); idx++ {
1352-
cntDraResA := draResA[idx]
1353-
cntDraResB := draResB[idx]
1354-
1355-
if cntDraResA.ClassName != cntDraResB.ClassName {
1356-
return false
1357-
}
1358-
if cntDraResA.ClaimName != cntDraResB.ClaimName {
1359-
return false
1360-
}
1361-
if cntDraResA.ClaimNamespace != cntDraResB.ClaimNamespace {
1362-
return false
1363-
}
1364-
if len(cntDraResA.ClaimResources) != len(cntDraResB.ClaimResources) {
1365-
return false
1366-
}
1367-
for i := 0; i < len(cntDraResA.ClaimResources); i++ {
1368-
claimResA := cntDraResA.ClaimResources[i]
1369-
claimResB := cntDraResB.ClaimResources[i]
1370-
if len(claimResA.CDIDevices) != len(claimResB.CDIDevices) {
1371-
return false
1372-
}
1373-
for y := 0; y < len(claimResA.CDIDevices); y++ {
1374-
cdiDeviceA := claimResA.CDIDevices[y]
1375-
cdiDeviceB := claimResB.CDIDevices[y]
1376-
if cdiDeviceA.Name != cdiDeviceB.Name {
1377-
return false
1378-
}
1379-
}
1380-
}
1381-
}
1382-
1383-
return true
1384-
}
1385-
1386-
func equalContainerDevices(devA, devB []*podresourcesapi.ContainerDevices) bool {
1387-
if len(devA) != len(devB) {
1388-
return false
1389-
}
1390-
1391-
for idx := 0; idx < len(devA); idx++ {
1392-
cntDevA := devA[idx]
1393-
cntDevB := devB[idx]
1394-
1395-
if cntDevA.ResourceName != cntDevB.ResourceName {
1396-
return false
1397-
}
1398-
if !equalTopology(cntDevA.Topology, cntDevB.Topology) {
1399-
return false
1400-
}
1401-
if !equalStrings(cntDevA.DeviceIds, cntDevB.DeviceIds) {
1402-
return false
1403-
}
1404-
}
1405-
1406-
return true
1407-
}
1408-
1409-
func equalInt64s(a, b []int64) bool {
1410-
if len(a) != len(b) {
1411-
return false
1412-
}
1413-
aCopy := append([]int64{}, a...)
1414-
sort.Slice(aCopy, func(i, j int) bool { return aCopy[i] < aCopy[j] })
1415-
bCopy := append([]int64{}, b...)
1416-
sort.Slice(bCopy, func(i, j int) bool { return bCopy[i] < bCopy[j] })
1417-
return reflect.DeepEqual(aCopy, bCopy)
1418-
}
1419-
1420-
func equalStrings(a, b []string) bool {
1421-
if len(a) != len(b) {
1422-
return false
1423-
}
1424-
aCopy := append([]string{}, a...)
1425-
sort.Strings(aCopy)
1426-
bCopy := append([]string{}, b...)
1427-
sort.Strings(bCopy)
1428-
return reflect.DeepEqual(aCopy, bCopy)
1429-
}
1430-
1431-
func equalTopology(a, b *podresourcesapi.TopologyInfo) bool {
1432-
if a == nil && b != nil {
1433-
return false
1434-
}
1435-
if a != nil && b == nil {
1436-
return false
1437-
}
1438-
return reflect.DeepEqual(a, b)
1439-
}
1440-
1441-
func equalAllocatableResourcesResponse(respA, respB *podresourcesapi.AllocatableResourcesResponse) bool {
1442-
if !equalInt64s(respA.CpuIds, respB.CpuIds) {
1443-
return false
1444-
}
1445-
return equalContainerDevices(respA.Devices, respB.Devices)
1446-
}
1447-
1448-
func equalGetResponse(ResA, ResB *podresourcesapi.GetPodResourcesResponse) bool {
1449-
podResA := ResA.PodResources
1450-
podResB := ResB.PodResources
1451-
if podResA.Name != podResB.Name {
1452-
return false
1453-
}
1454-
if podResA.Namespace != podResB.Namespace {
1455-
return false
1456-
}
1457-
if len(podResA.Containers) != len(podResB.Containers) {
1458-
return false
1459-
}
1460-
for jdx := 0; jdx < len(podResA.Containers); jdx++ {
1461-
cntA := podResA.Containers[jdx]
1462-
cntB := podResB.Containers[jdx]
1463-
1464-
if cntA.Name != cntB.Name {
1465-
return false
1466-
}
1467-
if !equalInt64s(cntA.CpuIds, cntB.CpuIds) {
1468-
return false
1469-
}
1470-
1471-
if !equalContainerDevices(cntA.Devices, cntB.Devices) {
1472-
return false
1473-
}
1474-
1475-
if !equalDynamicResources(cntA.DynamicResources, cntB.DynamicResources) {
1476-
return false
1477-
}
1478-
1479-
}
1480-
return true
1481-
}

‎pkg/kubelet/checkpointmanager/checksum/checksum.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ type Checksum uint64
2828

2929
// Verify verifies that passed checksum is same as calculated checksum
3030
func (cs Checksum) Verify(data interface{}) error {
31-
if cs != New(data) {
32-
return errors.ErrCorruptCheckpoint
31+
actualCS := New(data)
32+
if cs != actualCS {
33+
return &errors.CorruptCheckpointError{ActualCS: uint64(actualCS), ExpectedCS: uint64(cs)}
3334
}
3435
return nil
3536
}

‎pkg/kubelet/checkpointmanager/errors/errors.go‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,28 @@ package errors
1818

1919
import "fmt"
2020

21-
// ErrCorruptCheckpoint error is reported when checksum does not match
22-
var ErrCorruptCheckpoint = fmt.Errorf("checkpoint is corrupted")
21+
// ErrCorruptCheckpoint error is reported when checksum does not match.
22+
// Check for it with:
23+
//
24+
// var csErr *CorruptCheckpointError
25+
// if errors.As(err, &csErr) { ... }
26+
// if errors.Is(err, CorruptCheckpointError{}) { ... }
27+
type CorruptCheckpointError struct {
28+
ActualCS, ExpectedCS uint64
29+
}
30+
31+
func (err CorruptCheckpointError) Error() string {
32+
return "checkpoint is corrupted"
33+
}
34+
35+
func (err CorruptCheckpointError) Is(target error) bool {
36+
switch target.(type) {
37+
case *CorruptCheckpointError, CorruptCheckpointError:
38+
return true
39+
default:
40+
return false
41+
}
42+
}
2343

2444
// ErrCheckpointNotFound is reported when checkpoint is not found for a given key
2545
var ErrCheckpointNotFound = fmt.Errorf("checkpoint is not found")

‎pkg/kubelet/cm/cpumanager/state/checkpoint.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,12 @@ func (cp *CPUManagerCheckpointV1) VerifyChecksum() error {
110110

111111
hash := fnv.New32a()
112112
fmt.Fprintf(hash, "%v", object)
113-
if cp.Checksum != checksum.Checksum(hash.Sum32()) {
114-
return errors.ErrCorruptCheckpoint
113+
actualCS := checksum.Checksum(hash.Sum32())
114+
if cp.Checksum != actualCS {
115+
return &errors.CorruptCheckpointError{
116+
ActualCS: uint64(actualCS),
117+
ExpectedCS: uint64(cp.Checksum),
118+
}
115119
}
116120

117121
return nil

‎pkg/kubelet/cm/dra/state/checkpoint.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@ package state
1818

1919
import (
2020
"encoding/json"
21+
"errors"
2122
"fmt"
2223
"hash/fnv"
2324
"strings"
2425

2526
"k8s.io/apimachinery/pkg/util/dump"
2627
"k8s.io/kubernetes/pkg/kubelet/checkpointmanager"
2728
"k8s.io/kubernetes/pkg/kubelet/checkpointmanager/checksum"
28-
"k8s.io/kubernetes/pkg/kubelet/checkpointmanager/errors"
29+
cmerrors "k8s.io/kubernetes/pkg/kubelet/checkpointmanager/errors"
2930
)
3031

3132
var _ checkpointmanager.Checkpoint = &DRAManagerCheckpoint{}
@@ -79,7 +80,7 @@ func (dc *DRAManagerCheckpoint) VerifyChecksum() error {
7980
ck := dc.Checksum
8081
dc.Checksum = 0
8182
err := ck.Verify(dc)
82-
if err == errors.ErrCorruptCheckpoint {
83+
if errors.Is(err, cmerrors.CorruptCheckpointError{}) {
8384
// Verify with old structs without ResourceHandles field
8485
// TODO: remove in Beta
8586
err = verifyChecksumWithoutResourceHandles(dc, ck)
@@ -115,8 +116,12 @@ func verifyChecksumWithoutResourceHandles(dc *DRAManagerCheckpoint, checkSum che
115116
object = strings.Replace(object, "ClaimInfoStateListWithoutResourceHandles", "ClaimInfoStateList", 1)
116117
hash := fnv.New32a()
117118
fmt.Fprintf(hash, "%v", object)
118-
if checkSum != checksum.Checksum(hash.Sum32()) {
119-
return errors.ErrCorruptCheckpoint
119+
actualCS := checksum.Checksum(hash.Sum32())
120+
if checkSum != actualCS {
121+
return &cmerrors.CorruptCheckpointError{
122+
ActualCS: uint64(actualCS),
123+
ExpectedCS: uint64(checkSum),
124+
}
120125
}
121126
return nil
122127
}

‎pkg/kubelet/cm/dra/state/state_checkpoint.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (sc *stateCheckpoint) GetOrCreate() (ClaimInfoStateList, error) {
126126
return ClaimInfoStateList{}, nil
127127
}
128128
if err != nil {
129-
return nil, fmt.Errorf("failed to get checkpoint %v: %v", sc.checkpointName, err)
129+
return nil, fmt.Errorf("failed to get checkpoint %v: %w", sc.checkpointName, err)
130130
}
131131

132132
return checkpoint.Entries, nil

0 commit comments

Comments
 (0)