Merge pull request #113427 from wojtek-t/reuse_generic_tests_for_cacher

Reuse generic TestGet in cache tests

staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path"
 	"reflect"
 	"sync/atomic"
 	"testing"
@@ -60,8 +61,9 @@ func newPod() runtime.Object {
 	return &example.Pod{}
 }
 
-func checkStorageInvariants(etcdClient *clientv3.Client, codec runtime.Codec) storagetesting.KeyValidation {
+func checkStorageInvariants(prefix string, etcdClient *clientv3.Client, codec runtime.Codec) storagetesting.KeyValidation {
 	return func(ctx context.Context, t *testing.T, key string) {
+		key = path.Join(prefix, key)
 		getResp, err := etcdClient.KV.Get(ctx, key)
 		if err != nil {
 			t.Fatalf("etcdClient.KV.Get failed: %v", err)
@@ -85,7 +87,7 @@ 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))
+	storagetesting.RunTestCreate(ctx, t, store, checkStorageInvariants("/", etcdClient, store.codec))
 }
 
 func TestCreateWithTTL(t *testing.T) {
@@ -158,7 +160,7 @@ func (s *storeWithPrefixTransformer) UpdatePrefixTransformer(modifier storagetes
 
 func TestGuaranteedUpdate(t *testing.T) {
 	ctx, store, etcdClient := testSetup(t)
-	storagetesting.RunTestGuaranteedUpdate(ctx, t, &storeWithPrefixTransformer{store}, checkStorageInvariants(etcdClient, store.codec))
+	storagetesting.RunTestGuaranteedUpdate(ctx, t, &storeWithPrefixTransformer{store}, checkStorageInvariants("/", etcdClient, store.codec))
 }
 
 func TestGuaranteedUpdateWithTTL(t *testing.T) {

staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go

@@ -129,33 +129,40 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) {
 		ignoreNotFound    bool
 		expectNotFoundErr bool
 		expectRVTooLarge  bool
-		expectedOut       *example.Pod
+		expectedOut       []*example.Pod
 		rv                string
 	}{{
 		name:              "get existing",
 		key:               key,
 		ignoreNotFound:    false,
 		expectNotFoundErr: false,
-		expectedOut:       storedObj,
+		expectedOut:       []*example.Pod{storedObj},
 	}, {
-		name:        "resource version 0",
-		key:         key,
-		expectedOut: storedObj,
-		rv:          "0",
+		// 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",
 	}, {
+		// 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: storedObj,
+		expectedOut: []*example.Pod{createdObj, storedObj},
 		rv:          createdObj.ResourceVersion,
 	}, {
 		name:        "current object resource version, match=NotOlderThan",
 		key:         key,
-		expectedOut: storedObj,
+		expectedOut: []*example.Pod{storedObj},
 		rv:          fmt.Sprintf("%d", currentRV),
 	}, {
 		name:        "latest resource version",
 		key:         key,
-		expectedOut: storedObj,
+		expectedOut: []*example.Pod{storedObj},
 		rv:          fmt.Sprintf("%d", lastUpdatedCurrentRV),
 	}, {
 		name:             "too high resource version",
@@ -172,7 +179,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 {
@@ -194,7 +201,19 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) {
 			if err != nil {
 				t.Fatalf("Get failed: %v", err)
 			}
-			ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), tt.expectedOut, out)
+
+			if len(tt.expectedOut) == 1 {
+				ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), tt.expectedOut[0], out)
+			} else {
+				toInterfaceSlice := func(pods []*example.Pod) []interface{} {
+					result := make([]interface{}, 0, len(pods))
+					for i := range pods {
+						result = append(result, pods[i])
+					}
+					return result
+				}
+				ExpectContains(t, fmt.Sprintf("%s: incorrect pod", tt.name), toInterfaceSlice(tt.expectedOut), out)
+			}
 		})
 	}
 }
@@ -1675,7 +1694,7 @@ func RunTestConsistentList(ctx context.Context, t *testing.T, store InterfaceWit
 }
 
 func RunTestGuaranteedUpdate(ctx context.Context, t *testing.T, store InterfaceWithPrefixTransformer, validation KeyValidation) {
-	key := "/testkey"
+	key := "/foo"
 
 	tests := []struct {
 		name                string

staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go

@@ -86,7 +86,7 @@ func DeepEqualSafePodSpec() example.PodSpec {
 // keys and stored objects.
 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"
+	key := fmt.Sprintf("/%s/%s", obj.Namespace, obj.Name)
 	return key, TestPropagateStoreWithKey(ctx, t, store, key, obj)
 }
 
@@ -115,6 +115,24 @@ func ExpectNoDiff(t *testing.T, msg string, expected, got interface{}) {
 	}
 }
 
+func ExpectContains(t *testing.T, msg string, expectedList []interface{}, got interface{}) {
+	t.Helper()
+	for _, expected := range expectedList {
+		if reflect.DeepEqual(expected, got) {
+			return
+		}
+	}
+	if len(expectedList) == 0 {
+		t.Errorf("%s: empty expectedList", msg)
+		return
+	}
+	if diff := cmp.Diff(expectedList[0], got); diff != "" {
+		t.Errorf("%s: differs from all items, with first: %s", msg, diff)
+	} else {
+		t.Errorf("%s: differs from all items, first: %#v\ngot: %#v", msg, expectedList[0], got)
+	}
+}
+
 const dummyPrefix = "adapter"
 
 func EncodeContinueOrDie(key string, resourceVersion int64) string {

staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go

@@ -71,8 +71,8 @@ func init() {
 	utilruntime.Must(examplev1.AddToScheme(scheme))
 }
 
-// GetAttrs returns labels and fields of a given object for filtering purposes.
-func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, error) {
+// GetPodAttrs returns labels and fields of a given object for filtering purposes.
+func GetPodAttrs(obj runtime.Object) (labels.Set, fields.Set, error) {
 	pod, ok := obj.(*example.Pod)
 	if !ok {
 		return nil, nil, fmt.Errorf("not a pod")
@@ -124,7 +124,7 @@ func newTestCacherWithClock(s storage.Interface, clock clock.Clock) (*cacherstor
 		GroupResource:  schema.GroupResource{Resource: "pods"},
 		ResourcePrefix: prefix,
 		KeyFunc:        func(obj runtime.Object) (string, error) { return storage.NamespaceKeyFunc(prefix, obj) },
-		GetAttrsFunc:   GetAttrs,
+		GetAttrsFunc:   GetPodAttrs,
 		NewFunc:        newPod,
 		NewListFunc:    newPodList,
 		Codec:          codecs.LegacyCodec(examplev1.SchemeGroupVersion),
@@ -164,37 +164,9 @@ func updatePod(t *testing.T, s storage.Interface, obj, old *example.Pod) *exampl
 }
 
 func TestGet(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()
-
-	podFoo := makeTestPod("foo")
-	fooCreated := updatePod(t, etcdStorage, podFoo, nil)
-
-	// We pass the ResourceVersion from the above Create() operation.
-	result := &example.Pod{}
-	if err := cacher.Get(context.TODO(), "pods/ns/foo", storage.GetOptions{IgnoreNotFound: true, ResourceVersion: fooCreated.ResourceVersion}, result); err != nil {
-		t.Errorf("Unexpected error: %v", err)
-	}
-	if e, a := *fooCreated, *result; !reflect.DeepEqual(e, a) {
-		t.Errorf("Expected: %#v, got: %#v", e, a)
-	}
-
-	if err := cacher.Get(context.TODO(), "pods/ns/bar", storage.GetOptions{ResourceVersion: fooCreated.ResourceVersion, IgnoreNotFound: true}, result); err != nil {
-		t.Errorf("Unexpected error: %v", err)
-	}
-	emptyPod := example.Pod{}
-	if e, a := emptyPod, *result; !reflect.DeepEqual(e, a) {
-		t.Errorf("Expected: %#v, got: %#v", e, a)
-	}
-
-	if err := cacher.Get(context.TODO(), "pods/ns/bar", storage.GetOptions{ResourceVersion: fooCreated.ResourceVersion}, result); !storage.IsNotFound(err) {
-		t.Errorf("Unexpected error: %v", err)
-	}
+	ctx, cacher, terminate := testSetup(t)
+	defer terminate()
+	storagetesting.RunTestGet(ctx, t, cacher)
 }
 
 func TestGetListNonRecursive(t *testing.T) {
@@ -962,3 +934,58 @@ func TestWatchBookmarksWithCorrectResourceVersion(t *testing.T) {
 		}
 	}
 }
+
+// ===================================================
+// Test-setup related function are following.
+// ===================================================
+
+type tearDownFunc func()
+
+type setupOptions struct {
+	resourcePrefix string
+	keyFunc        func(runtime.Object) (string, error)
+	clock          clock.Clock
+}
+
+type setupOption func(*setupOptions)
+
+func withDefaults(options *setupOptions) {
+	prefix := ""
+
+	options.resourcePrefix = prefix
+	options.keyFunc = func(obj runtime.Object) (string, error) { return storage.NamespaceKeyFunc(prefix, obj) }
+	options.clock = clock.RealClock{}
+}
+
+func testSetup(t *testing.T, opts ...setupOption) (context.Context, *cacherstorage.Cacher, tearDownFunc) {
+	setupOpts := setupOptions{}
+	opts = append([]setupOption{withDefaults}, opts...)
+	for _, opt := range opts {
+		opt(&setupOpts)
+	}
+
+	server, etcdStorage := newEtcdTestStorage(t, etcd3testing.PathPrefix())
+	config := cacherstorage.Config{
+		Storage:        etcdStorage,
+		Versioner:      storage.APIObjectVersioner{},
+		GroupResource:  schema.GroupResource{Resource: "pods"},
+		ResourcePrefix: setupOpts.resourcePrefix,
+		KeyFunc:        setupOpts.keyFunc,
+		GetAttrsFunc:   GetPodAttrs,
+		NewFunc:        newPod,
+		NewListFunc:    newPodList,
+		Codec:          codecs.LegacyCodec(examplev1.SchemeGroupVersion),
+		Clock:          setupOpts.clock,
+	}
+	cacher, err := cacherstorage.NewCacherFromConfig(config)
+	if err != nil {
+		t.Fatalf("Failed to initialize cacher: %v", err)
+	}
+	ctx := context.Background()
+	terminate := func() {
+		cacher.Stop()
+		server.Terminate(t)
+	}
+
+	return ctx, cacher, terminate
+}