Skip to content

Commit

Permalink
storage: Get rid of TestListDeprecated
Browse files Browse the repository at this point in the history 
This commit extends the test cases of RunTestList
to include the things tested by TestListDeprecated
and subsequently deletes the test.

This additionally adds a test case for checking that
the list return the modified version of an object.

Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
  • Loading branch information
@MadhavJivrajani
MadhavJivrajani committed Feb 23, 2023
1 parent 1d63908 commit 7474d9b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 98 deletions.
55 changes: 54 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go
View file
Expand Up @@ -502,7 +502,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface, ign

getAttrs := func(obj runtime.Object) (labels.Set, fields.Set, error) {
pod := obj.(*example.Pod)
return nil, fields.Set{"metadata.name": pod.Name}, nil
return nil, fields.Set{"metadata.name": pod.Name, "spec.nodeName": pod.Spec.NodeName}, nil
}

tests := []struct {
Expand Down Expand Up @@ -993,6 +993,30 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface, ign
rvMatch: metav1.ResourceVersionMatchNotOlderThan,
expectedOut: []example.Pod{*preset[0], *preset[1], *preset[2], *preset[3], *preset[4]},
},
{
name: "verify list returns updated version of object; filter returns one item, page size equal to total list size with current resource version and match=NotOlderThan",
prefix: "/pods",
pred: storage.SelectionPredicate{
Field: fields.OneTermEqualSelector("spec.nodeName", "fakeNode"),
Label: labels.Everything(),
Limit: 5,
},
rv: list.ResourceVersion,
rvMatch: metav1.ResourceVersionMatchNotOlderThan,
expectedOut: []example.Pod{*preset[0]},
},
{
name: "verify list does not return deleted object; filter for deleted object, page size equal to total list size with current resource version and match=NotOlderThan",
prefix: "/pods",
pred: storage.SelectionPredicate{
Field: fields.OneTermEqualSelector("metadata.name", "baz"),
Label: labels.Everything(),
Limit: 5,
},
rv: list.ResourceVersion,
rvMatch: metav1.ResourceVersionMatchNotOlderThan,
expectedOut: []example.Pod{},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -1159,13 +1183,15 @@ func seedMultiLevelData(ctx context.Context, store storage.Interface) (string, [
// - second/
// | - bar
// | - foo
// | - [deleted] baz
// |
// - third/
// | - barfoo
// | - foo
barFirst := &example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "first", Name: "bar"}}
barSecond := &example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "second", Name: "bar"}}
fooSecond := &example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "second", Name: "foo"}}
bazSecond := &example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "second", Name: "baz"}}
barfooThird := &example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "third", Name: "barfoo"}}
fooThird := &example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "third", Name: "foo"}}

Expand Down Expand Up @@ -1194,6 +1220,10 @@ func seedMultiLevelData(ctx context.Context, store storage.Interface) (string, [
key: computePodKey(fooThird),
obj: fooThird,
},
{
key: computePodKey(bazSecond),
obj: bazSecond,
},
}

// we want to figure out the resourceVersion before we create anything
Expand All @@ -1211,6 +1241,29 @@ func seedMultiLevelData(ctx context.Context, store storage.Interface) (string, [
}
}

// For barFirst, we first create it with key /pods/first/bar and then we update
// it by changing its spec.nodeName. The point of doing this is to be able to
// test that if a pod with key /pods/first/bar is in fact returned, the returned
// pod is the updated one (i.e. with spec.nodeName changed).
preset[0].storedObj = &example.Pod{}
if err := store.GuaranteedUpdate(ctx, computePodKey(barFirst), preset[0].storedObj, true, nil,
func(input runtime.Object, _ storage.ResponseMeta) (output runtime.Object, ttl *uint64, err error) {
pod := input.(*example.Pod).DeepCopy()
pod.Spec.NodeName = "fakeNode"
return pod, nil, nil
}, nil); err != nil {
return "", nil, fmt.Errorf("failed to update object: %w", err)
}

// We now delete bazSecond provided it has been created first. We do this to enable
// testing cases that had an object exist initially and then was deleted and how this
// would be reflected in responses of different calls.
if err := store.Delete(ctx, computePodKey(bazSecond), preset[len(preset)-1].storedObj, nil, storage.ValidateAllObjectFunc, nil); err != nil {
return "", nil, fmt.Errorf("failed to delete object: %w", err)
}

// Since we deleted bazSecond (last element of preset), we remove it from preset.
preset = preset[:len(preset)-1]
var created []*example.Pod
for _, item := range preset {
created = append(created, item.storedObj)
Expand Down
97 changes: 0 additions & 97 deletions staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go
View file
Expand Up @@ -19,7 +19,6 @@ package tests
import (
"context"
"fmt"
"reflect"
goruntime "runtime"
"strconv"
"sync"
Expand All @@ -37,7 +36,6 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/apiserver/pkg/apis/example"
Expand Down Expand Up @@ -181,101 +179,6 @@ func TestList(t *testing.T) {
storagetesting.RunTestList(ctx, t, cacher, true)
}

// TODO(wojtek-t): We should extend the generic RunTestList test to cover the
// scenarios that are not yet covered by it and get rid of this test.
func TestListDeprecated(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")
podBar := makeTestPod("bar")
podBaz := makeTestPod("baz")

podFooPrime := makeTestPod("foo")
podFooPrime.Spec.NodeName = "fakeNode"

fooCreated := updatePod(t, etcdStorage, podFoo, nil)
_ = updatePod(t, etcdStorage, podBar, nil)
_ = updatePod(t, etcdStorage, podBaz, nil)

_ = updatePod(t, etcdStorage, podFooPrime, fooCreated)

// Create a pod in a namespace that contains "ns" as a prefix
// Make sure it is not returned in a watch of "ns"
podFooNS2 := makeTestPod("foo")
podFooNS2.Namespace += "2"
updatePod(t, etcdStorage, podFooNS2, nil)

deleted := example.Pod{}
if err := etcdStorage.Delete(context.TODO(), "pods/ns/bar", &deleted, nil, storage.ValidateAllObjectFunc, nil); err != nil {
t.Errorf("Unexpected error: %v", err)
}

// We first List directly from etcd by passing empty resourceVersion,
// to get the current etcd resourceVersion.
rvResult := &example.PodList{}
options := storage.ListOptions{
Predicate: storage.Everything,
Recursive: true,
}
if err := cacher.GetList(context.TODO(), "pods/ns", options, rvResult); err != nil {
t.Errorf("Unexpected error: %v", err)
}
deletedPodRV := rvResult.ListMeta.ResourceVersion

result := &example.PodList{}
// We pass the current etcd ResourceVersion received from the above List() operation,
// since there is not easy way to get ResourceVersion of barPod deletion operation.
options = storage.ListOptions{
ResourceVersion: deletedPodRV,
Predicate: storage.Everything,
Recursive: true,
}
if err := cacher.GetList(context.TODO(), "pods/ns", options, result); err != nil {
t.Errorf("Unexpected error: %v", err)
}
if result.ListMeta.ResourceVersion != deletedPodRV {
t.Errorf("Incorrect resource version: %v", result.ListMeta.ResourceVersion)
}
if len(result.Items) != 2 {
t.Errorf("Unexpected list result: %d", len(result.Items))
}
keys := sets.String{}
for _, item := range result.Items {
keys.Insert(item.Name)
}
if !keys.HasAll("foo", "baz") {
t.Errorf("Unexpected list result: %#v", result)
}
for _, item := range result.Items {
// unset fields that are set by the infrastructure
item.ResourceVersion = ""
item.CreationTimestamp = metav1.Time{}

if item.Namespace != "ns" {
t.Errorf("Unexpected namespace: %s", item.Namespace)
}

var expected *example.Pod
switch item.Name {
case "foo":
expected = podFooPrime
case "baz":
expected = podBaz
default:
t.Errorf("Unexpected item: %v", item)
}
if e, a := *expected, item; !reflect.DeepEqual(e, a) {
t.Errorf("Expected: %#v, got: %#v", e, a)
}
}
}

func verifyWatchEvent(t *testing.T, w watch.Interface, eventType watch.EventType, eventObject runtime.Object) {
_, _, line, _ := goruntime.Caller(1)
select {
Expand Down

0 comments on commit 7474d9b

Please sign in to comment.