From f894f8196d9266915424e2cefc9e4eb480ae6f5e Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Mon, 9 May 2022 07:59:38 -0700 Subject: [PATCH 1/3] etcd3/store: update creation test to use storage client There is no functional difference between checking for an empty key using the database client and doing so with the storage interface. Using the latter allows this test to be more portable. Signed-off-by: Steve Kuznetsov --- .../k8s.io/apiserver/pkg/storage/etcd3/store_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index 205dfce65c8..4013a9a3237 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -115,16 +115,11 @@ func TestCreate(t *testing.T) { obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", SelfLink: "testlink"}} // verify that kv pair is empty before set - getResp, err := etcdClient.KV.Get(ctx, key) - if err != nil { - t.Fatalf("etcdClient.KV.Get failed: %v", err) - } - if len(getResp.Kvs) != 0 { - t.Fatalf("expecting empty result on key: %s", key) + if err := store.Get(ctx, key, storage.GetOptions{}, out); !storage.IsNotFound(err) { + t.Fatalf("expecting empty result on key %s, got %v", key, err) } - err = store.Create(ctx, key, obj, out, 0) - if err != nil { + if err := store.Create(ctx, key, obj, out, 0); err != nil { t.Fatalf("Set failed: %v", err) } // basic tests of the output From 6d25e96cedaad249fe75aac4b1fe08bb69829a61 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Mon, 9 May 2022 08:03:41 -0700 Subject: [PATCH 2/3] etcd3/store: make creation test validation generic Different callers to this test may need to do different backend-specific validation on the stored data, so we allow them a callback for this. Signed-off-by: Steve Kuznetsov --- .../apiserver/pkg/storage/etcd3/store_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index 4013a9a3237..dd9d325554c 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -109,7 +109,14 @@ func newPod() runtime.Object { func TestCreate(t *testing.T) { ctx, store, etcdClient := testSetup(t) + RunTestCreate(ctx, t, store, func(ctx context.Context, t *testing.T, key string) { + checkStorageInvariants(ctx, t, etcdClient, store.codec, key) + }) +} +type KeyValidation func(ctx context.Context, t *testing.T, key string) + +func RunTestCreate(ctx context.Context, t *testing.T, store storage.Interface, validation KeyValidation) { key := "/testkey" out := &example.Pod{} obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", SelfLink: "testlink"}} @@ -133,10 +140,10 @@ func TestCreate(t *testing.T) { t.Errorf("output should have empty selfLink") } - checkStorageInvariants(ctx, t, etcdClient, store, key) + validation(ctx, t, key) } -func checkStorageInvariants(ctx context.Context, t *testing.T, etcdClient *clientv3.Client, store *store, key string) { +func checkStorageInvariants(ctx context.Context, t *testing.T, etcdClient *clientv3.Client, codec runtime.Codec, key string) { getResp, err := etcdClient.KV.Get(ctx, key) if err != nil { t.Fatalf("etcdClient.KV.Get failed: %v", err) @@ -144,7 +151,7 @@ func checkStorageInvariants(ctx context.Context, t *testing.T, etcdClient *clien if len(getResp.Kvs) == 0 { t.Fatalf("expecting non empty result on key: %s", key) } - decoded, err := runtime.Decode(store.codec, getResp.Kvs[0].Value[len(defaultTestPrefix):]) + decoded, err := runtime.Decode(codec, getResp.Kvs[0].Value[len(defaultTestPrefix):]) if err != nil { t.Fatalf("expecting successful decode of object from %v\n%v", err, string(getResp.Kvs[0].Value)) } @@ -374,7 +381,7 @@ func TestGuaranteedUpdate(t *testing.T) { } // verify that kv pair is not empty after set and that the underlying data matches expectations - checkStorageInvariants(ctx, t, etcdClient, store, key) + checkStorageInvariants(ctx, t, etcdClient, store.codec, key) switch tt.expectNoUpdate { case true: From 2e118f42465efb390bec005f1853a964dd339f97 Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Mon, 9 May 2022 08:04:53 -0700 Subject: [PATCH 3/3] storage/testing: move creation test to generic package Signed-off-by: Steve Kuznetsov --- .../apiserver/pkg/storage/etcd3/store_test.go | 31 +------------------ .../pkg/storage/testing/store_tests.go | 29 +++++++++++++++++ 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index dd9d325554c..f371558784d 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -109,40 +109,11 @@ func newPod() runtime.Object { func TestCreate(t *testing.T) { ctx, store, etcdClient := testSetup(t) - RunTestCreate(ctx, t, store, func(ctx context.Context, t *testing.T, key string) { + storagetesting.RunTestCreate(ctx, t, store, func(ctx context.Context, t *testing.T, key string) { checkStorageInvariants(ctx, t, etcdClient, store.codec, key) }) } -type KeyValidation func(ctx context.Context, t *testing.T, key string) - -func RunTestCreate(ctx context.Context, t *testing.T, store storage.Interface, validation KeyValidation) { - key := "/testkey" - out := &example.Pod{} - obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", SelfLink: "testlink"}} - - // verify that kv pair is empty before set - if err := store.Get(ctx, key, storage.GetOptions{}, out); !storage.IsNotFound(err) { - t.Fatalf("expecting empty result on key %s, got %v", key, err) - } - - if err := store.Create(ctx, key, obj, out, 0); err != nil { - t.Fatalf("Set failed: %v", err) - } - // basic tests of the output - if obj.ObjectMeta.Name != out.ObjectMeta.Name { - t.Errorf("pod name want=%s, get=%s", obj.ObjectMeta.Name, out.ObjectMeta.Name) - } - if out.ResourceVersion == "" { - t.Errorf("output should have non-empty resource version") - } - if out.SelfLink != "" { - t.Errorf("output should have empty selfLink") - } - - validation(ctx, t, key) -} - func checkStorageInvariants(ctx context.Context, t *testing.T, etcdClient *clientv3.Client, codec runtime.Codec, key string) { getResp, err := etcdClient.KV.Get(ctx, key) if err != nil { 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 6db38d80880..d272a1a6ae6 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 @@ -37,6 +37,35 @@ import ( "k8s.io/apiserver/pkg/storage" ) +type KeyValidation func(ctx context.Context, t *testing.T, key string) + +func RunTestCreate(ctx context.Context, t *testing.T, store storage.Interface, validation KeyValidation) { + key := "/testkey" + out := &example.Pod{} + obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", SelfLink: "testlink"}} + + // verify that kv pair is empty before set + if err := store.Get(ctx, key, storage.GetOptions{}, out); !storage.IsNotFound(err) { + t.Fatalf("expecting empty result on key %s, got %v", key, err) + } + + if err := store.Create(ctx, key, obj, out, 0); err != nil { + t.Fatalf("Set failed: %v", err) + } + // basic tests of the output + if obj.ObjectMeta.Name != out.ObjectMeta.Name { + t.Errorf("pod name want=%s, get=%s", obj.ObjectMeta.Name, out.ObjectMeta.Name) + } + if out.ResourceVersion == "" { + t.Errorf("output should have non-empty resource version") + } + if out.SelfLink != "" { + t.Errorf("output should have empty selfLink") + } + + validation(ctx, t, key) +} + func RunTestCreateWithTTL(ctx context.Context, t *testing.T, store storage.Interface) { input := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} key := "/somekey"