Skip to content

Commit c968317

Browse files
authored
Merge pull request kubernetes#89575 from tnqn/improve-tracker
Improve fake clientset performance
2 parents 59c66da + 7e15e31 commit c968317

File tree

3 files changed

+44
-57
lines changed

3 files changed

+44
-57
lines changed

staging/src/k8s.io/client-go/dynamic/fake/simple_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ func TestList(t *testing.T) {
7575
}
7676

7777
expected := []unstructured.Unstructured{
78-
*newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"),
7978
*newUnstructured("group/version", "TheKind", "ns-foo", "name-bar"),
8079
*newUnstructured("group/version", "TheKind", "ns-foo", "name-baz"),
80+
*newUnstructured("group/version", "TheKind", "ns-foo", "name-foo"),
8181
}
8282
if !equality.Semantic.DeepEqual(listFirst.Items, expected) {
8383
t.Fatal(diff.ObjectGoPrintDiff(expected, listFirst.Items))

staging/src/k8s.io/client-go/metadata/fake/simple_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ func TestList(t *testing.T) {
7979
}
8080

8181
expected := []metav1.PartialObjectMetadata{
82-
*newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-foo"),
8382
*newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-bar"),
8483
*newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-baz"),
84+
*newPartialObjectMetadata("group/version", "TheKind", "ns-foo", "name-foo"),
8585
}
8686
if !equality.Semantic.DeepEqual(listFirst.Items, expected) {
8787
t.Fatal(diff.ObjectGoPrintDiff(expected, listFirst.Items))

staging/src/k8s.io/client-go/testing/fixture.go

Lines changed: 42 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package testing
1919
import (
2020
"fmt"
2121
"reflect"
22+
"sort"
2223
"sync"
2324

2425
jsonpatch "github.com/evanphx/json-patch"
@@ -197,7 +198,7 @@ type tracker struct {
197198
scheme ObjectScheme
198199
decoder runtime.Decoder
199200
lock sync.RWMutex
200-
objects map[schema.GroupVersionResource][]runtime.Object
201+
objects map[schema.GroupVersionResource]map[types.NamespacedName]runtime.Object
201202
// The value type of watchers is a map of which the key is either a namespace or
202203
// all/non namespace aka "" and its value is list of fake watchers.
203204
// Manipulations on resources will broadcast the notification events into the
@@ -214,7 +215,7 @@ func NewObjectTracker(scheme ObjectScheme, decoder runtime.Decoder) ObjectTracke
214215
return &tracker{
215216
scheme: scheme,
216217
decoder: decoder,
217-
objects: make(map[schema.GroupVersionResource][]runtime.Object),
218+
objects: make(map[schema.GroupVersionResource]map[types.NamespacedName]runtime.Object),
218219
watchers: make(map[schema.GroupVersionResource]map[string][]*watch.RaceFreeFakeWatcher),
219220
}
220221
}
@@ -282,31 +283,15 @@ func (t *tracker) Get(gvr schema.GroupVersionResource, ns, name string) (runtime
282283
return nil, errNotFound
283284
}
284285

285-
var matchingObjs []runtime.Object
286-
for _, obj := range objs {
287-
acc, err := meta.Accessor(obj)
288-
if err != nil {
289-
return nil, err
290-
}
291-
if acc.GetNamespace() != ns {
292-
continue
293-
}
294-
if acc.GetName() != name {
295-
continue
296-
}
297-
matchingObjs = append(matchingObjs, obj)
298-
}
299-
if len(matchingObjs) == 0 {
286+
matchingObj, ok := objs[types.NamespacedName{Namespace: ns, Name: name}]
287+
if !ok {
300288
return nil, errNotFound
301289
}
302-
if len(matchingObjs) > 1 {
303-
return nil, fmt.Errorf("more than one object matched gvr %s, ns: %q name: %q", gvr, ns, name)
304-
}
305290

306291
// Only one object should match in the tracker if it works
307292
// correctly, as Add/Update methods enforce kind/namespace/name
308293
// uniqueness.
309-
obj := matchingObjs[0].DeepCopyObject()
294+
obj := matchingObj.DeepCopyObject()
310295
if status, ok := obj.(*metav1.Status); ok {
311296
if status.Status != metav1.StatusSuccess {
312297
return nil, &errors.StatusError{ErrStatus: *status}
@@ -405,29 +390,29 @@ func (t *tracker) add(gvr schema.GroupVersionResource, obj runtime.Object, ns st
405390
return errors.NewBadRequest(msg)
406391
}
407392

408-
for i, existingObj := range t.objects[gvr] {
409-
oldMeta, err := meta.Accessor(existingObj)
410-
if err != nil {
411-
return err
412-
}
413-
if oldMeta.GetNamespace() == newMeta.GetNamespace() && oldMeta.GetName() == newMeta.GetName() {
414-
if replaceExisting {
415-
for _, w := range t.getWatches(gvr, ns) {
416-
w.Modify(obj)
417-
}
418-
t.objects[gvr][i] = obj
419-
return nil
393+
_, ok := t.objects[gvr]
394+
if !ok {
395+
t.objects[gvr] = make(map[types.NamespacedName]runtime.Object)
396+
}
397+
398+
namespacedName := types.NamespacedName{Namespace: newMeta.GetNamespace(), Name: newMeta.GetName()}
399+
if _, ok = t.objects[gvr][namespacedName]; ok {
400+
if replaceExisting {
401+
for _, w := range t.getWatches(gvr, ns) {
402+
w.Modify(obj)
420403
}
421-
return errors.NewAlreadyExists(gr, newMeta.GetName())
404+
t.objects[gvr][namespacedName] = obj
405+
return nil
422406
}
407+
return errors.NewAlreadyExists(gr, newMeta.GetName())
423408
}
424409

425410
if replaceExisting {
426411
// Tried to update but no matching object was found.
427412
return errors.NewNotFound(gr, newMeta.GetName())
428413
}
429414

430-
t.objects[gvr] = append(t.objects[gvr], obj)
415+
t.objects[gvr][namespacedName] = obj
431416

432417
for _, w := range t.getWatches(gvr, ns) {
433418
w.Add(obj)
@@ -457,35 +442,28 @@ func (t *tracker) Delete(gvr schema.GroupVersionResource, ns, name string) error
457442
t.lock.Lock()
458443
defer t.lock.Unlock()
459444

460-
found := false
461-
462-
for i, existingObj := range t.objects[gvr] {
463-
objMeta, err := meta.Accessor(existingObj)
464-
if err != nil {
465-
return err
466-
}
467-
if objMeta.GetNamespace() == ns && objMeta.GetName() == name {
468-
obj := t.objects[gvr][i]
469-
t.objects[gvr] = append(t.objects[gvr][:i], t.objects[gvr][i+1:]...)
470-
for _, w := range t.getWatches(gvr, ns) {
471-
w.Delete(obj)
472-
}
473-
found = true
474-
break
475-
}
445+
objs, ok := t.objects[gvr]
446+
if !ok {
447+
return errors.NewNotFound(gvr.GroupResource(), name)
476448
}
477449

478-
if found {
479-
return nil
450+
namespacedName := types.NamespacedName{Namespace: ns, Name: name}
451+
obj, ok := objs[namespacedName]
452+
if !ok {
453+
return errors.NewNotFound(gvr.GroupResource(), name)
480454
}
481455

482-
return errors.NewNotFound(gvr.GroupResource(), name)
456+
delete(objs, namespacedName)
457+
for _, w := range t.getWatches(gvr, ns) {
458+
w.Delete(obj)
459+
}
460+
return nil
483461
}
484462

485463
// filterByNamespace returns all objects in the collection that
486464
// match provided namespace. Empty namespace matches
487465
// non-namespaced objects.
488-
func filterByNamespace(objs []runtime.Object, ns string) ([]runtime.Object, error) {
466+
func filterByNamespace(objs map[types.NamespacedName]runtime.Object, ns string) ([]runtime.Object, error) {
489467
var res []runtime.Object
490468

491469
for _, obj := range objs {
@@ -499,6 +477,15 @@ func filterByNamespace(objs []runtime.Object, ns string) ([]runtime.Object, erro
499477
res = append(res, obj)
500478
}
501479

480+
// Sort res to get deterministic order.
481+
sort.Slice(res, func(i, j int) bool {
482+
acc1, _ := meta.Accessor(res[i])
483+
acc2, _ := meta.Accessor(res[j])
484+
if acc1.GetNamespace() != acc2.GetNamespace() {
485+
return acc1.GetNamespace() < acc2.GetNamespace()
486+
}
487+
return acc1.GetName() < acc2.GetName()
488+
})
502489
return res, nil
503490
}
504491

0 commit comments

Comments
 (0)