From 37db332298fc6d14a798f610ce6049792299f0b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 3 Nov 2022 08:47:38 +0100 Subject: [PATCH 1/3] Stop exporting storage testing utility functions --- .../pkg/storage/testing/store_tests.go | 56 ++++++++++--------- .../apiserver/pkg/storage/testing/utils.go | 33 ++++------- .../pkg/storage/testing/watcher_tests.go | 30 +++++----- 3 files changed, 57 insertions(+), 62 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go index d6ef8ccc380..ef70b36b3af 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go @@ -85,12 +85,12 @@ func RunTestCreateWithTTL(ctx context.Context, t *testing.T, store storage.Inter if err != nil { t.Fatalf("Watch failed: %v", err) } - TestCheckEventType(t, watch.Deleted, w) + testCheckEventType(t, watch.Deleted, w) } func RunTestCreateWithKeyExist(ctx context.Context, t *testing.T, store storage.Interface) { obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} - key, _ := TestPropagateStore(ctx, t, store, obj) + key, _ := testPropagateStore(ctx, t, store, obj) out := &example.Pod{} err := store.Create(ctx, key, obj, out, 0) if err == nil || !storage.IsExist(err) { @@ -100,7 +100,7 @@ func RunTestCreateWithKeyExist(ctx context.Context, t *testing.T, store storage. func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { // create an object to test - key, createdObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, createdObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) // update the object once to allow get by exact resource version to be tested updateObj := createdObj.DeepCopy() updateObj.Annotations = map[string]string{"test-annotation": "1"} @@ -219,7 +219,7 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { } func RunTestUnconditionalDelete(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) tests := []struct { name string @@ -263,7 +263,7 @@ func RunTestUnconditionalDelete(ctx context.Context, t *testing.T, store storage } func RunTestConditionalDelete(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) tests := []struct { name string @@ -299,7 +299,7 @@ func RunTestConditionalDelete(ctx context.Context, t *testing.T, store storage.I } out.ResourceVersion = storedObj.ResourceVersion ExpectNoDiff(t, "incorrect pod:", storedObj, out) - key, storedObj = TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) + key, storedObj = testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) }) } } @@ -330,7 +330,7 @@ func RunTestConditionalDelete(ctx context.Context, t *testing.T, store storage.I // [DONE] Added TestPreconditionalDeleteWithSuggestion func RunTestDeleteWithSuggestion(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + key, originalPod := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) out := &example.Pod{} if err := store.Delete(ctx, key, out, nil, storage.ValidateAllObjectFunc, originalPod); err != nil { @@ -343,7 +343,7 @@ func RunTestDeleteWithSuggestion(ctx context.Context, t *testing.T, store storag } func RunTestDeleteWithSuggestionAndConflict(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + key, originalPod := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) // First update, so originalPod is outdated. updatedPod := &example.Pod{} @@ -367,7 +367,7 @@ func RunTestDeleteWithSuggestionAndConflict(ctx context.Context, t *testing.T, s } func RunTestDeleteWithSuggestionOfDeletedObject(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + key, originalPod := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) // First delete, so originalPod is outdated. deletedPod := &example.Pod{} @@ -383,7 +383,7 @@ func RunTestDeleteWithSuggestionOfDeletedObject(ctx context.Context, t *testing. } func RunTestValidateDeletionWithSuggestion(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + key, originalPod := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) // Check that validaing fresh object fails is called once and fails. validationCalls := 0 @@ -435,7 +435,7 @@ func RunTestValidateDeletionWithSuggestion(ctx context.Context, t *testing.T, st } func RunTestPreconditionalDeleteWithSuggestion(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) + key, originalPod := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "name"}}) // First update, so originalPod is outdated. updatedPod := &example.Pod{} @@ -673,7 +673,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { expectContinue: true, expectedRemainingItemCount: utilpointer.Int64Ptr(1), rv: "0", - expectRVFunc: ResourceVersionNotOlderThan(list.ResourceVersion), + expectRVFunc: resourceVersionNotOlderThan(list.ResourceVersion), }, { name: "test List with limit at resource version 0 match=NotOlderThan", @@ -688,7 +688,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { expectedRemainingItemCount: utilpointer.Int64Ptr(1), rv: "0", rvMatch: metav1.ResourceVersionMatchNotOlderThan, - expectRVFunc: ResourceVersionNotOlderThan(list.ResourceVersion), + expectRVFunc: resourceVersionNotOlderThan(list.ResourceVersion), }, { name: "test List with limit at resource version before first write and match=Exact", @@ -795,7 +795,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.OneTermEqualSelector("metadata.name", "bar"), Label: labels.Everything(), Limit: 2, - Continue: EncodeContinueOrDie("z-level/3", int64(continueRV)), + Continue: encodeContinueOrDie("z-level/3", int64(continueRV)), }, expectedOut: []*example.Pod{preset[4]}, }, @@ -806,7 +806,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.OneTermEqualSelector("metadata.name", "bar"), Label: labels.Everything(), Limit: 1, - Continue: EncodeContinueOrDie("z-level/3/test-2", int64(continueRV)), + Continue: encodeContinueOrDie("z-level/3/test-2", int64(continueRV)), }, expectedOut: []*example.Pod{preset[4]}, }, @@ -817,7 +817,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.OneTermEqualSelector("metadata.name", "bar"), Label: labels.Everything(), Limit: 2, - Continue: EncodeContinueOrDie("z-level/3/test-2", int64(continueRV)), + Continue: encodeContinueOrDie("z-level/3/test-2", int64(continueRV)), }, expectedOut: []*example.Pod{preset[4]}, }, @@ -1048,10 +1048,10 @@ func seedMultiLevelData(ctx context.Context, store storage.Interface) (string, [ } func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage.Interface) { - prevKey, prevStoredObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "prev"}}) + prevKey, prevStoredObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "prev"}}) prevRV, _ := strconv.Atoi(prevStoredObj.ResourceVersion) - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) currentRV, _ := strconv.Atoi(storedObj.ResourceVersion) tests := []struct { @@ -1572,7 +1572,7 @@ func RunTestListInconsistentContinuation(ctx context.Context, t *testing.T, stor if len(out.Continue) == 0 { t.Fatalf("No continuation token set") } - validateResourceVersion := ResourceVersionNotOlderThan(lastRVString) + validateResourceVersion := resourceVersionNotOlderThan(lastRVString) ExpectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj}, out.Items) if err := validateResourceVersion(out.ResourceVersion); err != nil { t.Fatal(err) @@ -1776,7 +1776,7 @@ func RunTestGuaranteedUpdate(ctx context.Context, t *testing.T, store InterfaceW for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { - key, storeObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) + key, storeObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) out := &example.Pod{} name := fmt.Sprintf("foo-%d", i) @@ -1866,7 +1866,7 @@ func RunTestGuaranteedUpdateWithTTL(ctx context.Context, t *testing.T, store sto if err != nil { t.Fatalf("Watch failed: %v", err) } - TestCheckEventType(t, watch.Deleted, w) + testCheckEventType(t, watch.Deleted, w) } func RunTestGuaranteedUpdateChecksStoredData(ctx context.Context, t *testing.T, store InterfaceWithPrefixTransformer) { @@ -1880,7 +1880,7 @@ func RunTestGuaranteedUpdateChecksStoredData(ctx context.Context, t *testing.T, transformer.prefix = []byte(string(transformer.prefix) + " ") return transformer }) - _, initial := TestPropagateStore(ctx, t, store, input) + _, initial := testPropagateStore(ctx, t, store, input) revertTransformer() // this update should write the canonical value to etcd because the new serialization differs @@ -1935,7 +1935,7 @@ func RunTestGuaranteedUpdateChecksStoredData(ctx context.Context, t *testing.T, } func RunTestGuaranteedUpdateWithConflict(ctx context.Context, t *testing.T, store storage.Interface) { - key, _ := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, _ := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) errChan := make(chan error, 1) var firstToFinish sync.WaitGroup @@ -1980,7 +1980,7 @@ func RunTestGuaranteedUpdateWithConflict(ctx context.Context, t *testing.T, stor } func RunTestGuaranteedUpdateWithSuggestionAndConflict(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, originalPod := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) // First, update without a suggestion so originalPod is outdated updatedPod := &example.Pod{} @@ -2142,7 +2142,9 @@ func RunTestCount(ctx context.Context, t *testing.T, store storage.Interface) { obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("foo-%d", i)}} key := fmt.Sprintf("%s/%d", resourceA, i) - TestPropagateStoreWithKey(ctx, t, store, key, obj) + if err := store.Create(ctx, key, obj, nil, 0); err != nil { + t.Fatalf("Create failed: %v", err) + } } resourceBCount := 4 @@ -2150,7 +2152,9 @@ func RunTestCount(ctx context.Context, t *testing.T, store storage.Interface) { obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("foo-%d", i)}} key := fmt.Sprintf("%s/%d", resourceB, i) - TestPropagateStoreWithKey(ctx, t, store, key, obj) + if err := store.Create(ctx, key, obj, nil, 0); err != nil { + t.Fatalf("Create failed: %v", err) + } } resourceACountGot, err := store.Count(resourceA) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go index d66ba8c3092..79f2654d43a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go @@ -37,11 +37,6 @@ import ( "k8s.io/apiserver/pkg/storage/value" ) -// CreateObj will create a single object using the storage interface. -func CreateObj(helper storage.Interface, name string, obj, out runtime.Object, ttl uint64) error { - return helper.Create(context.TODO(), name, obj, out, ttl) -} - // CreateObjList will create a list from the array of objects. func CreateObjList(prefix string, helper storage.Interface, items []runtime.Object) error { for i := range items { @@ -50,7 +45,7 @@ func CreateObjList(prefix string, helper storage.Interface, items []runtime.Obje if err != nil { return err } - err = CreateObj(helper, path.Join(prefix, meta.GetName()), obj, obj, 0) + err = helper.Create(context.Background(), path.Join(prefix, meta.GetName()), obj, obj, 0) if err != nil { return err } @@ -82,16 +77,12 @@ func DeepEqualSafePodSpec() example.PodSpec { } } -// TestPropagateStore helps propagates store with objects, automates key generation, and returns +// testPropagateStore helps propagates store with objects, automates key generation, and returns // keys and stored objects. -func TestPropagateStore(ctx context.Context, t *testing.T, store storage.Interface, obj *example.Pod) (string, *example.Pod) { +func testPropagateStore(ctx context.Context, t *testing.T, store storage.Interface, obj *example.Pod) (string, *example.Pod) { // Setup store with a key and grab the output for returning. key := fmt.Sprintf("/%s/%s", obj.Namespace, obj.Name) - return key, TestPropagateStoreWithKey(ctx, t, store, key, obj) -} -// TestPropagateStoreWithKey helps propagate store with objects, the given object will be stored at the specified key. -func TestPropagateStoreWithKey(ctx context.Context, t *testing.T, store storage.Interface, key string, obj *example.Pod) *example.Pod { // Setup store with the specified key and grab the output for returning. err := store.Delete(ctx, key, &example.Pod{}, nil, storage.ValidateAllObjectFunc, nil) if err != nil && !storage.IsNotFound(err) { @@ -101,7 +92,7 @@ func TestPropagateStoreWithKey(ctx context.Context, t *testing.T, store storage. if err := store.Create(ctx, key, obj, setOutput, 0); err != nil { t.Fatalf("Set failed: %v", err) } - return setOutput + return key, setOutput } func ExpectNoDiff(t *testing.T, msg string, expected, got interface{}) { @@ -135,7 +126,7 @@ func ExpectContains(t *testing.T, msg string, expectedList []interface{}, got in const dummyPrefix = "adapter" -func EncodeContinueOrDie(key string, resourceVersion int64) string { +func encodeContinueOrDie(key string, resourceVersion int64) string { token, err := storage.EncodeContinue(dummyPrefix+key, dummyPrefix, resourceVersion) if err != nil { panic(err) @@ -143,7 +134,7 @@ func EncodeContinueOrDie(key string, resourceVersion int64) string { return token } -func TestCheckEventType(t *testing.T, expectEventType watch.EventType, w watch.Interface) { +func testCheckEventType(t *testing.T, expectEventType watch.EventType, w watch.Interface) { select { case res := <-w.ResultChan(): if res.Type != expectEventType { @@ -154,14 +145,14 @@ func TestCheckEventType(t *testing.T, expectEventType watch.EventType, w watch.I } } -func TestCheckResult(t *testing.T, expectEventType watch.EventType, w watch.Interface, expectObj *example.Pod) { - TestCheckResultFunc(t, expectEventType, w, func(object runtime.Object) error { +func testCheckResult(t *testing.T, expectEventType watch.EventType, w watch.Interface, expectObj *example.Pod) { + testCheckResultFunc(t, expectEventType, w, func(object runtime.Object) error { ExpectNoDiff(t, "incorrect object", expectObj, object) return nil }) } -func TestCheckResultFunc(t *testing.T, expectEventType watch.EventType, w watch.Interface, check func(object runtime.Object) error) { +func testCheckResultFunc(t *testing.T, expectEventType watch.EventType, w watch.Interface, check func(object runtime.Object) error) { select { case res := <-w.ResultChan(): if res.Type != expectEventType { @@ -176,7 +167,7 @@ func TestCheckResultFunc(t *testing.T, expectEventType watch.EventType, w watch. } } -func TestCheckStop(t *testing.T, w watch.Interface) { +func testCheckStop(t *testing.T, w watch.Interface) { select { case e, ok := <-w.ResultChan(): if ok { @@ -194,10 +185,10 @@ func TestCheckStop(t *testing.T, w watch.Interface) { } } -// ResourceVersionNotOlderThan returns a function to validate resource versions. Resource versions +// resourceVersionNotOlderThan returns a function to validate resource versions. Resource versions // referring to points in logical time before the sentinel generate an error. All logical times as // new as the sentinel or newer generate no error. -func ResourceVersionNotOlderThan(sentinel string) func(string) error { +func resourceVersionNotOlderThan(sentinel string) func(string) error { return func(resourceVersion string) error { objectVersioner := storage.APIObjectVersioner{} actualRV, err := objectVersioner.ParseResourceVersion(resourceVersion) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go index 4876cf36ba9..4c846cd8eda 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go @@ -113,12 +113,12 @@ func testWatch(ctx context.Context, t *testing.T, store storage.Interface, recur expectObj = prevObj expectObj.ResourceVersion = out.ResourceVersion } - TestCheckResult(t, watchTest.watchType, w, expectObj) + testCheckResult(t, watchTest.watchType, w, expectObj) } prevObj = out } w.Stop() - TestCheckStop(t, w) + testCheckStop(t, w) }) } } @@ -127,13 +127,13 @@ func testWatch(ctx context.Context, t *testing.T, store storage.Interface, recur // - watch from 0 should sync up and grab the object added before // - watch from 0 is able to return events for objects whose previous version has been compacted func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Interface, compaction Compaction) { - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "ns"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "ns"}}) w, err := store.Watch(ctx, key, storage.ListOptions{ResourceVersion: "0", Predicate: storage.Everything}) if err != nil { t.Fatalf("Watch failed: %v", err) } - TestCheckResult(t, watch.Added, w, storedObj) + testCheckResult(t, watch.Added, w, storedObj) w.Stop() // Update @@ -151,7 +151,7 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter if err != nil { t.Fatalf("Watch failed: %v", err) } - TestCheckResult(t, watch.Added, w, out) + testCheckResult(t, watch.Added, w, out) w.Stop() if compaction == nil { @@ -176,11 +176,11 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter if err != nil { t.Fatalf("Watch failed: %v", err) } - TestCheckResult(t, watch.Added, w, out) + testCheckResult(t, watch.Added, w, out) } func RunTestDeleteTriggerWatch(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) w, err := store.Watch(ctx, key, storage.ListOptions{ResourceVersion: storedObj.ResourceVersion, Predicate: storage.Everything}) if err != nil { t.Fatalf("Watch failed: %v", err) @@ -188,11 +188,11 @@ func RunTestDeleteTriggerWatch(ctx context.Context, t *testing.T, store storage. if err := store.Delete(ctx, key, &example.Pod{}, nil, storage.ValidateAllObjectFunc, nil); err != nil { t.Fatalf("Delete failed: %v", err) } - TestCheckEventType(t, watch.Deleted, w) + testCheckEventType(t, watch.Deleted, w) } func RunTestWatchFromNoneZero(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) w, err := store.Watch(ctx, key, storage.ListOptions{ResourceVersion: storedObj.ResourceVersion, Predicate: storage.Everything}) if err != nil { @@ -203,7 +203,7 @@ func RunTestWatchFromNoneZero(ctx context.Context, t *testing.T, store storage.I func(runtime.Object) (runtime.Object, error) { return &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "bar"}}, err }), nil) - TestCheckResult(t, watch.Modified, w, out) + testCheckResult(t, watch.Modified, w, out) } func RunTestWatchError(ctx context.Context, t *testing.T, store InterfaceWithPrefixTransformer) { @@ -236,7 +236,7 @@ func RunTestWatchError(ctx context.Context, t *testing.T, store InterfaceWithPre if err != nil { t.Fatalf("Watch failed: %v", err) } - TestCheckEventType(t, watch.Error, w) + testCheckEventType(t, watch.Error, w) } func RunTestWatchContextCancel(ctx context.Context, t *testing.T, store storage.Interface) { @@ -263,7 +263,7 @@ func RunTestWatchContextCancel(ctx context.Context, t *testing.T, store storage. } func RunTestWatchDeleteEventObjectHaveLatestRV(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) watchCtx, _ := context.WithTimeout(ctx, wait.ForeverTestTimeout) w, err := store.Watch(watchCtx, key, storage.ListOptions{ResourceVersion: storedObj.ResourceVersion, Predicate: storage.Everything}) @@ -295,7 +295,7 @@ func RunTestWatchInitializationSignal(ctx context.Context, t *testing.T, store s initSignal := utilflowcontrol.NewInitializationSignal() ctx = utilflowcontrol.WithInitializationSignal(ctx, initSignal) - key, storedObj := TestPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) _, err := store.Watch(ctx, key, storage.ListOptions{ResourceVersion: storedObj.ResourceVersion, Predicate: storage.Everything}) if err != nil { t.Fatalf("Watch failed: %v", err) @@ -315,7 +315,7 @@ func RunOptionalTestProgressNotify(ctx context.Context, t *testing.T, store stor if err := store.Create(ctx, key, input, out, 0); err != nil { t.Fatalf("Create failed: %v", err) } - validateResourceVersion := ResourceVersionNotOlderThan(out.ResourceVersion) + validateResourceVersion := resourceVersionNotOlderThan(out.ResourceVersion) opts := storage.ListOptions{ ResourceVersion: out.ResourceVersion, @@ -329,7 +329,7 @@ func RunOptionalTestProgressNotify(ctx context.Context, t *testing.T, store stor // when we send a bookmark event, the client expects the event to contain an // object of the correct type, but with no fields set other than the resourceVersion - TestCheckResultFunc(t, watch.Bookmark, w, func(object runtime.Object) error { + testCheckResultFunc(t, watch.Bookmark, w, func(object runtime.Object) error { // first, check that we have the correct resource version obj, ok := object.(metav1.Object) if !ok { From e301306d89a69ff1a328a4ae3fa39a9e3deb022e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 3 Nov 2022 14:58:34 +0100 Subject: [PATCH 2/3] Reuse generic ListNonRecurisve test for cacher --- .../pkg/storage/testing/store_tests.go | 155 ++++++++++-------- .../pkg/storage/tests/cacher_test.go | 59 +------ 2 files changed, 88 insertions(+), 126 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go index ef70b36b3af..9b892f02f37 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go @@ -124,45 +124,46 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { // TODO(jpbetz): Add exact test cases tests := []struct { - name string - key string - ignoreNotFound bool - expectNotFoundErr bool - expectRVTooLarge bool - expectedOut []*example.Pod - rv string + name string + key string + ignoreNotFound bool + expectNotFoundErr bool + expectRVTooLarge bool + expectedOut *example.Pod + expectedAlternatives []*example.Pod + rv string }{{ name: "get existing", key: key, ignoreNotFound: false, expectNotFoundErr: false, - expectedOut: []*example.Pod{storedObj}, + expectedOut: storedObj, }, { // For RV=0 arbitrarily old version is allowed, including from the moment // when the object didn't yet exist. // As a result, we allow it by setting ignoreNotFound and allowing an empty // object in expectedOut. - name: "resource version 0", - key: key, - ignoreNotFound: true, - expectedOut: []*example.Pod{{}, createdObj, storedObj}, - rv: "0", + name: "resource version 0", + key: key, + ignoreNotFound: true, + expectedAlternatives: []*example.Pod{{}, createdObj, storedObj}, + rv: "0", }, { // Given that Get with set ResourceVersion is effectively always // NotOlderThan semantic, both versions of object are allowed. - name: "object created resource version", - key: key, - expectedOut: []*example.Pod{createdObj, storedObj}, - rv: createdObj.ResourceVersion, + name: "object created resource version", + key: key, + expectedAlternatives: []*example.Pod{createdObj, storedObj}, + rv: createdObj.ResourceVersion, }, { name: "current object resource version, match=NotOlderThan", key: key, - expectedOut: []*example.Pod{storedObj}, + expectedOut: storedObj, rv: fmt.Sprintf("%d", currentRV), }, { name: "latest resource version", key: key, - expectedOut: []*example.Pod{storedObj}, + expectedOut: storedObj, rv: fmt.Sprintf("%d", lastUpdatedCurrentRV), }, { name: "too high resource version", @@ -179,7 +180,7 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { key: "/non-existing", ignoreNotFound: true, expectNotFoundErr: false, - expectedOut: []*example.Pod{{}}, + expectedOut: &example.Pod{}, }} for _, tt := range tests { @@ -202,8 +203,8 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { t.Fatalf("Get failed: %v", err) } - if len(tt.expectedOut) == 1 { - ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), tt.expectedOut[0], out) + if tt.expectedAlternatives == nil { + ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), tt.expectedOut, out) } else { toInterfaceSlice := func(pods []*example.Pod) []interface{} { result := make([]interface{}, 0, len(pods)) @@ -212,7 +213,7 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { } return result } - ExpectContains(t, fmt.Sprintf("%s: incorrect pod", tt.name), toInterfaceSlice(tt.expectedOut), out) + ExpectContains(t, fmt.Sprintf("%s: incorrect pod", tt.name), toInterfaceSlice(tt.expectedAlternatives), out) } }) } @@ -1048,77 +1049,86 @@ func seedMultiLevelData(ctx context.Context, store storage.Interface) (string, [ } func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage.Interface) { - prevKey, prevStoredObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "prev"}}) - + key, prevStoredObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) prevRV, _ := strconv.Atoi(prevStoredObj.ResourceVersion) - key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + + storedObj := &example.Pod{} + if err := store.GuaranteedUpdate(ctx, key, storedObj, false, nil, + func(_ runtime.Object, _ storage.ResponseMeta) (runtime.Object, *uint64, error) { + newPod := prevStoredObj.DeepCopy() + newPod.Annotations = map[string]string{"version": "second"} + return newPod, nil, nil + }, nil); err != nil { + t.Fatalf("update failed: %v", err) + } currentRV, _ := strconv.Atoi(storedObj.ResourceVersion) tests := []struct { - name string - key string - pred storage.SelectionPredicate - expectedOut []*example.Pod - rv string - rvMatch metav1.ResourceVersionMatch - expectRVTooLarge bool + name string + key string + pred storage.SelectionPredicate + expectedOut []example.Pod + expectedAlternatives [][]example.Pod + rv string + rvMatch metav1.ResourceVersionMatch + expectRVTooLarge bool }{{ name: "existing key", key: key, pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, + expectedOut: []example.Pod{*storedObj}, }, { - name: "existing key, resourceVersion=0", + name: "existing key, resourceVersion=0", + key: key, + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{nil, {*storedObj}}, + rv: "0", + }, { + name: "existing key, resourceVersion=0, resourceVersionMatch=notOlderThan", + key: key, + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{nil, {*storedObj}}, + rv: "0", + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + }, { + name: "existing key, resourceVersion=previous, resourceVersionMatch=notOlderThan", + key: key, + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{{*prevStoredObj}, {*storedObj}}, + rv: fmt.Sprintf("%d", prevRV), + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + }, { + name: "existing key, resourceVersion=current, resourceVersionMatch=notOlderThan", key: key, pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, - rv: "0", - }, { - name: "existing key, resourceVersion=0, resourceVersionMatch=notOlderThan", - key: key, - pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, - rv: "0", + expectedOut: []example.Pod{*storedObj}, + rv: fmt.Sprintf("%d", currentRV), rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { name: "existing key, resourceVersion=current", key: key, pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, + expectedOut: []example.Pod{*storedObj}, rv: fmt.Sprintf("%d", currentRV), - }, { - name: "existing key, resourceVersion=current, resourceVersionMatch=notOlderThan", - key: key, - pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, - rv: fmt.Sprintf("%d", currentRV), - rvMatch: metav1.ResourceVersionMatchNotOlderThan, - }, { - name: "existing key, resourceVersion=previous, resourceVersionMatch=notOlderThan", - key: key, - pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, - rv: fmt.Sprintf("%d", prevRV), - rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { name: "existing key, resourceVersion=current, resourceVersionMatch=exact", key: key, pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, + expectedOut: []example.Pod{*storedObj}, rv: fmt.Sprintf("%d", currentRV), rvMatch: metav1.ResourceVersionMatchExact, }, { - name: "existing key, resourceVersion=current, resourceVersionMatch=exact", - key: prevKey, + name: "existing key, resourceVersion=previous, resourceVersionMatch=exact", + key: key, pred: storage.Everything, - expectedOut: []*example.Pod{prevStoredObj}, + expectedOut: []example.Pod{*prevStoredObj}, rv: fmt.Sprintf("%d", prevRV), rvMatch: metav1.ResourceVersionMatchExact, }, { name: "existing key, resourceVersion=too high", key: key, pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, + expectedOut: []example.Pod{*storedObj}, rv: strconv.FormatInt(math.MaxInt64, 10), expectRVTooLarge: true, }, { @@ -1164,13 +1174,18 @@ func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage if len(out.ResourceVersion) == 0 { t.Errorf("%s: unset resourceVersion", tt.name) } - if len(out.Items) != len(tt.expectedOut) { - t.Errorf("%s: length of list want=%d, get=%d", tt.name, len(tt.expectedOut), len(out.Items)) - return - } - for j, wantPod := range tt.expectedOut { - getPod := &out.Items[j] - ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), wantPod, getPod) + + if tt.expectedAlternatives == nil { + ExpectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items) + } else { + toInterfaceSlice := func(podLists [][]example.Pod) []interface{} { + result := make([]interface{}, 0, len(podLists)) + for i := range podLists { + result = append(result, podLists[i]) + } + return result + } + ExpectContains(t, "incorrect list pods", toInterfaceSlice(tt.expectedAlternatives), out.Items) } }) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go index 5860b2a854b..7eeb1f80f70 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go @@ -170,62 +170,9 @@ func TestGet(t *testing.T) { } func TestGetListNonRecursive(t *testing.T) { - server, etcdStorage := newEtcdTestStorage(t, etcd3testing.PathPrefix()) - defer server.Terminate(t) - cacher, _, err := newTestCacher(etcdStorage) - if err != nil { - t.Fatalf("Couldn't create cacher: %v", err) - } - defer cacher.Stop() - - storedObj := updatePod(t, etcdStorage, makeTestPod("foo"), nil) - key := "pods/" + storedObj.Namespace + "/" + storedObj.Name - - tests := []struct { - key string - pred storage.SelectionPredicate - expectedOut []*example.Pod - }{{ // test non-recursive GetList on existing key - key: key, - pred: storage.Everything, - expectedOut: []*example.Pod{storedObj}, - }, { // test non-recursive GetList on non-existing key - key: "/non-existing", - pred: storage.Everything, - expectedOut: nil, - }, { // test non-recursive GetList with matching pod name - key: "/non-existing", - pred: storage.SelectionPredicate{ - Label: labels.Everything(), - Field: fields.ParseSelectorOrDie("metadata.name!=" + storedObj.Name), - GetAttrs: func(obj runtime.Object) (labels.Set, fields.Set, error) { - pod := obj.(*example.Pod) - return nil, fields.Set{"metadata.name": pod.Name}, nil - }, - }, - expectedOut: nil, - }} - - for i, tt := range tests { - out := &example.PodList{} - err := cacher.GetList(context.TODO(), tt.key, storage.ListOptions{Predicate: tt.pred, Recursive: false}, out) - if err != nil { - t.Fatalf("GetList failed: %v", err) - } - if len(out.ResourceVersion) == 0 { - t.Errorf("#%d: unset resourceVersion", i) - } - if len(out.Items) != len(tt.expectedOut) { - t.Errorf("#%d: length of list want=%d, get=%d", i, len(tt.expectedOut), len(out.Items)) - continue - } - for j, wantPod := range tt.expectedOut { - getPod := &out.Items[j] - if !reflect.DeepEqual(wantPod, getPod) { - t.Errorf("#%d: pod want=%#v, get=%#v", i, wantPod, getPod) - } - } - } + ctx, cacher, terminate := testSetup(t) + defer terminate() + storagetesting.RunTestGetListNonRecursive(ctx, t, cacher) } func TestList(t *testing.T) { From 6d85f947bf86d12e4459d31029c41d6049d40c0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 4 Nov 2022 10:23:40 +0100 Subject: [PATCH 3/3] Parallelize tests cases in some storage tests --- .../pkg/storage/testing/store_tests.go | 28 +++++++++++-------- .../pkg/storage/tests/cacher_test.go | 4 +-- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go index 9b892f02f37..8594b2031b6 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go @@ -184,7 +184,10 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { }} for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() + out := &example.Pod{} err := store.Get(ctx, tt.key, storage.GetOptions{IgnoreNotFound: tt.ignoreNotFound, ResourceVersion: tt.rv}, out) if tt.expectNotFoundErr { @@ -1091,12 +1094,11 @@ func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage rv: "0", rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { - name: "existing key, resourceVersion=previous, resourceVersionMatch=notOlderThan", - key: key, - pred: storage.Everything, - expectedAlternatives: [][]example.Pod{{*prevStoredObj}, {*storedObj}}, - rv: fmt.Sprintf("%d", prevRV), - rvMatch: metav1.ResourceVersionMatchNotOlderThan, + name: "existing key, resourceVersion=current", + key: key, + pred: storage.Everything, + expectedOut: []example.Pod{*storedObj}, + rv: fmt.Sprintf("%d", currentRV), }, { name: "existing key, resourceVersion=current, resourceVersionMatch=notOlderThan", key: key, @@ -1105,11 +1107,12 @@ func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage rv: fmt.Sprintf("%d", currentRV), rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { - name: "existing key, resourceVersion=current", - key: key, - pred: storage.Everything, - expectedOut: []example.Pod{*storedObj}, - rv: fmt.Sprintf("%d", currentRV), + name: "existing key, resourceVersion=previous, resourceVersionMatch=notOlderThan", + key: key, + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{{*prevStoredObj}, {*storedObj}}, + rv: fmt.Sprintf("%d", prevRV), + rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { name: "existing key, resourceVersion=current, resourceVersionMatch=exact", key: key, @@ -1151,7 +1154,10 @@ func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage }} for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() + out := &example.PodList{} storageOpts := storage.ListOptions{ ResourceVersion: tt.rv, diff --git a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go index 7eeb1f80f70..9223132a9fa 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go @@ -165,13 +165,13 @@ func updatePod(t *testing.T, s storage.Interface, obj, old *example.Pod) *exampl func TestGet(t *testing.T) { ctx, cacher, terminate := testSetup(t) - defer terminate() + t.Cleanup(terminate) storagetesting.RunTestGet(ctx, t, cacher) } func TestGetListNonRecursive(t *testing.T) { ctx, cacher, terminate := testSetup(t) - defer terminate() + t.Cleanup(terminate) storagetesting.RunTestGetListNonRecursive(ctx, t, cacher) }