diff --git a/pkg/controller/certificates/clustertrustbundlepublisher/publisher_test.go b/pkg/controller/certificates/clustertrustbundlepublisher/publisher_test.go index b68557e6272..b9bac8efaad 100644 --- a/pkg/controller/certificates/clustertrustbundlepublisher/publisher_test.go +++ b/pkg/controller/certificates/clustertrustbundlepublisher/publisher_test.go @@ -313,6 +313,10 @@ func fakeKubeClientSetWithCTBList(t *testing.T, signerName string, ctbs ...runti } } + // Ensure that Watch doesn't return any objects either by bumping up the resource version + // beyond what the fake client uses. + retList.ResourceVersion = "1000" + return true, retList, nil }) diff --git a/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go b/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go index 50c6bd87e3b..63ace4298af 100644 --- a/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go +++ b/staging/src/k8s.io/client-go/dynamic/fake/simple_test.go @@ -186,7 +186,7 @@ func Test_ListKind(t *testing.T) { "kind": "TheKindList", "metadata": map[string]interface{}{ "continue": "", - "resourceVersion": "3", // Three objects created so far. + "resourceVersion": "4", // Three objects created so far, starting value is 1. }, }, Items: []unstructured.Unstructured{ @@ -340,7 +340,7 @@ func TestListWithUnstructuredObjectsAndTypedScheme(t *testing.T) { expectedList := &unstructured.UnstructuredList{} expectedList.SetGroupVersionKind(listGVK) - expectedList.SetResourceVersion("1") // One object created so far. + expectedList.SetResourceVersion("2") // One object created so far, initial value is 1. expectedList.SetContinue("") expectedList.Items = append(expectedList.Items, u) @@ -369,7 +369,7 @@ func TestListWithNoFixturesAndTypedScheme(t *testing.T) { expectedList := &unstructured.UnstructuredList{} expectedList.SetGroupVersionKind(listGVK) - expectedList.SetResourceVersion("") // by product of the fake setting resource version + expectedList.SetResourceVersion("1") // No objects created so far. expectedList.SetContinue("") if diff := cmp.Diff(expectedList, list); diff != "" { @@ -402,7 +402,7 @@ func TestListWithNoScheme(t *testing.T) { expectedList := &unstructured.UnstructuredList{} expectedList.SetGroupVersionKind(listGVK) - expectedList.SetResourceVersion("1") // One object created so far. + expectedList.SetResourceVersion("2") // One object created so far, initial value is 1. expectedList.SetContinue("") expectedList.Items = append(expectedList.Items, u) @@ -443,7 +443,7 @@ func TestListWithTypedFixtures(t *testing.T) { expectedList := &unstructured.UnstructuredList{} expectedList.SetGroupVersionKind(listGVK) - expectedList.SetResourceVersion("1") // One object created so far. + expectedList.SetResourceVersion("2") // One object created so far, initial value is 1. expectedList.SetContinue("") expectedList.Items = []unstructured.Unstructured{u} diff --git a/staging/src/k8s.io/client-go/testing/fixture.go b/staging/src/k8s.io/client-go/testing/fixture.go index 7b8b614d3b2..4a91a99c70a 100644 --- a/staging/src/k8s.io/client-go/testing/fixture.go +++ b/staging/src/k8s.io/client-go/testing/fixture.go @@ -297,8 +297,11 @@ type tracker struct { // see apimachinery/pkg/watch.DefaultChanSize) will cause a panic. watchers map[schema.GroupVersionResource]map[string][]*watch.RaceFreeFakeWatcher // resourceVersions is the highest resource version of any tracked object with - // a certain gvr. The resource version for that set of objects gets bumped before - // storing a new or modified object, so all entries are larger than 0. + // a certain gvr. Conceptually it starts at 1 when no objects are stored (0 is + // special in queries) but the map contains no entries in that case. + // The resource version for that set of objects gets bumped before + // storing a new or modified object. + // // Object content does not get changed to preserve the traditional behavior // (hence also the versionedObject type instead of storing a runtime.Object // with modified ResourceVersion). @@ -322,6 +325,8 @@ type tracker struct { // but this is not how fake client-go has traditionally worked and starting to do // that now might break tests. type versionedObject struct { + // resourceVersion is always > 1 for a stored object because 1 + // is the initial value for an empty set of objects. resourceVersion int64 runtime.Object } @@ -368,6 +373,14 @@ func (t *tracker) List(gvr schema.GroupVersionResource, gvk schema.GroupVersionK t.lock.RLock() defer t.lock.RUnlock() + if listMeta, err := meta.ListAccessor(list); err == nil { + resourceVersion, ok := t.resourceVersions[gvr] + if !ok { + resourceVersion = 1 + } + listMeta.SetResourceVersion(fmt.Sprintf("%d", resourceVersion)) + } + objs, ok := t.objects[gvr] if !ok { return list, nil @@ -384,9 +397,6 @@ func (t *tracker) List(gvr schema.GroupVersionResource, gvk schema.GroupVersionK if err := meta.SetList(list, matchingObjs); err != nil { return nil, err } - if listMeta, err := meta.ListAccessor(list); err == nil { - listMeta.SetResourceVersion(fmt.Sprintf("%d", t.resourceVersions[gvr])) - } return list.DeepCopyObject(), nil } @@ -395,16 +405,26 @@ func (t *tracker) Watch(gvr schema.GroupVersionResource, ns string, opts ...meta if err != nil { return nil, err } + // By default, emulate the traditional behavior of the tracker and don't deliver - // *any* existing objects. This ensures compatibility which users - // which don't pass options and then also don't expect the additional objects. - resourceVersion := int64(-1) - if len(opts) > 0 && opts[0].ResourceVersion != "" { - rv, err := strconv.ParseInt(opts[0].ResourceVersion, 10, 64) - if err != nil { - return nil, fmt.Errorf("invalid ResourceVersion %q in ListOptions, must be int64: %w", opts[0].ResourceVersion, err) + // *any* existing objects unless list options are provided. + addExisting := false + addFromRV := int64(0) + if len(opts) > 0 { + // Providing options, as the generated client-go fake does, enables support + // for existing objects depending on the resource version. + // + // The default if ResourceVersion is empty is "start at most recent", + // which includes delivering all existing objects. addFromRV == 0 + // matches all objects below because all stored objects have addFromRV > 0. + addExisting = true + if opts[0].ResourceVersion != "" { + rv, err := strconv.ParseInt(opts[0].ResourceVersion, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid ResourceVersion %q in ListOptions, must be int64: %w", opts[0].ResourceVersion, err) + } + addFromRV = rv } - resourceVersion = rv } t.lock.Lock() @@ -419,14 +439,14 @@ func (t *tracker) Watch(gvr schema.GroupVersionResource, ns string, opts ...meta // Deliver all objects that match the list options, for example // between the initial List and the following Watch. - if resourceVersion != -1 { + if addExisting { objs := t.objects[gvr] matchingObjs, err := filterByNamespace(objs, ns) if err != nil { return nil, err } for _, obj := range matchingObjs { - if resourceVersion == 0 || resourceVersion < obj.resourceVersion { + if addFromRV < obj.resourceVersion { fakewatcher.Add(obj.Object) } } @@ -632,16 +652,23 @@ func (t *tracker) add(gvr schema.GroupVersionResource, obj runtime.Object, ns st t.objects[gvr] = make(map[types.NamespacedName]versionedObject) } + // Determine resource version for the new or updated object. + resourceVersion, ok := t.resourceVersions[gvr] + if !ok { + resourceVersion = 1 + } + resourceVersion++ + namespacedName := types.NamespacedName{Namespace: newMeta.GetNamespace(), Name: newMeta.GetName()} if _, ok = t.objects[gvr][namespacedName]; ok { if replaceExisting { - resourceVersion := t.resourceVersions[gvr] + 1 t.resourceVersions[gvr] = resourceVersion + t.objects[gvr][namespacedName] = versionedObject{resourceVersion, obj} + for _, w := range t.getWatches(gvr, ns) { // To avoid the object from being accidentally modified by watcher w.Modify(obj.DeepCopyObject()) } - t.objects[gvr][namespacedName] = versionedObject{resourceVersion, obj} return nil } return apierrors.NewAlreadyExists(gr, newMeta.GetName()) @@ -652,7 +679,6 @@ func (t *tracker) add(gvr schema.GroupVersionResource, obj runtime.Object, ns st return apierrors.NewNotFound(gr, newMeta.GetName()) } - resourceVersion := t.resourceVersions[gvr] + 1 t.resourceVersions[gvr] = resourceVersion t.objects[gvr][namespacedName] = versionedObject{resourceVersion, obj} diff --git a/staging/src/k8s.io/client-go/testing/internal/listandwatch_test.go b/staging/src/k8s.io/client-go/testing/internal/listandwatch_test.go index e1996490dfe..732eaabee10 100644 --- a/staging/src/k8s.io/client-go/testing/internal/listandwatch_test.go +++ b/staging/src/k8s.io/client-go/testing/internal/listandwatch_test.go @@ -67,14 +67,14 @@ func testListAndWatch(t *testing.T) { logger.Info("Listed", "configMaps", objs, "err", err) if err != nil { t.Errorf("Unexpected List error: %v", err) - } else if objs.ResourceVersion != "1" { - t.Errorf("Expected ListMeta ResourceVersion 1, got %q", objs.ResourceVersion) + } else if objs.ResourceVersion != "2" { + t.Errorf("Expected ListMeta ResourceVersion 2, got %q", objs.ResourceVersion) } return objs, err }, WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - if options.ResourceVersion != "1" { - t.Errorf("Expected ListOptions ResourceVersion 1, got %q", options.ResourceVersion) + if options.ResourceVersion != "2" { + t.Errorf("Expected ListOptions ResourceVersion 2, got %q", options.ResourceVersion) } logger.Info("Delaying Watch...") <-createDone @@ -84,6 +84,17 @@ func testListAndWatch(t *testing.T) { }, client), &v1.ConfigMap{}, defaultEventHandlerResyncPeriod, nil) }) + var adds, updates, deletes int + handle, err := configMapInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(_ any) { adds++ }, + UpdateFunc: func(_, _ any) { updates++ }, + DeleteFunc: func(_ any) { deletes++ }, + }) + if err != nil { + t.Fatalf("Unexpected error adding event handler: %v", err) + } + defer configMapInformer.RemoveEventHandler(handle) + configMapStore := configMapInformer.GetStore() f.Start(stopCh) f.WaitForCacheSync(stopCh) @@ -100,7 +111,7 @@ func testListAndWatch(t *testing.T) { Namespace: "default", }, } - _, err := client.CoreV1().ConfigMaps(cm.Namespace).Create(ctx, cm, metav1.CreateOptions{}) + _, err = client.CoreV1().ConfigMaps(cm.Namespace).Create(ctx, cm, metav1.CreateOptions{}) if err != nil { t.Fatalf("Unexpected error creating ConfigMap: %v", err) } @@ -112,6 +123,13 @@ func testListAndWatch(t *testing.T) { objs = configMapStore.List() if len(objs) != 2 { - t.Fatalf("Unexpected item(s) in informer cache, want 2, got %d = %v", len(objs), objs) + t.Errorf("Unexpected item(s) in informer cache, want 2, got %d = %v", len(objs), objs) + } + + if !handle.HasSynced() { + t.Error("Expected event handler to have synced, it didn't") + } + if adds != 2 || updates != 0 || deletes != 0 { + t.Errorf("Expected two new objects, got adds/updates/deletes %d/%d/%d", adds, updates, deletes) } } diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/helpers_test.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/helpers_test.go index 1ed27015e7b..e9b5cadd3c2 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/helpers_test.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/helpers_test.go @@ -51,7 +51,7 @@ func TestGetPodList(t *testing.T) { sortBy: func(pods []*corev1.Pod) sort.Interface { return podutils.ByLogging(pods) }, expected: &corev1.PodList{ ListMeta: metav1.ListMeta{ - ResourceVersion: "2", // Two objects created. + ResourceVersion: "3", // Two objects created, initial value is 1. }, Items: []corev1.Pod{ {