From 3783a720e7278466859fe140d2bfbbfb054f5313 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 14 Jan 2026 12:19:32 +0100 Subject: [PATCH] client-go testing: start ResourceVersion at 1 for empty set List should never return "0", that has a special meaning in queries. --- .../client-go/dynamic/fake/simple_test.go | 10 +++---- .../src/k8s.io/client-go/testing/fixture.go | 28 ++++++++++++++----- .../testing/internal/listandwatch_test.go | 8 +++--- .../pkg/polymorphichelpers/helpers_test.go | 2 +- 4 files changed, 31 insertions(+), 17 deletions(-) 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 825c01da8d7..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("0") // No objects created so far. + 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 b031ce9c80d..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,7 +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 > 0 for a stored object. + // resourceVersion is always > 1 for a stored object because 1 + // is the initial value for an empty set of objects. resourceVersion int64 runtime.Object } @@ -370,7 +374,11 @@ func (t *tracker) List(gvr schema.GroupVersionResource, gvk schema.GroupVersionK defer t.lock.RUnlock() if listMeta, err := meta.ListAccessor(list); err == nil { - listMeta.SetResourceVersion(fmt.Sprintf("%d", t.resourceVersions[gvr])) + resourceVersion, ok := t.resourceVersions[gvr] + if !ok { + resourceVersion = 1 + } + listMeta.SetResourceVersion(fmt.Sprintf("%d", resourceVersion)) } objs, ok := t.objects[gvr] @@ -644,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()) @@ -664,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 e7bb134d078..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 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{ {