From 801ef4c2445eb3b9fb103d21544da0beb0abbbb2 Mon Sep 17 00:00:00 2001 From: Valerian Roche Date: Mon, 28 Jul 2025 13:53:34 -0400 Subject: [PATCH 1/3] [client-go #1415] Embed proper interface in TransformingStore to ensure DeltaFIFO and RealFIFO are implementing it Signed-off-by: Valerian Roche --- .../client-go/tools/cache/delta_fifo.go | 3 +- .../client-go/tools/cache/delta_fifo_test.go | 22 ++++---- .../k8s.io/client-go/tools/cache/reflector.go | 2 +- .../client-go/tools/cache/reflector_test.go | 56 +++++++++---------- .../client-go/tools/cache/the_real_fifo.go | 3 +- 5 files changed, 41 insertions(+), 45 deletions(-) diff --git a/staging/src/k8s.io/client-go/tools/cache/delta_fifo.go b/staging/src/k8s.io/client-go/tools/cache/delta_fifo.go index 9d9e238cccedd..a0d7a834aa0da 100644 --- a/staging/src/k8s.io/client-go/tools/cache/delta_fifo.go +++ b/staging/src/k8s.io/client-go/tools/cache/delta_fifo.go @@ -270,7 +270,8 @@ func NewDeltaFIFOWithOptions(opts DeltaFIFOOptions) *DeltaFIFO { } var ( - _ = Queue(&DeltaFIFO{}) // DeltaFIFO is a Queue + _ = Queue(&DeltaFIFO{}) // DeltaFIFO is a Queue + _ = TransformingStore(&DeltaFIFO{}) // DeltaFIFO implements TransformingStore to allow memory optimizations ) var ( diff --git a/staging/src/k8s.io/client-go/tools/cache/delta_fifo_test.go b/staging/src/k8s.io/client-go/tools/cache/delta_fifo_test.go index 9c67012cc1c57..24f0ec87d04bb 100644 --- a/staging/src/k8s.io/client-go/tools/cache/delta_fifo_test.go +++ b/staging/src/k8s.io/client-go/tools/cache/delta_fifo_test.go @@ -28,7 +28,7 @@ import ( // from the most recent Delta. // You should treat the items returned inside the deltas as immutable. // This function was moved here because it is not consistent with normal list semantics, but is used in unit testing. -func (f *DeltaFIFO) List() []interface{} { +func (f *DeltaFIFO) list() []interface{} { f.lock.RLock() defer f.lock.RUnlock() return f.listLocked() @@ -46,7 +46,7 @@ func (f *DeltaFIFO) listLocked() []interface{} { // ListKeys returns a list of all the keys of the objects currently // in the FIFO. // This function was moved here because it is not consistent with normal list semantics, but is used in unit testing. -func (f *DeltaFIFO) ListKeys() []string { +func (f *DeltaFIFO) listKeys() []string { f.lock.RLock() defer f.lock.RUnlock() list := make([]string, 0, len(f.queue)) @@ -60,19 +60,19 @@ func (f *DeltaFIFO) ListKeys() []string { // or sets exists=false. // You should treat the items returned inside the deltas as immutable. // This function was moved here because it is not consistent with normal list semantics, but is used in unit testing. -func (f *DeltaFIFO) Get(obj interface{}) (item interface{}, exists bool, err error) { +func (f *DeltaFIFO) get(obj interface{}) (item interface{}, exists bool, err error) { key, err := f.KeyOf(obj) if err != nil { return nil, false, KeyError{obj, err} } - return f.GetByKey(key) + return f.getByKey(key) } // GetByKey returns the complete list of deltas for the requested item, // setting exists=false if that list is empty. // You should treat the items returned inside the deltas as immutable. // This function was moved here because it is not consistent with normal list semantics, but is used in unit testing. -func (f *DeltaFIFO) GetByKey(key string) (item interface{}, exists bool, err error) { +func (f *DeltaFIFO) getByKey(key string) (item interface{}, exists bool, err error) { f.lock.RLock() defer f.lock.RUnlock() d, exists := f.items[key] @@ -320,10 +320,10 @@ func TestDeltaFIFO_addUpdate(t *testing.T) { f.Update(mkFifoObj("foo", 12)) f.Delete(mkFifoObj("foo", 15)) - if e, a := []interface{}{mkFifoObj("foo", 15)}, f.List(); !reflect.DeepEqual(e, a) { + if e, a := []interface{}{mkFifoObj("foo", 15)}, f.list(); !reflect.DeepEqual(e, a) { t.Errorf("Expected %+v, got %+v", e, a) } - if e, a := []string{"foo"}, f.ListKeys(); !reflect.DeepEqual(e, a) { + if e, a := []string{"foo"}, f.listKeys(); !reflect.DeepEqual(e, a) { t.Errorf("Expected %+v, got %+v", e, a) } @@ -349,7 +349,7 @@ func TestDeltaFIFO_addUpdate(t *testing.T) { t.Errorf("Got second value %v", unexpected.val) case <-time.After(50 * time.Millisecond): } - _, exists, _ := f.Get(mkFifoObj("foo", "")) + _, exists, _ := f.get(mkFifoObj("foo", "")) if exists { t.Errorf("item did not get removed") } @@ -397,7 +397,7 @@ func TestDeltaFIFO_transformer(t *testing.T) { must(f.Replace([]interface{}{}, "")) // Should be empty - if e, a := []string{"foo", "bar"}, f.ListKeys(); !reflect.DeepEqual(e, a) { + if e, a := []string{"foo", "bar"}, f.listKeys(); !reflect.DeepEqual(e, a) { t.Errorf("Expected %+v, got %+v", e, a) } @@ -507,7 +507,7 @@ func TestDeltaFIFO_addReplace(t *testing.T) { t.Errorf("Got second value %v", unexpected.val) case <-time.After(50 * time.Millisecond): } - _, exists, _ := f.Get(mkFifoObj("foo", "")) + _, exists, _ := f.get(mkFifoObj("foo", "")) if exists { t.Errorf("item did not get removed") } @@ -991,7 +991,7 @@ func BenchmarkDeltaFIFOListKeys(b *testing.B) { b.ResetTimer() b.RunParallel(func(pb *testing.PB) { for pb.Next() { - _ = f.ListKeys() + _ = f.listKeys() } }) b.StopTimer() diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector.go b/staging/src/k8s.io/client-go/tools/cache/reflector.go index ee9be77278ab4..c68f0d87d249f 100644 --- a/staging/src/k8s.io/client-go/tools/cache/reflector.go +++ b/staging/src/k8s.io/client-go/tools/cache/reflector.go @@ -80,7 +80,7 @@ type ReflectorStore interface { // TransformingStore is an optional interface that can be implemented by the provided store. // If implemented on the provided store reflector will use the same transformer in its internal stores. type TransformingStore interface { - Store + ReflectorStore Transformer() TransformFunc } diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_test.go b/staging/src/k8s.io/client-go/tools/cache/reflector_test.go index 8c20ae0b02a4e..1dfafb465ff0e 100644 --- a/staging/src/k8s.io/client-go/tools/cache/reflector_test.go +++ b/staging/src/k8s.io/client-go/tools/cache/reflector_test.go @@ -1962,7 +1962,7 @@ func TestReflectorReplacesStoreOnUnsafeDelete(t *testing.T) { s := NewFIFO(MetaNamespaceKeyFunc) var replaceInvoked atomic.Int32 store := &fakeStore{ - Store: s, + ReflectorStore: s, beforeReplace: func(list []interface{}, rv string) { // interested in the Replace call that happens after the Error event if rv == lastExpectedRV { @@ -2085,30 +2085,16 @@ func TestReflectorRespectStoreTransformer(t *testing.T) { {Type: watch.Added, Object: pod3}, } - s := NewFIFO(MetaNamespaceKeyFunc) - var replaceInvoked atomic.Int32 - store := &fakeStore{ - Store: s, - beforeReplace: func(list []interface{}, rv string) { - replaceInvoked.Add(1) - // Only two pods are present at the point when Replace is called. - if len(list) != 2 { - t.Errorf("unexpected nb of objects: expected 2 received %d", len(list)) - } - for _, obj := range list { - cast := obj.(*v1.Pod) - if cast.Spec.Hostname != "transformed" { - t.Error("Object was not transformed prior to replacement") - } - } - }, - afterReplace: func(rv string, err error) {}, - transformer: func(i interface{}) (interface{}, error) { + var transformerInvoked atomic.Int32 + s := NewDeltaFIFOWithOptions(DeltaFIFOOptions{ + KeyFunction: MetaNamespaceKeyFunc, + Transformer: func(i interface{}) (interface{}, error) { + transformerInvoked.Add(1) cast := i.(*v1.Pod) cast.Spec.Hostname = "transformed" return cast, nil }, - } + }) var once sync.Once lw := &ListWatch{ @@ -2130,7 +2116,7 @@ func TestReflectorRespectStoreTransformer(t *testing.T) { } clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.WatchListClient, true) - r := NewReflector(lw, &v1.Pod{}, store, 0) + r := NewReflector(lw, &v1.Pod{}, s, 0) ctx, cancel := context.WithCancel(context.Background()) doneCh := make(chan struct{}) go func() { @@ -2152,8 +2138,21 @@ func TestReflectorRespectStoreTransformer(t *testing.T) { if want, got := lastExpectedRV, r.LastSyncResourceVersion(); want != got { t.Errorf("expected LastSyncResourceVersion to be %q, but got: %q", want, got) } - if want, got := 1, int(replaceInvoked.Load()); want != got { - t.Errorf("expected replace to be invoked %d times, but got: %d", want, got) + + if want, got := 3, len(s.list()); want != got { + t.Errorf("expected informer to contain %d objects, but got: %d", want, got) + } + for _, item := range s.list() { + cast := item.(*v1.Pod) + if cast.Spec.Hostname != "transformed" { + t.Error("Object was not transformed prior to replacement") + } + } + + // Transformer should have been invoked twice for the initial sync in the informer on the temporary store, + // then twice on replace, then once on the following update. + if want, got := 5, int(transformerInvoked.Load()); want != got { + t.Errorf("expected transformer to be invoked %d times, but got: %d", want, got) } cancel() @@ -2165,23 +2164,18 @@ func TestReflectorRespectStoreTransformer(t *testing.T) { } type fakeStore struct { - Store + ReflectorStore beforeReplace func(list []interface{}, s string) afterReplace func(rv string, err error) - transformer TransformFunc } func (f *fakeStore) Replace(list []interface{}, rv string) error { f.beforeReplace(list, rv) - err := f.Store.Replace(list, rv) + err := f.ReflectorStore.Replace(list, rv) f.afterReplace(rv, err) return err } -func (f *fakeStore) Transformer() TransformFunc { - return f.transformer -} - func BenchmarkExtractList(b *testing.B) { _, _, podList := getPodListItems(0, fakeItemsNum) _, _, configMapList := getConfigmapListItems(0, fakeItemsNum) diff --git a/staging/src/k8s.io/client-go/tools/cache/the_real_fifo.go b/staging/src/k8s.io/client-go/tools/cache/the_real_fifo.go index ef322bea85078..b907410dc05e0 100644 --- a/staging/src/k8s.io/client-go/tools/cache/the_real_fifo.go +++ b/staging/src/k8s.io/client-go/tools/cache/the_real_fifo.go @@ -61,7 +61,8 @@ type RealFIFO struct { } var ( - _ = Queue(&RealFIFO{}) // RealFIFO is a Queue + _ = Queue(&RealFIFO{}) // RealFIFO is a Queue + _ = TransformingStore(&RealFIFO{}) // RealFIFO implements TransformingStore to allow memory optimizations ) // Close the queue. From 43c4010d6e948f9c9b0cd1c0bf993ce6d45673d6 Mon Sep 17 00:00:00 2001 From: Valerian Roche Date: Thu, 31 Jul 2025 17:41:40 -0400 Subject: [PATCH 2/3] Update transformer test to run on both real-fifo and delta-fifo Signed-off-by: Valerian Roche --- .../client-go/tools/cache/reflector_test.go | 205 ++++++++++-------- .../tools/cache/the_real_fifo_test.go | 17 ++ 2 files changed, 130 insertions(+), 92 deletions(-) diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_test.go b/staging/src/k8s.io/client-go/tools/cache/reflector_test.go index 1dfafb465ff0e..3f22dabc69db6 100644 --- a/staging/src/k8s.io/client-go/tools/cache/reflector_test.go +++ b/staging/src/k8s.io/client-go/tools/cache/reflector_test.go @@ -2057,109 +2057,130 @@ func TestReflectorReplacesStoreOnUnsafeDelete(t *testing.T) { } func TestReflectorRespectStoreTransformer(t *testing.T) { - mkPod := func(id string, rv string) *v1.Pod { - return &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: id, ResourceVersion: rv}, - Spec: v1.PodSpec{ - Hostname: "test", - }, - } - } + type testReflectorStore interface { + ReflectorStore + list() []interface{} + } + + for name, storeBuilder := range map[string]func(counter *atomic.Int32) testReflectorStore{ + "real-fifo": func(counter *atomic.Int32) testReflectorStore { + return NewRealFIFO(MetaNamespaceKeyFunc, NewStore(MetaNamespaceKeyFunc), func(i interface{}) (interface{}, error) { + counter.Add(1) + cast := i.(*v1.Pod) + cast.Spec.Hostname = "transformed" + return cast, nil + }) + }, + "delta-fifo": func(counter *atomic.Int32) testReflectorStore { + return NewDeltaFIFOWithOptions(DeltaFIFOOptions{ + KeyFunction: MetaNamespaceKeyFunc, + Transformer: func(i interface{}) (interface{}, error) { + counter.Add(1) + cast := i.(*v1.Pod) + cast.Spec.Hostname = "transformed" + return cast, nil + }, + }) + }, + } { + t.Run(name, func(t *testing.T) { + mkPod := func(id string, rv string) *v1.Pod { + return &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: id, ResourceVersion: rv}, + Spec: v1.PodSpec{ + Hostname: "test", + }, + } + } - preExisting1 := mkPod("foo-1", "1") - preExisting2 := mkPod("foo-2", "2") - pod3 := mkPod("foo-3", "3") + preExisting1 := mkPod("foo-1", "1") + preExisting2 := mkPod("foo-2", "2") + pod3 := mkPod("foo-3", "3") + + lastExpectedRV := "3" + events := []watch.Event{ + {Type: watch.Added, Object: preExisting1}, + {Type: watch.Added, Object: preExisting2}, + {Type: watch.Bookmark, Object: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + ResourceVersion: lastExpectedRV, + Annotations: map[string]string{ + metav1.InitialEventsAnnotationKey: "true", + }, + }, + }}, + {Type: watch.Added, Object: pod3}, + } - lastExpectedRV := "3" - events := []watch.Event{ - {Type: watch.Added, Object: preExisting1}, - {Type: watch.Added, Object: preExisting2}, - {Type: watch.Bookmark, Object: &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - ResourceVersion: lastExpectedRV, - Annotations: map[string]string{ - metav1.InitialEventsAnnotationKey: "true", - }, - }, - }}, - {Type: watch.Added, Object: pod3}, - } + var transformerInvoked atomic.Int32 + s := storeBuilder(&transformerInvoked) - var transformerInvoked atomic.Int32 - s := NewDeltaFIFOWithOptions(DeltaFIFOOptions{ - KeyFunction: MetaNamespaceKeyFunc, - Transformer: func(i interface{}) (interface{}, error) { - transformerInvoked.Add(1) - cast := i.(*v1.Pod) - cast.Spec.Hostname = "transformed" - return cast, nil - }, - }) + var once sync.Once + lw := &ListWatch{ + WatchFunc: func(metav1.ListOptions) (watch.Interface, error) { + fw := watch.NewFake() + go func() { + once.Do(func() { + for _, e := range events { + fw.Action(e.Type, e.Object) + } + }) + }() + return fw, nil + }, + // ListFunc should never be used in WatchList mode + ListFunc: func(metav1.ListOptions) (runtime.Object, error) { + return nil, errors.New("list call not expected in WatchList mode") + }, + } - var once sync.Once - lw := &ListWatch{ - WatchFunc: func(metav1.ListOptions) (watch.Interface, error) { - fw := watch.NewFake() + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.WatchListClient, true) + r := NewReflector(lw, &v1.Pod{}, s, 0) + ctx, cancel := context.WithCancel(context.Background()) + doneCh := make(chan struct{}) go func() { - once.Do(func() { - for _, e := range events { - fw.Action(e.Type, e.Object) - } - }) + defer close(doneCh) + r.RunWithContext(ctx) }() - return fw, nil - }, - // ListFunc should never be used in WatchList mode - ListFunc: func(metav1.ListOptions) (runtime.Object, error) { - return nil, errors.New("list call not expected in WatchList mode") - }, - } - - clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.WatchListClient, true) - r := NewReflector(lw, &v1.Pod{}, s, 0) - ctx, cancel := context.WithCancel(context.Background()) - doneCh := make(chan struct{}) - go func() { - defer close(doneCh) - r.RunWithContext(ctx) - }() - // wait for the RV to sync to the version returned by the final list - err := wait.PollUntilContextTimeout(context.Background(), 100*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (done bool, err error) { - if rv := r.LastSyncResourceVersion(); rv == lastExpectedRV { - return true, nil - } - return false, nil - }) - if err != nil { - t.Fatalf("reflector never caught up with expected revision: %q, err: %v", lastExpectedRV, err) - } + // wait for the RV to sync to the version returned by the final list + err := wait.PollUntilContextTimeout(context.Background(), 100*time.Millisecond, 5*time.Second, true, func(ctx context.Context) (done bool, err error) { + if rv := r.LastSyncResourceVersion(); rv == lastExpectedRV { + return true, nil + } + return false, nil + }) + if err != nil { + t.Fatalf("reflector never caught up with expected revision: %q, err: %v", lastExpectedRV, err) + } - if want, got := lastExpectedRV, r.LastSyncResourceVersion(); want != got { - t.Errorf("expected LastSyncResourceVersion to be %q, but got: %q", want, got) - } + if want, got := lastExpectedRV, r.LastSyncResourceVersion(); want != got { + t.Errorf("expected LastSyncResourceVersion to be %q, but got: %q", want, got) + } - if want, got := 3, len(s.list()); want != got { - t.Errorf("expected informer to contain %d objects, but got: %d", want, got) - } - for _, item := range s.list() { - cast := item.(*v1.Pod) - if cast.Spec.Hostname != "transformed" { - t.Error("Object was not transformed prior to replacement") - } - } + if want, got := 3, len(s.list()); want != got { + t.Errorf("expected informer to contain %d objects, but got: %d", want, got) + } + for _, item := range s.list() { + cast := item.(*v1.Pod) + if cast.Spec.Hostname != "transformed" { + t.Error("Object was not transformed prior to replacement") + } + } - // Transformer should have been invoked twice for the initial sync in the informer on the temporary store, - // then twice on replace, then once on the following update. - if want, got := 5, int(transformerInvoked.Load()); want != got { - t.Errorf("expected transformer to be invoked %d times, but got: %d", want, got) - } + // Transformer should have been invoked twice for the initial sync in the informer on the temporary store, + // then twice on replace, then once on the following update. + if want, got := 5, int(transformerInvoked.Load()); want != got { + t.Errorf("expected transformer to be invoked %d times, but got: %d", want, got) + } - cancel() - select { - case <-doneCh: - case <-time.After(wait.ForeverTestTimeout): - t.Errorf("timed out waiting for Run to return") + cancel() + select { + case <-doneCh: + case <-time.After(wait.ForeverTestTimeout): + t.Errorf("timed out waiting for Run to return") + } + }) } } diff --git a/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go b/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go index 649ea36872315..b124a059f7968 100644 --- a/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go +++ b/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go @@ -18,8 +18,10 @@ package cache import ( "fmt" + "maps" "reflect" "runtime" + "slices" "testing" "time" ) @@ -33,6 +35,21 @@ func (f *RealFIFO) getItems() []Delta { return ret } +func (f *RealFIFO) list() []interface{} { + f.lock.Lock() + defer f.lock.Unlock() + + objects := make(map[string]interface{}) + for _, item := range f.items { + if item.Type == Deleted { + continue + } + key, _ := f.keyFunc(item.Object) + objects[key] = item.Object + } + return slices.Collect(maps.Values(objects)) +} + const closedFIFOName = "FIFO WAS CLOSED" func popN(queue Queue, count int) []interface{} { From 288d49fbe6e139660bb8f4a9c6500c7905a97b08 Mon Sep 17 00:00:00 2001 From: Valerian Roche Date: Tue, 5 Aug 2025 10:04:30 -0400 Subject: [PATCH 3/3] Rework the test to remove the need for new test accessor on RealFIFO --- .../client-go/tools/cache/reflector_test.go | 67 ++++++++++++------- .../tools/cache/the_real_fifo_test.go | 17 ----- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/staging/src/k8s.io/client-go/tools/cache/reflector_test.go b/staging/src/k8s.io/client-go/tools/cache/reflector_test.go index 3f22dabc69db6..2c1965dd1a62a 100644 --- a/staging/src/k8s.io/client-go/tools/cache/reflector_test.go +++ b/staging/src/k8s.io/client-go/tools/cache/reflector_test.go @@ -20,10 +20,12 @@ import ( "context" "errors" "fmt" + "maps" "math/rand" "net/http" "reflect" goruntime "runtime" + "slices" "strconv" "sync" "sync/atomic" @@ -2057,30 +2059,48 @@ func TestReflectorReplacesStoreOnUnsafeDelete(t *testing.T) { } func TestReflectorRespectStoreTransformer(t *testing.T) { - type testReflectorStore interface { - ReflectorStore - list() []interface{} - } - - for name, storeBuilder := range map[string]func(counter *atomic.Int32) testReflectorStore{ - "real-fifo": func(counter *atomic.Int32) testReflectorStore { - return NewRealFIFO(MetaNamespaceKeyFunc, NewStore(MetaNamespaceKeyFunc), func(i interface{}) (interface{}, error) { - counter.Add(1) - cast := i.(*v1.Pod) - cast.Spec.Hostname = "transformed" - return cast, nil - }) - }, - "delta-fifo": func(counter *atomic.Int32) testReflectorStore { - return NewDeltaFIFOWithOptions(DeltaFIFOOptions{ - KeyFunction: MetaNamespaceKeyFunc, - Transformer: func(i interface{}) (interface{}, error) { + for name, test := range map[string]struct { + storeBuilder func(counter *atomic.Int32) ReflectorStore + items func(rs ReflectorStore) []interface{} + }{ + "real-fifo": { + storeBuilder: func(counter *atomic.Int32) ReflectorStore { + return NewRealFIFO(MetaNamespaceKeyFunc, NewStore(MetaNamespaceKeyFunc), func(i interface{}) (interface{}, error) { counter.Add(1) cast := i.(*v1.Pod) cast.Spec.Hostname = "transformed" return cast, nil - }, - }) + }) + }, + items: func(rs ReflectorStore) []interface{} { + store := rs.(*RealFIFO) + objects := make(map[string]interface{}) + for _, item := range store.getItems() { + key, _ := store.keyFunc(item.Object) + if item.Type == Deleted { + delete(objects, key) + } else { + objects[key] = item.Object + } + } + return slices.Collect(maps.Values(objects)) + }, + }, + "delta-fifo": { + storeBuilder: func(counter *atomic.Int32) ReflectorStore { + return NewDeltaFIFOWithOptions(DeltaFIFOOptions{ + KeyFunction: MetaNamespaceKeyFunc, + Transformer: func(i interface{}) (interface{}, error) { + counter.Add(1) + cast := i.(*v1.Pod) + cast.Spec.Hostname = "transformed" + return cast, nil + }, + }) + }, + items: func(rs ReflectorStore) []interface{} { + return rs.(*DeltaFIFO).list() + }, }, } { t.Run(name, func(t *testing.T) { @@ -2113,7 +2133,7 @@ func TestReflectorRespectStoreTransformer(t *testing.T) { } var transformerInvoked atomic.Int32 - s := storeBuilder(&transformerInvoked) + s := test.storeBuilder(&transformerInvoked) var once sync.Once lw := &ListWatch{ @@ -2158,10 +2178,11 @@ func TestReflectorRespectStoreTransformer(t *testing.T) { t.Errorf("expected LastSyncResourceVersion to be %q, but got: %q", want, got) } - if want, got := 3, len(s.list()); want != got { + informerItems := test.items(s) + if want, got := 3, len(informerItems); want != got { t.Errorf("expected informer to contain %d objects, but got: %d", want, got) } - for _, item := range s.list() { + for _, item := range informerItems { cast := item.(*v1.Pod) if cast.Spec.Hostname != "transformed" { t.Error("Object was not transformed prior to replacement") diff --git a/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go b/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go index b124a059f7968..649ea36872315 100644 --- a/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go +++ b/staging/src/k8s.io/client-go/tools/cache/the_real_fifo_test.go @@ -18,10 +18,8 @@ package cache import ( "fmt" - "maps" "reflect" "runtime" - "slices" "testing" "time" ) @@ -35,21 +33,6 @@ func (f *RealFIFO) getItems() []Delta { return ret } -func (f *RealFIFO) list() []interface{} { - f.lock.Lock() - defer f.lock.Unlock() - - objects := make(map[string]interface{}) - for _, item := range f.items { - if item.Type == Deleted { - continue - } - key, _ := f.keyFunc(item.Object) - objects[key] = item.Object - } - return slices.Collect(maps.Values(objects)) -} - const closedFIFOName = "FIFO WAS CLOSED" func popN(queue Queue, count int) []interface{} {