From d9a3685d6fc87458a49ec98475e467f75389ae24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Tue, 25 Oct 2022 14:09:36 +0200 Subject: [PATCH] Minor cleanup of storage tests Kubernetes-commit: 5344bc5e1b7e50d5ff359c714d522b421b28bd12 --- pkg/storage/etcd3/store_test.go | 35 +++-------------- pkg/storage/etcd3/watcher_test.go | 4 +- pkg/storage/testing/store_tests.go | 59 ++++++++++++---------------- pkg/storage/testing/utils.go | 10 ++--- pkg/storage/testing/watcher_tests.go | 6 +-- 5 files changed, 40 insertions(+), 74 deletions(-) diff --git a/pkg/storage/etcd3/store_test.go b/pkg/storage/etcd3/store_test.go index 3661c5233..c888c73e4 100644 --- a/pkg/storage/etcd3/store_test.go +++ b/pkg/storage/etcd3/store_test.go @@ -66,11 +66,6 @@ func newPod() runtime.Object { return &example.Pod{} } -func TestCreate(t *testing.T) { - ctx, store, etcdClient := testSetup(t) - storagetesting.RunTestCreate(ctx, t, store, checkStorageInvariants(etcdClient, store.codec)) -} - func checkStorageInvariants(etcdClient *clientv3.Client, codec runtime.Codec) storagetesting.KeyValidation { return func(ctx context.Context, t *testing.T, key string) { getResp, err := etcdClient.KV.Get(ctx, key) @@ -94,6 +89,11 @@ func checkStorageInvariants(etcdClient *clientv3.Client, codec runtime.Codec) st } } +func TestCreate(t *testing.T) { + ctx, store, etcdClient := testSetup(t) + storagetesting.RunTestCreate(ctx, t, store, checkStorageInvariants(etcdClient, store.codec)) +} + func TestCreateWithTTL(t *testing.T) { ctx, store, _ := testSetup(t) storagetesting.RunTestCreateWithTTL(ctx, t, store) @@ -119,31 +119,6 @@ func TestConditionalDelete(t *testing.T) { storagetesting.RunTestConditionalDelete(ctx, t, store) } -// The following set of Delete tests are testing the logic of adding `suggestion` -// as a parameter with probably value of the current state. -// Introducing it for GuaranteedUpdate cause a number of issues, so we're addressing -// all of those upfront by adding appropriate tests: -// - https://github.com/kubernetes/kubernetes/pull/35415 -// [DONE] Lack of tests originally - added TestDeleteWithSuggestion. -// - https://github.com/kubernetes/kubernetes/pull/40664 -// [DONE] Irrelevant for delete, as Delete doesn't write data (nor compare it). -// - https://github.com/kubernetes/kubernetes/pull/47703 -// [DONE] Irrelevant for delete, because Delete doesn't persist data. -// - https://github.com/kubernetes/kubernetes/pull/48394/ -// [DONE] Irrelevant for delete, because Delete doesn't compare data. -// - https://github.com/kubernetes/kubernetes/pull/43152 -// [DONE] Added TestDeleteWithSuggestionAndConflict -// - https://github.com/kubernetes/kubernetes/pull/54780 -// [DONE] Irrelevant for delete, because Delete doesn't compare data. -// - https://github.com/kubernetes/kubernetes/pull/58375 -// [DONE] Irrelevant for delete, because Delete doesn't compare data. -// - https://github.com/kubernetes/kubernetes/pull/77619 -// [DONE] Added TestValidateDeletionWithSuggestion for corresponding delete checks. -// - https://github.com/kubernetes/kubernetes/pull/78713 -// [DONE] Bug was in getState function which is shared with the new code. -// - https://github.com/kubernetes/kubernetes/pull/78713 -// [DONE] Added TestPreconditionalDeleteWithSuggestion - func TestDeleteWithSuggestion(t *testing.T) { ctx, store, _ := testSetup(t) storagetesting.RunTestDeleteWithSuggestion(ctx, t, store) diff --git a/pkg/storage/etcd3/watcher_test.go b/pkg/storage/etcd3/watcher_test.go index 07cd86abb..cf76a1627 100644 --- a/pkg/storage/etcd3/watcher_test.go +++ b/pkg/storage/etcd3/watcher_test.go @@ -54,7 +54,7 @@ func TestDeleteTriggerWatch(t *testing.T) { // - watch from 0 is able to return events for objects whose previous version has been compacted func TestWatchFromZero(t *testing.T) { ctx, store, client := testSetup(t) - key, storedObj := storagetesting.TestPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "ns"}}) + key, storedObj := storagetesting.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 { @@ -164,7 +164,7 @@ func TestWatchErrResultNotBlockAfterCancel(t *testing.T) { func TestWatchDeleteEventObjectHaveLatestRV(t *testing.T) { ctx, store, client := testSetup(t) - key, storedObj := storagetesting.TestPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) + key, storedObj := storagetesting.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 { diff --git a/pkg/storage/testing/store_tests.go b/pkg/storage/testing/store_tests.go index 2fd609113..6e68036f6 100644 --- a/pkg/storage/testing/store_tests.go +++ b/pkg/storage/testing/store_tests.go @@ -88,7 +88,7 @@ func RunTestCreateWithTTL(ctx context.Context, t *testing.T, store storage.Inter func RunTestCreateWithKeyExist(ctx context.Context, t *testing.T, store storage.Interface) { obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} - key, _ := TestPropogateStore(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) { @@ -98,7 +98,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 := TestPropogateStore(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"} @@ -129,43 +129,43 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { expectRVTooLarge bool expectedOut *example.Pod rv string - }{{ // test get on existing item + }{{ name: "get existing", key: key, ignoreNotFound: false, expectNotFoundErr: false, expectedOut: storedObj, - }, { // test get on existing item with resource version set to 0 + }, { name: "resource version 0", key: key, expectedOut: storedObj, rv: "0", - }, { // test get on existing item with resource version set to the resource version is was created on + }, { name: "object created resource version", key: key, expectedOut: storedObj, rv: createdObj.ResourceVersion, - }, { // test get on existing item with resource version set to current resource version of the object + }, { name: "current object resource version, match=NotOlderThan", key: key, expectedOut: storedObj, rv: fmt.Sprintf("%d", currentRV), - }, { // test get on existing item with resource version set to latest pod resource version + }, { name: "latest resource version", key: key, expectedOut: storedObj, rv: fmt.Sprintf("%d", lastUpdatedCurrentRV), - }, { // test get on existing item with resource version set too high + }, { name: "too high resource version", key: key, expectRVTooLarge: true, rv: strconv.FormatInt(math.MaxInt64, 10), - }, { // test get on non-existing item with ignoreNotFound=false + }, { name: "get non-existing", key: "/non-existing", ignoreNotFound: false, expectNotFoundErr: true, - }, { // test get on non-existing item with ignoreNotFound=true + }, { name: "get non-existing, ignore not found", key: "/non-existing", ignoreNotFound: true, @@ -198,7 +198,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 := TestPropogateStore(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 @@ -236,7 +236,7 @@ func RunTestUnconditionalDelete(ctx context.Context, t *testing.T, store storage } func RunTestConditionalDelete(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropogateStore(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 @@ -266,7 +266,7 @@ func RunTestConditionalDelete(ctx context.Context, t *testing.T, store storage.I t.Fatalf("%s: Delete failed: %v", tt.name, err) } ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), storedObj, out) - key, storedObj = TestPropogateStore(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"}}) }) } } @@ -297,8 +297,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 := TestPropogateStore(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 { @@ -311,8 +310,7 @@ func RunTestDeleteWithSuggestion(ctx context.Context, t *testing.T, store storag } func RunTestDeleteWithSuggestionAndConflict(ctx context.Context, t *testing.T, store storage.Interface) { - - key, originalPod := TestPropogateStore(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{} @@ -336,8 +334,7 @@ func RunTestDeleteWithSuggestionAndConflict(ctx context.Context, t *testing.T, s } func RunTestDeleteWithSuggestionOfDeletedObject(ctx context.Context, t *testing.T, store storage.Interface) { - - key, originalPod := TestPropogateStore(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{} @@ -353,8 +350,7 @@ func RunTestDeleteWithSuggestionOfDeletedObject(ctx context.Context, t *testing. } func RunTestValidateDeletionWithSuggestion(ctx context.Context, t *testing.T, store storage.Interface) { - - key, originalPod := TestPropogateStore(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 @@ -406,8 +402,7 @@ func RunTestValidateDeletionWithSuggestion(ctx context.Context, t *testing.T, st } func RunTestPreconditionalDeleteWithSuggestion(ctx context.Context, t *testing.T, store storage.Interface) { - - key, originalPod := TestPropogateStore(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{} @@ -1020,12 +1015,10 @@ func seedMultiLevelData(ctx context.Context, store storage.Interface) (string, [ } func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage.Interface) { - prevKey, prevStoredObj := TestPropogateStore(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 := TestPropogateStore(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 { @@ -1241,7 +1234,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 := TestPropogateStore(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) @@ -1314,7 +1307,6 @@ func RunTestGuaranteedUpdate(ctx context.Context, t *testing.T, store InterfaceW } func RunTestGuaranteedUpdateWithTTL(ctx context.Context, t *testing.T, store storage.Interface) { - input := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} key := "/somekey" @@ -1336,7 +1328,7 @@ func RunTestGuaranteedUpdateWithTTL(ctx context.Context, t *testing.T, store sto } func RunTestGuaranteedUpdateWithConflict(ctx context.Context, t *testing.T, store storage.Interface) { - key, _ := TestPropogateStore(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 @@ -1381,7 +1373,7 @@ func RunTestGuaranteedUpdateWithConflict(ctx context.Context, t *testing.T, stor } func RunTestGuaranteedUpdateWithSuggestionAndConflict(ctx context.Context, t *testing.T, store storage.Interface) { - key, originalPod := TestPropogateStore(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{} @@ -1456,7 +1448,6 @@ func RunTestGuaranteedUpdateWithSuggestionAndConflict(ctx context.Context, t *te } func RunTestCount(ctx context.Context, t *testing.T, store storage.Interface) { - resourceA := "/foo.bar.io/abc" // resourceA is intentionally a prefix of resourceB to ensure that the count @@ -1468,7 +1459,7 @@ 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) - TestPropogateStoreWithKey(ctx, t, store, key, obj) + TestPropagateStoreWithKey(ctx, t, store, key, obj) } resourceBCount := 4 @@ -1476,7 +1467,7 @@ 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) - TestPropogateStoreWithKey(ctx, t, store, key, obj) + TestPropagateStoreWithKey(ctx, t, store, key, obj) } resourceACountGot, err := store.Count(resourceA) diff --git a/pkg/storage/testing/utils.go b/pkg/storage/testing/utils.go index 090accb57..42db0cbff 100644 --- a/pkg/storage/testing/utils.go +++ b/pkg/storage/testing/utils.go @@ -82,16 +82,16 @@ func DeepEqualSafePodSpec() example.PodSpec { } } -// TestPropogateStore 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 TestPropogateStore(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 := "/testkey" - return key, TestPropogateStoreWithKey(ctx, t, store, key, obj) + return key, TestPropagateStoreWithKey(ctx, t, store, key, obj) } -// TestPropogateStoreWithKey helps propagate store with objects, the given object will be stored at the specified key. -func TestPropogateStoreWithKey(ctx context.Context, t *testing.T, store storage.Interface, key string, obj *example.Pod) *example.Pod { +// 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) { diff --git a/pkg/storage/testing/watcher_tests.go b/pkg/storage/testing/watcher_tests.go index f6717fcbb..bbdf5c4ad 100644 --- a/pkg/storage/testing/watcher_tests.go +++ b/pkg/storage/testing/watcher_tests.go @@ -122,7 +122,7 @@ func testWatch(ctx context.Context, t *testing.T, store storage.Interface, recur } func RunTestDeleteTriggerWatch(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropogateStore(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) @@ -134,7 +134,7 @@ func RunTestDeleteTriggerWatch(ctx context.Context, t *testing.T, store storage. } func RunTestWatchFromNoneZero(ctx context.Context, t *testing.T, store storage.Interface) { - key, storedObj := TestPropogateStore(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 { @@ -176,7 +176,7 @@ func RunTestWatchInitializationSignal(ctx context.Context, t *testing.T, store s initSignal := utilflowcontrol.NewInitializationSignal() ctx = utilflowcontrol.WithInitializationSignal(ctx, initSignal) - key, storedObj := TestPropogateStore(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)