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 4affc333d14..3b9a25ec420 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 @@ -35,7 +35,7 @@ import ( clientv3 "go.etcd.io/etcd/client/v3" "google.golang.org/grpc/grpclog" - apitesting "k8s.io/apimachinery/pkg/api/apitesting" + "k8s.io/apimachinery/pkg/api/apitesting" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/conversion" @@ -300,34 +300,39 @@ func TestUnconditionalDelete(t *testing.T) { key, storedObj := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) tests := []struct { + name string key string expectedObj *example.Pod expectNotFoundErr bool - }{{ // test unconditional delete on existing key + }{{ + name: "existing key", key: key, expectedObj: storedObj, expectNotFoundErr: false, - }, { // test unconditional delete on non-existing key + }, { + name: "non-existing key", key: "/non-existing", expectedObj: nil, expectNotFoundErr: true, }} - for i, tt := range tests { - out := &example.Pod{} // reset - err := store.Delete(ctx, tt.key, out, nil, storage.ValidateAllObjectFunc, nil) - if tt.expectNotFoundErr { - if err == nil || !storage.IsNotFound(err) { - t.Errorf("#%d: expecting not found error, but get: %s", i, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &example.Pod{} // reset + err := store.Delete(ctx, tt.key, out, nil, storage.ValidateAllObjectFunc, nil) + if tt.expectNotFoundErr { + if err == nil || !storage.IsNotFound(err) { + t.Errorf("%s: expecting not found error, but get: %s", tt.name, err) + } + return } - continue - } - if err != nil { - t.Fatalf("Delete failed: %v", err) - } - if !reflect.DeepEqual(tt.expectedObj, out) { - t.Errorf("#%d: pod want=%#v, get=%#v", i, tt.expectedObj, out) - } + if err != nil { + t.Fatalf("%s: Delete failed: %v", tt.name, err) + } + if !reflect.DeepEqual(tt.expectedObj, out) { + t.Errorf("%s: pod want=%#v, get=%#v", tt.name, tt.expectedObj, out) + } + }) } } @@ -336,32 +341,37 @@ func TestConditionalDelete(t *testing.T) { key, storedObj := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) tests := []struct { + name string precondition *storage.Preconditions expectInvalidObjErr bool - }{{ // test conditional delete with UID match + }{{ + name: "UID match", precondition: storage.NewUIDPreconditions("A"), expectInvalidObjErr: false, - }, { // test conditional delete with UID mismatch + }, { + name: "UID mismatch", precondition: storage.NewUIDPreconditions("B"), expectInvalidObjErr: true, }} - for i, tt := range tests { - out := &example.Pod{} - err := store.Delete(ctx, key, out, tt.precondition, storage.ValidateAllObjectFunc, nil) - if tt.expectInvalidObjErr { - if err == nil || !storage.IsInvalidObj(err) { - t.Errorf("#%d: expecting invalid UID error, but get: %s", i, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &example.Pod{} + err := store.Delete(ctx, key, out, tt.precondition, storage.ValidateAllObjectFunc, nil) + if tt.expectInvalidObjErr { + if err == nil || !storage.IsInvalidObj(err) { + t.Errorf("%s: expecting invalid UID error, but get: %s", tt.name, err) + } + return } - continue - } - if err != nil { - t.Fatalf("Delete failed: %v", err) - } - if !reflect.DeepEqual(storedObj, out) { - t.Errorf("#%d: pod want=%#v, get=%#v", i, storedObj, out) - } - key, storedObj = testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) + if err != nil { + t.Fatalf("%s: Delete failed: %v", tt.name, err) + } + if !reflect.DeepEqual(storedObj, out) { + t.Errorf("%s: pod want=%#v, get=%#v", tt.name, storedObj, out) + } + key, storedObj = testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) + }) } } @@ -542,68 +552,80 @@ func TestGetToList(t *testing.T) { currentRV, _ := strconv.Atoi(storedObj.ResourceVersion) tests := []struct { + name string key string pred storage.SelectionPredicate expectedOut []*example.Pod rv string rvMatch metav1.ResourceVersionMatch expectRVTooLarge bool - }{{ // test GetToList on existing key + }{{ + name: "existing key", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, - }, { // test GetToList on existing key with minimum resource version set to 0 + }, { + name: "existing key, resourceVersion=0", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: "0", - }, { // test GetToList on existing key with minimum resource version set to 0, match=minimum + }, { + name: "existing key, resourceVersion=0, resourceVersionMatch=notOlderThan", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: "0", rvMatch: metav1.ResourceVersionMatchNotOlderThan, - }, { // test GetToList on existing key with minimum resource version set to current resource version + }, { + name: "existing key, resourceVersion=current", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: fmt.Sprintf("%d", currentRV), - }, { // test GetToList on existing key with minimum resource version set to current resource version, match=minimum + }, { + name: "existing key, resourceVersion=current, resourceVersionMatch=notOlderThan", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: fmt.Sprintf("%d", currentRV), rvMatch: metav1.ResourceVersionMatchNotOlderThan, - }, { // test GetToList on existing key with minimum resource version set to previous resource version, match=minimum + }, { + name: "existing key, resourceVersion=previous, resourceVersionMatch=notOlderThan", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: fmt.Sprintf("%d", prevRV), rvMatch: metav1.ResourceVersionMatchNotOlderThan, - }, { // test GetToList on existing key with resource version set to current resource version, match=exact + }, { + name: "existing key, resourceVersion=current, resourceVersionMatch=exact", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: fmt.Sprintf("%d", currentRV), rvMatch: metav1.ResourceVersionMatchExact, - }, { // test GetToList on existing key with resource version set to previous resource version, match=exact + }, { + name: "existing key, resourceVersion=current, resourceVersionMatch=exact", key: prevKey, pred: storage.Everything, expectedOut: []*example.Pod{prevStoredObj}, rv: fmt.Sprintf("%d", prevRV), rvMatch: metav1.ResourceVersionMatchExact, - }, { // test GetToList on existing key with minimum resource version set too high + }, { + name: "existing key, resourceVersion=too high", key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, rv: fmt.Sprintf("%d", currentRV+1), expectRVTooLarge: true, - }, { // test GetToList on non-existing key + }, { + name: "non-existing key", key: "/non-existing", pred: storage.Everything, expectedOut: nil, - }, { // test GetToList with matching pod name - key: "/non-existing", + }, { + name: "with matching pod name", + key: "/non-existing", pred: storage.SelectionPredicate{ Label: labels.Everything(), Field: fields.ParseSelectorOrDie("metadata.name!=" + storedObj.Name), @@ -615,33 +637,35 @@ func TestGetToList(t *testing.T) { expectedOut: nil, }} - for i, tt := range tests { - out := &example.PodList{} - err := store.GetToList(ctx, tt.key, storage.ListOptions{ResourceVersion: tt.rv, ResourceVersionMatch: tt.rvMatch, Predicate: tt.pred}, out) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := &example.PodList{} + err := store.GetToList(ctx, tt.key, storage.ListOptions{ResourceVersion: tt.rv, ResourceVersionMatch: tt.rvMatch, Predicate: tt.pred}, out) - if tt.expectRVTooLarge { - if err == nil || !storage.IsTooLargeResourceVersion(err) { - t.Errorf("#%d: expecting resource version too high error, but get: %s", i, err) + if tt.expectRVTooLarge { + if err == nil || !storage.IsTooLargeResourceVersion(err) { + t.Errorf("%s: expecting resource version too high error, but get: %s", tt.name, err) + } + return } - continue - } - if err != nil { - t.Fatalf("GetToList 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) + if err != nil { + t.Fatalf("%s: GetToList failed: %v", tt.name, err) } - } + 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] + if !reflect.DeepEqual(wantPod, getPod) { + t.Errorf("%s: pod want=%#v, get=%#v", tt.name, wantPod, getPod) + } + } + }) } } @@ -650,6 +674,7 @@ func TestGuaranteedUpdate(t *testing.T) { key := "/testkey" tests := []struct { + name string key string ignoreNotFound bool precondition *storage.Preconditions @@ -658,35 +683,40 @@ func TestGuaranteedUpdate(t *testing.T) { expectNoUpdate bool transformStale bool hasSelfLink bool - }{{ // GuaranteedUpdate on non-existing key with ignoreNotFound=false + }{{ + name: "non-existing key, ignoreNotFound=false", key: "/non-existing", ignoreNotFound: false, precondition: nil, expectNotFoundErr: true, expectInvalidObjErr: false, expectNoUpdate: false, - }, { // GuaranteedUpdate on non-existing key with ignoreNotFound=true + }, { + name: "non-existing key, ignoreNotFound=true", key: "/non-existing", ignoreNotFound: true, precondition: nil, expectNotFoundErr: false, expectInvalidObjErr: false, expectNoUpdate: false, - }, { // GuaranteedUpdate on existing key + }, { + name: "existing key", key: key, ignoreNotFound: false, precondition: nil, expectNotFoundErr: false, expectInvalidObjErr: false, expectNoUpdate: false, - }, { // GuaranteedUpdate with same data + }, { + name: "same data", key: key, ignoreNotFound: false, precondition: nil, expectNotFoundErr: false, expectInvalidObjErr: false, expectNoUpdate: true, - }, { // GuaranteedUpdate with same data AND a selfLink + }, { + name: "same data, a selfLink", key: key, ignoreNotFound: false, precondition: nil, @@ -694,7 +724,8 @@ func TestGuaranteedUpdate(t *testing.T) { expectInvalidObjErr: false, expectNoUpdate: true, hasSelfLink: true, - }, { // GuaranteedUpdate with same data but stale + }, { + name: "same data, stale", key: key, ignoreNotFound: false, precondition: nil, @@ -702,14 +733,16 @@ func TestGuaranteedUpdate(t *testing.T) { expectInvalidObjErr: false, expectNoUpdate: false, transformStale: true, - }, { // GuaranteedUpdate with UID match + }, { + name: "UID match", key: key, ignoreNotFound: false, precondition: storage.NewUIDPreconditions("A"), expectNotFoundErr: false, expectInvalidObjErr: false, expectNoUpdate: true, - }, { // GuaranteedUpdate with UID mismatch + }, { + name: "UID mismatch", key: key, ignoreNotFound: false, precondition: storage.NewUIDPreconditions("B"), @@ -719,71 +752,73 @@ func TestGuaranteedUpdate(t *testing.T) { }} for i, tt := range tests { - key, storeObj := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) + t.Run(tt.name, func(t *testing.T) { + key, storeObj := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "A"}}) - out := &example.Pod{} - name := fmt.Sprintf("foo-%d", i) - if tt.expectNoUpdate { - name = storeObj.Name - } - originalTransformer := store.transformer.(*prefixTransformer) - if tt.transformStale { - transformer := *originalTransformer - transformer.stale = true - store.transformer = &transformer - } - version := storeObj.ResourceVersion - err := store.GuaranteedUpdate(ctx, tt.key, out, tt.ignoreNotFound, tt.precondition, - storage.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) { - if tt.expectNotFoundErr && tt.ignoreNotFound { - if pod := obj.(*example.Pod); pod.Name != "" { - t.Errorf("#%d: expecting zero value, but get=%#v", i, pod) + out := &example.Pod{} + name := fmt.Sprintf("foo-%d", i) + if tt.expectNoUpdate { + name = storeObj.Name + } + originalTransformer := store.transformer.(*prefixTransformer) + if tt.transformStale { + transformer := *originalTransformer + transformer.stale = true + store.transformer = &transformer + } + version := storeObj.ResourceVersion + err := store.GuaranteedUpdate(ctx, tt.key, out, tt.ignoreNotFound, tt.precondition, + storage.SimpleUpdate(func(obj runtime.Object) (runtime.Object, error) { + if tt.expectNotFoundErr && tt.ignoreNotFound { + if pod := obj.(*example.Pod); pod.Name != "" { + t.Errorf("%s: expecting zero value, but get=%#v", tt.name, pod) + } } + pod := *storeObj + if tt.hasSelfLink { + pod.SelfLink = "testlink" + } + pod.Name = name + return &pod, nil + }), nil) + store.transformer = originalTransformer + + if tt.expectNotFoundErr { + if err == nil || !storage.IsNotFound(err) { + t.Errorf("%s: expecting not found error, but get: %v", tt.name, err) } - pod := *storeObj - if tt.hasSelfLink { - pod.SelfLink = "testlink" + return + } + if tt.expectInvalidObjErr { + if err == nil || !storage.IsInvalidObj(err) { + t.Errorf("%s: expecting invalid UID error, but get: %s", tt.name, err) } - pod.Name = name - return &pod, nil - }), nil) - store.transformer = originalTransformer + return + } + if err != nil { + t.Fatalf("%s: GuaranteedUpdate failed: %v", tt.name, err) + } + if out.ObjectMeta.Name != name { + t.Errorf("%s: pod name want=%s, get=%s", tt.name, name, out.ObjectMeta.Name) + } + if out.SelfLink != "" { + t.Errorf("%s: selfLink should not be set", tt.name) + } - if tt.expectNotFoundErr { - if err == nil || !storage.IsNotFound(err) { - t.Errorf("#%d: expecting not found error, but get: %v", i, err) - } - continue - } - if tt.expectInvalidObjErr { - if err == nil || !storage.IsInvalidObj(err) { - t.Errorf("#%d: expecting invalid UID error, but get: %s", i, err) - } - continue - } - if err != nil { - t.Fatalf("GuaranteedUpdate failed: %v", err) - } - if out.ObjectMeta.Name != name { - t.Errorf("#%d: pod name want=%s, get=%s", i, name, out.ObjectMeta.Name) - } - if out.SelfLink != "" { - t.Errorf("#%d: selfLink should not be set", i) - } + // verify that kv pair is not empty after set and that the underlying data matches expectations + checkStorageInvariants(ctx, t, etcdClient, store, key) - // verify that kv pair is not empty after set and that the underlying data matches expectations - checkStorageInvariants(ctx, t, etcdClient, store, key) - - switch tt.expectNoUpdate { - case true: - if version != out.ResourceVersion { - t.Errorf("#%d: expect no version change, before=%s, after=%s", i, version, out.ResourceVersion) + switch tt.expectNoUpdate { + case true: + if version != out.ResourceVersion { + t.Errorf("%s: expect no version change, before=%s, after=%s", tt.name, version, out.ResourceVersion) + } + case false: + if version == out.ResourceVersion { + t.Errorf("%s: expect version change, but get the same version=%s", tt.name, version) + } } - case false: - if version == out.ResourceVersion { - t.Errorf("#%d: expect version change, but get the same version=%s", i, version) - } - } + }) } }