Skip to content

Commit ffe2ce2

Browse files
committed
nrt: test: ensure generation is updated correctly
Add a unit test mainly for GetCachedNRTCopy() to verify the returned generation. To help with the verification, make FlushNodes not only report about the new generation (which only updated there) but also return it so we'd be able to compare the values. Signed-off-by: Shereen Haj <[email protected]>
1 parent 4bf93e9 commit ffe2ce2

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

pkg/noderesourcetopology/cache/overreserve.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ func (ov *OverReserve) Resync() {
340340
}
341341

342342
// FlushNodes drops all the cached information about a given node, resetting its state clean.
343-
func (ov *OverReserve) FlushNodes(lh logr.Logger, nrts ...*topologyv1alpha2.NodeResourceTopology) {
343+
func (ov *OverReserve) FlushNodes(lh logr.Logger, nrts ...*topologyv1alpha2.NodeResourceTopology) uint64 {
344344
ov.lock.Lock()
345345
defer ov.lock.Unlock()
346346

@@ -354,12 +354,14 @@ func (ov *OverReserve) FlushNodes(lh logr.Logger, nrts ...*topologyv1alpha2.Node
354354
}
355355

356356
if len(nrts) == 0 {
357-
return
357+
return ov.generation
358358
}
359359

360360
// increase only if we mutated the internal state
361361
ov.generation += 1
362362
lh.V(2).Info("generation", "new", ov.generation)
363+
return ov.generation
364+
363365
}
364366

365367
// to be used only in tests

pkg/noderesourcetopology/cache/overreserve_test.go

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"k8s.io/apimachinery/pkg/api/equality"
3131
"k8s.io/apimachinery/pkg/api/resource"
3232
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
33+
"k8s.io/apimachinery/pkg/runtime"
3334
podlisterv1 "k8s.io/client-go/listers/core/v1"
3435
"k8s.io/klog/v2"
3536

@@ -434,17 +435,35 @@ func TestFlush(t *testing.T) {
434435

435436
lh := klog.Background()
436437

437-
nrtCache.FlushNodes(lh, expectedNodeTopology.DeepCopy())
438+
expectedGen := nrtCache.generation + 1
439+
gen1 := nrtCache.FlushNodes(lh, expectedNodeTopology.DeepCopy())
440+
if gen1 != expectedGen {
441+
t.Fatalf("generation is expected to increase once after flushing a dirty node\ngot %d expected %d", gen1, expectedGen)
442+
}
438443

439444
dirtyNodes := nrtCache.GetDesyncedNodes(lh)
440445
if dirtyNodes.Len() != 0 {
441446
t.Errorf("dirty nodes after flush: %v", dirtyNodes)
442447
}
443448

444-
nrtObj, _ := nrtCache.GetCachedNRTCopy(context.Background(), "node1", testPod)
449+
nrtObj, nrtInfo := nrtCache.GetCachedNRTCopy(context.Background(), "node1", testPod)
445450
if !reflect.DeepEqual(nrtObj, expectedNodeTopology) {
446451
t.Fatalf("unexpected object from cache\ngot: %s\nexpected: %s\n", dumpNRT(nrtObj), dumpNRT(nodeTopologies[0]))
447452
}
453+
454+
expectedNrtInfo := CachedNRTInfo{
455+
Fresh: true,
456+
Generation: expectedGen,
457+
}
458+
if !reflect.DeepEqual(nrtInfo, expectedNrtInfo) {
459+
t.Fatalf("unexpected NRT info from cache\ngot: %+v\nexpected: %+v\n", nrtInfo, expectedNrtInfo)
460+
}
461+
462+
// flush again without dirty nodes
463+
gen2 := nrtCache.FlushNodes(lh)
464+
if gen2 != expectedGen {
465+
t.Fatalf("generation shouldn't change with no dirty nodes\ngot %d expected %d", gen2, expectedGen)
466+
}
448467
}
449468

450469
func TestResyncNoPodFingerprint(t *testing.T) {
@@ -971,3 +990,49 @@ func TestMakeNodeToPodDataMap(t *testing.T) {
971990
})
972991
}
973992
}
993+
994+
func TestOverresevedGetCachedNRTCopyWithForeignPods(t *testing.T) {
995+
testNodeName := "worker-node-G"
996+
nrt := makeTestNRT(testNodeName)
997+
pod := &corev1.Pod{} // API placeholder
998+
fakePodLister := &fakePodLister{}
999+
1000+
objs := []runtime.Object{nrt}
1001+
fakeClient, err := tu.NewFakeClient(objs...)
1002+
if err != nil {
1003+
t.Fatal(err)
1004+
}
1005+
1006+
ctx := context.Background()
1007+
lh := klog.Background()
1008+
nrtCache, err := NewOverReserve(ctx, lh, nil, fakeClient, fakePodLister, podprovider.IsPodRelevantAlways)
1009+
if err != nil {
1010+
t.Fatalf("unexpected error creating cache: %v", err)
1011+
}
1012+
1013+
expectedNrtInfo := CachedNRTInfo{
1014+
Generation: 0,
1015+
Fresh: true,
1016+
}
1017+
_, gotInfo := nrtCache.GetCachedNRTCopy(ctx, testNodeName, pod)
1018+
if !reflect.DeepEqual(gotInfo, expectedNrtInfo) {
1019+
t.Errorf("mismatched nrt info from cache, got %+v expected %+v", gotInfo, expectedNrtInfo)
1020+
}
1021+
1022+
// pointless, but will force a generation increase
1023+
gen := nrtCache.FlushNodes(lh, nrt)
1024+
if gen == 0 {
1025+
t.Fatalf("FlushNodes didn't increase the generation")
1026+
}
1027+
1028+
// expected to mark cached data as stale (!fresh)
1029+
nrtCache.NodeHasForeignPods(testNodeName, pod)
1030+
1031+
_, gotInfo = nrtCache.GetCachedNRTCopy(ctx, testNodeName, pod)
1032+
if gotInfo.Generation != gen {
1033+
t.Errorf("mismatched generation, got %v expected %v", gotInfo.Generation, gen)
1034+
}
1035+
if gotInfo.Fresh {
1036+
t.Errorf("cached data reported fresh when node has foreign pods")
1037+
}
1038+
}

0 commit comments

Comments
 (0)