Skip to content

Commit

Permalink
base comparison on key generated from name, ns, cluster; use slices.B…
Browse files Browse the repository at this point in the history 
…inarySearchFunc over sort.Search; update v2 sets unit tests
  • Loading branch information
@conradhanson
conradhanson committed Mar 27, 2024
1 parent 2e6dfb4 commit 3b95a7b
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 73 deletions.
63 changes: 33 additions & 30 deletions contrib/pkg/sets/v2/sets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package sets_v2
import (
"iter"
"slices"
"sort"
"sync"

sk_sets "github.com/solo-io/skv2/contrib/pkg/sets"
Expand Down Expand Up @@ -46,7 +45,7 @@ type ResourceSet[T client.Object] interface {
// Clone returns a deep copy of the set
Clone() ResourceSet[T]
// Get the compare function used by the set
GetCompareFunc() func(a, b client.Object) int
GetCompareFunc() func(a, b ezkube.ResourceId) int
}

// ResourceDelta represents the set of changes between two ResourceSets.
Expand All @@ -67,11 +66,11 @@ func (r *ResourceDelta[T]) DeltaV1() sk_sets.ResourceDelta {
type resourceSet[T client.Object] struct {
lock sync.RWMutex
set []T
compareFunc func(a, b client.Object) int
compareFunc func(a, b ezkube.ResourceId) int
}

func NewResourceSet[T client.Object](
compareFunc func(a, b client.Object) int,
compareFunc func(a, b ezkube.ResourceId) int,
resources ...T,
) ResourceSet[T] {
rs := &resourceSet[T]{
Expand Down Expand Up @@ -127,14 +126,15 @@ func (s *resourceSet[T]) Insert(resources ...T) {
s.lock.RLock()
defer s.lock.RUnlock()
for _, resource := range resources {
insertIndex := sort.Search(len(s.set), func(i int) bool { return s.compareFunc(resource, s.set[i]) < 0 })

// if the resource is already in the set, replace it
if insertIndex < len(s.set) && s.compareFunc(resource, s.set[insertIndex]) == 0 {
insertIndex, found := slices.BinarySearchFunc(
s.set,
resource,
func(a, b T) int { return s.compareFunc(a, b) },
)
if found {
s.set[insertIndex] = resource
return
continue
}

// insert the resource at the determined index
s.set = slices.Insert(s.set, insertIndex, resource)
}
Expand All @@ -143,10 +143,12 @@ func (s *resourceSet[T]) Insert(resources ...T) {
func (s *resourceSet[T]) Has(resource T) bool {
s.lock.RLock()
defer s.lock.RUnlock()
i := sort.Search(len(s.set), func(i int) bool {
return s.compareFunc(s.set[i], resource) >= 0
})
return i < len(s.set) && s.compareFunc(s.set[i], resource) == 0
_, found := slices.BinarySearchFunc(
s.set,
resource,
func(a, b T) int { return s.compareFunc(a, b) },
)
return found
}

func (s *resourceSet[T]) Equal(
Expand All @@ -161,11 +163,11 @@ func (s *resourceSet[T]) Delete(resource ezkube.ResourceId) {
s.lock.Lock()
defer s.lock.Unlock()

desired_key := sk_sets.Key(resource)
i := sort.Search(len(s.set), func(i int) bool {
return sk_sets.Key(s.set[i]) >= desired_key
})
found := i < len(s.set) && sk_sets.Key(s.set[i]) == desired_key
i, found := slices.BinarySearchFunc(
s.set,
resource,
func(a T, b ezkube.ResourceId) int { return s.compareFunc(a, b) },
)
if found {
s.set = slices.Delete(s.set, i, i+1)
}
Expand Down Expand Up @@ -214,20 +216,21 @@ func (s *resourceSet[T]) Intersection(set ResourceSet[T]) ResourceSet[T] {
return result
}

// this is only possible when the compare func is ezkube.ResourceIdsCompare
// otherwise the set won't be sorted to allow for binary search
func (s *resourceSet[T]) Find(resource ezkube.ResourceId) (T, error) {
s.lock.RLock()
defer s.lock.RUnlock()
desired_key := sk_sets.Key(resource)
i := sort.Search(len(s.set), func(i int) bool {
return sk_sets.Key(s.set[i]) >= desired_key
})
var found T
if i < len(s.set) && sk_sets.Key(s.set[i]) == desired_key {
return s.set[i], nil

insertIndex, found := slices.BinarySearchFunc(
s.set,
resource,
func(a T, b ezkube.ResourceId) int { return s.compareFunc(a, b) },
)
if found {
return s.set[insertIndex], nil
}
return found, sk_sets.NotFoundErr(found, resource)

var r T
return r, sk_sets.NotFoundErr(r, resource)
}

func (s *resourceSet[T]) Len() int {
Expand Down Expand Up @@ -293,6 +296,6 @@ func (s *resourceSet[T]) Generic() sk_sets.ResourceSet {
return set
}

func (s *resourceSet[T]) GetCompareFunc() func(a, b client.Object) int {
func (s *resourceSet[T]) GetCompareFunc() func(a, b ezkube.ResourceId) int {
return s.compareFunc
}
56 changes: 38 additions & 18 deletions contrib/tests/set_v2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ var _ = FDescribe("PaintSetV2", func() {
Expect(setA.Len()).To(Equal(3))
})

When("inserting an existing resource", func() {
It("should overwrite the existing resource", func() {
setA.Insert(paintA)
setA.Insert(paintBCluster2)
setA.Insert(paintA)
Expect(setA.Len()).To(Equal(2))
})
})

It("should return set existence", func() {
setA.Insert(paintA)
Expect(setA.Has(paintA)).To(BeTrue())
Expand All @@ -61,6 +70,7 @@ var _ = FDescribe("PaintSetV2", func() {
setA.Insert(paintA, paintBCluster2, paintC)
Expect(setA.Has(paintA)).To(BeTrue())
setA.Delete(paintA)
Expect(setA.Len()).To(Equal(2))
Expect(setA.Has(paintA)).To(BeFalse())
})

Expand Down Expand Up @@ -207,64 +217,74 @@ var _ = FDescribe("PaintSetV2", func() {
Annotations: map[string]string{ezkube.ClusterAnnotation: "orange"},
},
}
// remove
oldPaintB := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{Name: "ugly", Namespace: "color"},
}
// add
newPaintC := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{Name: "beautiful", Namespace: "color"},
}
paintList := []*v1.Paint{oldPaintA, newPaintA, oldPaintB, newPaintC}
paintList := []*v1.Paint{oldPaintA, newPaintA, oldPaintB, newPaintC, newPaintC}
expectedPaintList := []*v1.Paint{newPaintC, oldPaintB, newPaintA}
setA.Insert(paintList...)

for _, paint := range paintList {
Expect(setA.Len()).To(Equal(3))

for _, paint := range expectedPaintList {
found, err := setA.Find(paint)
Expect(Expect(err).NotTo(HaveOccurred()))
Expect(found).To(Equal(paint))
}
})

It("should sort resources first by cluster, then by namespace, then by name", func() {
paintAAcluster1 := &v1.Paint{
It("should sort resources by name.ns.cluster", func() {
paintAA1 := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "a",
Namespace: "a",
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-1"},
Annotations: map[string]string{ezkube.ClusterAnnotation: "1"},
},
}
paintABcluster1 := &v1.Paint{
paintAB1 := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "a",
Namespace: "b",
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-1"},
Annotations: map[string]string{ezkube.ClusterAnnotation: "1"},
},
}
paintBBcluster1 := &v1.Paint{
paintBB1 := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "b",
Namespace: "b",
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-1"},
Annotations: map[string]string{ezkube.ClusterAnnotation: "1"},
},
}
paintACluster2 := &v1.Paint{
paintAC2 := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "a",
Namespace: "c",
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-2"},
Annotations: map[string]string{ezkube.ClusterAnnotation: "2"},
},
}
paintBCluster2 := &v1.Paint{
paintBC2 := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "b",
Namespace: "c",
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-2"},
Annotations: map[string]string{ezkube.ClusterAnnotation: "2"},
},
}
expectedOrder := []*v1.Paint{
paintAAcluster1, paintABcluster1, paintBBcluster1,
paintACluster2, paintBCluster2,
paintAC := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "a",
Namespace: "c",
},
}
paintBC := &v1.Paint{
ObjectMeta: metav1.ObjectMeta{
Name: "b",
Namespace: "c",
},
}
expectedOrder := []*v1.Paint{paintAA1, paintAB1, paintAC, paintAC2, paintBB1, paintBC, paintBC2}
setA.Insert(expectedOrder...)

var paintList []*v1.Paint
Expand Down
31 changes: 6 additions & 25 deletions pkg/ezkube/resource_id.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,35 +174,16 @@ func ResourceIdFromKeyWithSeparator(key string, separator string) (ResourceId, e
}
}

// CompareResourceId returns an integer comparing two ResourceIds lexicographically.
// ResourceIdsCompare returns an integer comparing two ResourceIds lexicographically.
// The result will be 0 if a == b, -1 if a < b, and +1 if a > b.
// a, b must be of type ResourceId
func ResourceIdsCompare(a, b client.Object) int {
if resourceIdsEqual(a, b) {
func ResourceIdsCompare(a, b ResourceId) int {
key_a := KeyWithSeparator(a, ".")
key_b := KeyWithSeparator(b, ".")
if key_a == key_b {
return 0
}
if resourceIdsLessThan(a, b) {
if key_a < key_b {
return -1
}
return 1
}

func resourceIdsEqual(a, b client.Object) bool {
return a.GetName() == b.GetName() && a.GetNamespace() == b.GetNamespace() && GetClusterName(a) == GetClusterName(b)
}

func resourceIdsLessThan(a, b client.Object) bool {
// cluster name is the primary sort key
if GetClusterName(a) > GetClusterName(b) {
return false
}
// namespace is the secondary sort key
if a.GetNamespace() > b.GetNamespace() {
return false
}
// name is the tertiary sort key
if a.GetName() > b.GetName() {
return false
}
return true
}

0 comments on commit 3b95a7b

Please sign in to comment.