Skip to content

Commit

Permalink
Merge pull request #94818 from tkashem/automated-cherry-pick-of-#9477…
Browse files Browse the repository at this point in the history
…3-upstream-release-1.18

Automated cherry pick of #94773: count of etcd object should be limited to the specified
  • Loading branch information
k8s-ci-robot committed Oct 6, 2020
2 parents 5b7305b + 23d452b commit 94a04e8
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
8 changes: 8 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go
Expand Up @@ -440,6 +440,14 @@ func getNewItemFunc(listObj runtime.Object, v reflect.Value) func() runtime.Obje

func (s *store) Count(key string) (int64, error) {
key = path.Join(s.pathPrefix, key)

// We need to make sure the key ended with "/" so that we only get children "directories".
// e.g. if we have key "/a", "/a/b", "/ab", getting keys with prefix "/a" will return all three,
// while with prefix "/a/" will return only "/a/b" which is the correct answer.
if !strings.HasSuffix(key, "/") {
key += "/"
}

startTime := time.Now()
getResp, err := s.client.KV.Get(context.Background(), key, clientv3.WithRange(clientv3.GetPrefixRangeEnd(key)), clientv3.WithCountOnly())
metrics.RecordEtcdRequestLatency("listWithCount", key, startTime)
Expand Down
46 changes: 45 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go
Expand Up @@ -1617,6 +1617,12 @@ func testSetup(t *testing.T) (context.Context, *store, *integration.ClusterV3) {
func testPropogateStore(ctx context.Context, t *testing.T, store *store, 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)
}

// 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 *store, key string, obj *example.Pod) *example.Pod {
// Setup store with the specified key and grab the output for returning.
v, err := conversion.EnforcePtr(obj)
if err != nil {
panic("unable to convert output object to pointer")
Expand All @@ -1629,7 +1635,7 @@ func testPropogateStore(ctx context.Context, t *testing.T, store *store, obj *ex
if err := store.Create(ctx, key, obj, setOutput, 0); err != nil {
t.Fatalf("Set failed: %v", err)
}
return key, setOutput
return setOutput
}

func TestPrefix(t *testing.T) {
Expand Down Expand Up @@ -1846,3 +1852,41 @@ func TestConsistentList(t *testing.T) {
t.Errorf("inconsistent lists: %#v, %#v", result1, result2)
}
}

func TestCount(t *testing.T) {
ctx, store, cluster := testSetup(t)
defer cluster.Terminate(t)

resourceA := "/foo.bar.io/abc"

// resourceA is intentionally a prefix of resourceB to ensure that the count
// for resourceA does not include any objects from resourceB.
resourceB := fmt.Sprintf("%sdef", resourceA)

resourceACountExpected := 5
for i := 1; i <= resourceACountExpected; i++ {
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)
}

resourceBCount := 4
for i := 1; i <= resourceBCount; i++ {
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)
}

resourceACountGot, err := store.Count(resourceA)
if err != nil {
t.Fatalf("store.Count failed: %v", err)
}

// count for resourceA should not include the objects for resourceB
// even though resourceA is a prefix of resourceB.
if int64(resourceACountExpected) != resourceACountGot {
t.Fatalf("store.Count for resource %s: expected %d but got %d", resourceA, resourceACountExpected, resourceACountGot)
}
}

0 comments on commit 94a04e8

Please sign in to comment.