From 9f6b5ae7a9e1eab4518bef8e9bdc91ac755d6431 Mon Sep 17 00:00:00 2001 From: Keerthan Ekbote Date: Wed, 10 Apr 2024 13:19:30 -0400 Subject: [PATCH] use slices as backing data structure for v2_sets (#540) * update * remove compare resource ids * add iter * fix iter * add iter impl * add inefficient list as cop out * add filters to inefficient list * fix inefficient list * fix inefficient list * inefficient list inverted * remove inefficient list * add length method to set/ * update * update * update v2 sets with better funciton names * update v2 sets with better funciton names * remove FilterOutAndIterate method * add Get * add comments * add changelog and remove test focus * move changelog/ * move changelog * fixes for PR review --- changelog/v0.38.0/sets-v2-iter.yaml | 8 + contrib/pkg/sets/v2/sets.go | 324 +++++++++++++++++----------- contrib/tests/set_v2_test.go | 252 +++++++++++++++------- pkg/ezkube/creationtimestamp.go | 15 ++ pkg/ezkube/resource_id.go | 36 ++++ 5 files changed, 431 insertions(+), 204 deletions(-) create mode 100644 changelog/v0.38.0/sets-v2-iter.yaml create mode 100644 pkg/ezkube/creationtimestamp.go diff --git a/changelog/v0.38.0/sets-v2-iter.yaml b/changelog/v0.38.0/sets-v2-iter.yaml new file mode 100644 index 000000000..2cf272319 --- /dev/null +++ b/changelog/v0.38.0/sets-v2-iter.yaml @@ -0,0 +1,8 @@ +changelog: + - type: BREAKING_CHANGE + issueLink: https://github.com/solo-io/skv2/issues/543 + description: > + v2 sets have been refactored to use a slice as the backing data structure, allowing for faster GC time of entire sets, + and more efficient iteration over the set. A few methods have been removed, namely `List` and `UnsortedList`, both which have been + replaced with a more accurate name: "FilterOutAndCreateList". In addition, a `Filter` and `Iter` method have been added to v2 sets. + skipCI: "false" diff --git a/contrib/pkg/sets/v2/sets.go b/contrib/pkg/sets/v2/sets.go index a88dac62c..b86ad6319 100644 --- a/contrib/pkg/sets/v2/sets.go +++ b/contrib/pkg/sets/v2/sets.go @@ -1,46 +1,70 @@ package sets_v2 import ( + "slices" "sync" sk_sets "github.com/solo-io/skv2/contrib/pkg/sets" - "github.com/solo-io/skv2/pkg/controllerutils" "github.com/solo-io/skv2/pkg/ezkube" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" ) +// ResourceSet is a thread-safe container for a set of resources. +// It provides a set of operations for working with the set of resources, typically used for managing Kubernetes resources. +// The ResourceSet is a generic interface that can be used with any type that satisfies the client.Object interface. type ResourceSet[T client.Object] interface { // Get the set stored keys - Keys() sets.String - // List of resources stored in the set. Pass an optional filter function to filter on the list. - List(filterResource ...func(T) bool) []T - // Unsorted list of resources stored in the set. Pass an optional filter function to filter on the list. - UnsortedList(filterResource ...func(T) bool) []T + Keys() sets.Set[string] + + // Filter returns an iterator that will iterate over the set of elements + // that match the provided filter. If the filter returns true, the resource will be included in the iteration. + // The index and resource are passed to the provided function for every element in the *filtered set*. + // The index is the index of the resource in the *filtered* set. + // The iteration can be stopped by returning false from the function. This can be thought of as a "break" statement in a loop. + // Returning true will continue the iteration. This can be thought of as a "continue" statement in a loop. + // For iteration that does not need to be filtered, use Iter. + Filter(filterResource func(T) bool) func(yield func(int, T) bool) + + // Iter iterates over the set, passing the index and resource to the provided function for every element in the set. + // The iteration can be stopped by returning false from the function. This can be thought of as a "break" statement in a loop. + // Returning true will continue the iteration. This can be thought of as a "continue" statement in a loop. + Iter(func(int, T) bool) + + // FilterOutAndCreateList constructs a list of resource that do not match any of the provided filters. + // Use of this function should be limited to only when a filtered list is needed. + // For iteration that does not require creating a new list, use Iter. + // For iteration that requires typical filtering semantics (i.e. filters that return true for resources that should be included), + // use + FilterOutAndCreateList(filterResource ...func(T) bool) []T // Return the Set as a map of key to resource. Map() map[string]T // Insert a resource into the set. Insert(resource ...T) + // Compare the equality of the keys in two sets (not the resources themselves) Equal(set ResourceSet[T]) bool - // Check if the set contains a key matching the resource (not the resource itself) + // Check if the set contains the resource. Has(resource T) bool - // Delete the key matching the resource - Delete(resource T) + // Delete the matching resource. + Delete(resource ezkube.ResourceId) // Return the union with the provided set Union(set ResourceSet[T]) ResourceSet[T] // Return the difference with the provided set Difference(set ResourceSet[T]) ResourceSet[T] // Return the intersection with the provided set Intersection(set ResourceSet[T]) ResourceSet[T] - // Find the resource with the given ID - Find(id ezkube.ResourceId) (T, error) + // Find the resource with the given ID. + // Returns a NotFoundErr error if the resource is not found. + Find(resource ezkube.ResourceId) (T, error) + // Find the resource with the given ID. + // Returns nil if the resource is not found. + Get(resource ezkube.ResourceId) T // Get the length of the set + Len() int Length() int // returns the generic implementation of the set Generic() sk_sets.ResourceSet - // returns the delta between this and and another ResourceSet[T] - Delta(newSet ResourceSet[T]) sk_sets.ResourceDelta // Clone returns a deep copy of the set Clone() ResourceSet[T] } @@ -61,93 +85,123 @@ func (r *ResourceDelta[T]) DeltaV1() sk_sets.ResourceDelta { } type resourceSet[T client.Object] struct { - lock sync.RWMutex - set sets.String - mapping map[string]T + lock sync.RWMutex + set []T } -func NewResourceSet[T client.Object](resources ...T) ResourceSet[T] { - set := sets.NewString() - mapping := map[string]T{} - for _, resource := range resources { - key := sk_sets.Key(resource) - set.Insert(key) - mapping[key] = resource +func NewResourceSet[T client.Object]( + resources ...T, +) ResourceSet[T] { + rs := &resourceSet[T]{ + set: []T{}, } - return &resourceSet[T]{set: set, mapping: mapping} + rs.Insert(resources...) + return rs } -func (s *resourceSet[T]) Keys() sets.String { - return sets.NewString(s.set.List()...) +func (s *resourceSet[T]) Keys() sets.Set[string] { + s.lock.RLock() + defer s.lock.RUnlock() + keys := sets.Set[string]{} + for _, resource := range s.set { + keys.Insert(sk_sets.Key(resource)) + } + return sets.Set[string]{} } -func (s *resourceSet[T]) List(filterResource ...func(T) bool) []T { +// Filter returns an iterator that will iterate over the set of elements +// that match the provided filter. If the filter returns true, the resource will be included in the iteration. +func (s *resourceSet[T]) Filter(filterResource func(T) bool) func(yield func(int, T) bool) { s.lock.RLock() defer s.lock.RUnlock() - var resources []T - for _, key := range s.set.List() { - var filtered bool - for _, filter := range filterResource { - if filter(s.mapping[key]) { - filtered = true - break + return func(yield func(int, T) bool) { + i := 0 + for _, resource := range s.set { + if filterResource(resource) { + if !yield(i, resource) { + break + } + i += 1 } } - if !filtered { - resources = append(resources, s.mapping[key]) - } } - return resources } -func (s *resourceSet[T]) UnsortedList(filterResource ...func(T) bool) []T { +// This is a convenience function that constructs a list of resource that do not match any of the provided filters. +// Use of this function should be limited to only when a filtered list is needed, as this allocates a new list. +func (s *resourceSet[T]) FilterOutAndCreateList(filterResource ...func(T) bool) []T { s.lock.RLock() defer s.lock.RUnlock() - - keys := s.set.UnsortedList() - resources := make([]T, 0, len(keys)) - - for _, key := range keys { - var filtered bool + var ret []T + for _, resource := range s.set { + filtered := false for _, filter := range filterResource { - if filter(s.mapping[key]) { + if filter(resource) { filtered = true break } } if !filtered { - resources = append(resources, s.mapping[key]) + ret = append(ret, resource) + } + } + return ret +} + +// Iter iterates over the set, passing the index and resource to the provided function for every element in the set. +// The iteration can be stopped by returning false from the function. This can be thought of as a "break" statement in a loop. +// Returning true will continue the iteration. This can be thought of as a "continue" statement in a loop. +func (s *resourceSet[T]) Iter(yield func(int, T) bool) { + s.lock.RLock() + defer s.lock.RUnlock() + for i, resource := range s.set { + if !yield(i, resource) { + break } } - return resources } func (s *resourceSet[T]) Map() map[string]T { s.lock.RLock() defer s.lock.RUnlock() newMap := map[string]T{} - for k, v := range s.mapping { - newMap[k] = v + for _, resource := range s.set { + newMap[sk_sets.Key(resource)] = resource } return newMap } -func (s *resourceSet[T]) Insert( - resources ...T, -) { - s.lock.Lock() - defer s.lock.Unlock() +// Insert adds items to the set in inserted order. +// If an item is already in the set, it is overwritten. +// The set is sorted based on the ResourceId of the resources. +func (s *resourceSet[T]) Insert(resources ...T) { + s.lock.RLock() + defer s.lock.RUnlock() for _, resource := range resources { - key := sk_sets.Key(resource) - s.mapping[key] = resource - s.set.Insert(key) + insertIndex, found := slices.BinarySearchFunc( + s.set, + resource, + func(a, b T) int { return ezkube.ResourceIdsCompare(a, b) }, + ) + if found { + s.set[insertIndex] = resource + continue + } + // insert the resource at the determined index + s.set = slices.Insert(s.set, insertIndex, resource) } } +// Uses binary search to check if the set contains the resource. func (s *resourceSet[T]) Has(resource T) bool { s.lock.RLock() defer s.lock.RUnlock() - return s.set.Has(sk_sets.Key(resource)) + _, found := slices.BinarySearchFunc( + s.set, + resource, + func(a, b T) int { return ezkube.ResourceIdsCompare(a, b) }, + ) + return found } func (s *resourceSet[T]) Equal( @@ -155,106 +209,125 @@ func (s *resourceSet[T]) Equal( ) bool { s.lock.RLock() defer s.lock.RUnlock() - return s.set.Equal(set.Keys()) + return s.Generic().Equal(set.Generic()) } -func (s *resourceSet[T]) Delete(resource T) { +func (s *resourceSet[T]) Delete(resource ezkube.ResourceId) { s.lock.Lock() defer s.lock.Unlock() - key := sk_sets.Key(resource) - delete(s.mapping, key) - s.set.Delete(key) + + i, found := slices.BinarySearchFunc( + s.set, + resource, + func(a T, b ezkube.ResourceId) int { return ezkube.ResourceIdsCompare(a, b) }, + ) + if found { + s.set = slices.Delete(s.set, i, i+1) + } } func (s *resourceSet[T]) Union(set ResourceSet[T]) ResourceSet[T] { - return NewResourceSet[T](append(s.List(), set.List()...)...) + list := []T{} + for _, resource := range s.Generic().Union(set.Generic()).List() { + list = append(list, resource.(T)) + } + return NewResourceSet[T]( + list..., + ) } func (s *resourceSet[T]) Difference(set ResourceSet[T]) ResourceSet[T] { s.lock.RLock() defer s.lock.RUnlock() - newSet := s.set.Difference(set.Keys()) - var newResources []T - for key, _ := range newSet { - val, _ := s.mapping[key] - newResources = append(newResources, val) + result := NewResourceSet[T]() + for _, resource := range s.set { + if !set.Has(resource) { + result.Insert(resource) + } } - return NewResourceSet[T](newResources...) + return result } func (s *resourceSet[T]) Intersection(set ResourceSet[T]) ResourceSet[T] { s.lock.RLock() defer s.lock.RUnlock() - newSet := s.set.Intersection(set.Keys()) - var newResources []T - for key, _ := range newSet { - val, _ := s.mapping[key] - newResources = append(newResources, val) + var walk, other ResourceSet[T] + result := NewResourceSet[T]() + if len(s.set) < set.Len() { + walk = NewResourceSet(s.set...) + other = set + } else { + walk = set + other = NewResourceSet(s.set...) } - return NewResourceSet[T](newResources...) + walk.Iter(func(_ int, key T) bool { + if other.Has(key) { + result.Insert(key) + } + return true + }) + return result } -func (s *resourceSet[T]) Find( - id ezkube.ResourceId, -) (T, error) { +// Get the resource with the given ID. +// Returns nil if the resource is not found. +// Uses binary search to find the resource. +func (s *resourceSet[T]) Get(resource ezkube.ResourceId) T { s.lock.RLock() defer s.lock.RUnlock() - key := sk_sets.Key(id) - resource, ok := s.mapping[key] - if !ok { - return resource, sk_sets.NotFoundErr(resource, id) - } - return resource, nil + insertIndex, found := slices.BinarySearchFunc( + s.set, + resource, + func(a T, b ezkube.ResourceId) int { return ezkube.ResourceIdsCompare(a, b) }, + ) + if found { + return s.set[insertIndex] + } + var r T + return r } -func (s *resourceSet[T]) Length() int { - +// Find the resource with the given ID. +// Returns a NotFoundErr error if the resource is not found. +// Uses binary search to find the resource. +// This function is useful when you need to distinguish between a resource not found and a resource that is found but is nil, +// but it is less efficient than Get as it allocates an error object. +func (s *resourceSet[T]) Find(resource ezkube.ResourceId) (T, error) { s.lock.RLock() defer s.lock.RUnlock() - return len(s.mapping) + + insertIndex, found := slices.BinarySearchFunc( + s.set, + resource, + func(a T, b ezkube.ResourceId) int { return ezkube.ResourceIdsCompare(a, b) }, + ) + if found { + return s.set[insertIndex], nil + } + + var r T + return r, sk_sets.NotFoundErr(r, resource) } -// note that this function will currently panic if called for a ResourceSet[T] containing non-runtime.Objects -func (oldSet *resourceSet[T]) Delta(newSet ResourceSet[T]) sk_sets.ResourceDelta { - updated, removed := NewResourceSet[T](), NewResourceSet[T]() - - // find objects updated or removed - oldSet.List(func(oldObj T) bool { - newObj, err := newSet.Find(oldObj) - switch { - case err != nil: - // obj removed - removed.Insert(oldObj) - case !controllerutils.ObjectsEqual(oldObj, newObj): - // obj updated - updated.Insert(newObj) - default: - // obj the same - } - return true // return value ignored - }) +func (s *resourceSet[T]) Len() int { + s.lock.RLock() + defer s.lock.RUnlock() + return len(s.set) +} - // find objects added - newSet.List(func(newObj T) bool { - if _, err := oldSet.Find(newObj); err != nil { - // obj added - updated.Insert(newObj) - } - return true // return value ignored - }) - delta := &ResourceDelta[T]{ - Inserted: updated, - Removed: removed, - } - return delta.DeltaV1() +func (s *resourceSet[T]) Length() int { + s.lock.RLock() + defer s.lock.RUnlock() + return len(s.set) } // Create a clone of the current set // note that this function will currently panic if called for a ResourceSet[T] containing non-runtime.Objects func (oldSet *resourceSet[T]) Clone() ResourceSet[T] { new := NewResourceSet[T]() - oldSet.List(func(oldObj T) bool { + + oldSet.Iter(func(_ int, oldObj T) bool { copy := oldObj.DeepCopyObject().(T) new.Insert(copy) return true @@ -263,9 +336,10 @@ func (oldSet *resourceSet[T]) Clone() ResourceSet[T] { } func (s *resourceSet[T]) Generic() sk_sets.ResourceSet { - set := sk_sets.NewResourceSet() - for _, v := range s.List() { + set := sk_sets.NewResourceSet(nil) + s.Iter(func(_ int, v T) bool { set.Insert(v) - } + return true + }) return set } diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index 752494f8a..7c700e555 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -6,13 +6,14 @@ import ( v1 "github.com/solo-io/skv2/codegen/test/api/things.test.io/v1" "github.com/solo-io/skv2/contrib/pkg/sets" sets_v2 "github.com/solo-io/skv2/contrib/pkg/sets/v2" + "github.com/solo-io/skv2/pkg/ezkube" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var _ = Describe("PaintSetV2", func() { var ( - setA, setB sets_v2.ResourceSet[*v1.Paint] - paintA, paintB, paintC *v1.Paint + setA, setB sets_v2.ResourceSet[*v1.Paint] + paintA, paintBCluster2, paintC *v1.Paint ) BeforeEach(func() { @@ -21,7 +22,7 @@ var _ = Describe("PaintSetV2", func() { paintA = &v1.Paint{ ObjectMeta: metav1.ObjectMeta{Name: "nameA", Namespace: "nsA"}, } - paintB = &v1.Paint{ + paintBCluster2 = &v1.Paint{ ObjectMeta: metav1.ObjectMeta{Name: "nameB", Namespace: "nsB"}, } paintC = &v1.Paint{ @@ -31,157 +32,250 @@ var _ = Describe("PaintSetV2", func() { It("should insert", func() { setA.Insert(paintA) - list := setA.List() - Expect(list).To(ConsistOf(paintA)) - setA.Insert(paintB, paintC) - list = setA.List() - Expect(list).To(ConsistOf(paintA, paintB, paintC)) + Expect(setA.Has(paintA)).To(BeTrue()) + setA.Insert(paintBCluster2, paintC) + Expect(setA.Has(paintBCluster2)).To(BeTrue()) + Expect(setA.Has(paintC)).To(BeTrue()) + 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()) - Expect(setA.Has(paintB)).To(BeFalse()) - setA.Insert(paintB, paintC) + Expect(setA.Has(paintBCluster2)).To(BeFalse()) + setA.Insert(paintBCluster2, paintC) Expect(setA.Has(paintA)).To(BeTrue()) - Expect(setA.Has(paintB)).To(BeTrue()) + Expect(setA.Has(paintBCluster2)).To(BeTrue()) Expect(setA.Has(paintC)).To(BeTrue()) }) It("should return set equality", func() { - setB.Insert(paintA, paintB, paintC) + setB.Insert(paintA, paintBCluster2, paintC) setA.Insert(paintA) Expect(setA.Equal(setB)).To(BeFalse()) - setA.Insert(paintC, paintB) + setA.Insert(paintC, paintBCluster2) Expect(setA.Equal(setB)).To(BeTrue()) }) It("should delete", func() { - setA.Insert(paintA, paintB, paintC) + 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()) }) - It("should filter UnsortedList", func() { - setA.Insert(paintA, paintB, paintC) + It("should filter out", func() { + setA.Insert(paintA, paintBCluster2, paintC) Expect(setA.Has(paintA)).To(BeTrue()) - filtered := setA.UnsortedList(func(p *v1.Paint) bool { - return p.Name == "nameA" - }) - Expect(filtered).To(HaveLen(2)) - Expect(filtered).To(ContainElements(paintB, paintC)) - }) + Expect(setA.Len()).To(Equal(3)) + + filterFn := func(p *v1.Paint) bool { return p.GetName() == "nameA" } + + for i, filtered := range setA.FilterOutAndCreateList(filterFn) { + if i == 0 { + Expect(filtered).To(Equal(paintBCluster2)) + } + if i == 1 { + Expect(filtered).To(Equal(paintC)) + } + } - It("should double filter UnsortedList", func() { - setA.Insert(paintA, paintB, paintC) - Expect(setA.Has(paintA)).To(BeTrue()) - filtered := setA.UnsortedList(func(p *v1.Paint) bool { - return p.Name == "nameA" - }, func(p *v1.Paint) bool { - return p.Name == "nameB" - }) - Expect(filtered).To(HaveLen(1)) - Expect(filtered).To(ContainElement(paintC)) }) - It("should filter List", func() { - setA.Insert(paintA, paintB, paintC) + + It("should filter", func() { + setA.Insert(paintA, paintBCluster2, paintC) Expect(setA.Has(paintA)).To(BeTrue()) - filtered := setA.List(func(p *v1.Paint) bool { - return p.Name == "nameA" + Expect(setA.Len()).To(Equal(3)) + + filterFn := func(p *v1.Paint) bool { return p.GetName() == "nameA" } + + setA.Filter(filterFn)(func(i int, p *v1.Paint) bool { + if i == 0 { + Expect(p).To(Equal(paintA)) + } + if i != 0 { + Fail("only one item should be in the filtered set") + } + return true }) - Expect(filtered).To(HaveLen(2)) - Expect(filtered).To(ContainElements(paintB, paintC)) }) It("should double filter List", func() { - setA.Insert(paintA, paintB, paintC) + setA.Insert(paintA, paintBCluster2, paintC) Expect(setA.Has(paintA)).To(BeTrue()) - filtered := setA.List(func(p *v1.Paint) bool { + for _, filtered := range setA.FilterOutAndCreateList(func(p *v1.Paint) bool { return p.Name == "nameA" }, func(p *v1.Paint) bool { return p.Name == "nameB" - }) - Expect(filtered).To(HaveLen(1)) - Expect(filtered).To(ContainElement(paintC)) + }) { + Expect(filtered).To(Equal(paintC)) + } }) It("should union two sets and return new set", func() { - setA.Insert(paintA, paintB) - setB.Insert(paintA, paintB, paintC) + setA.Insert(paintA, paintBCluster2) + setB.Insert(paintA, paintBCluster2, paintC) unionSet := setA.Union(setB) - Expect(unionSet.List()).To(ConsistOf(paintA, paintB, paintC)) + Expect(unionSet.Len()).To(Equal(3)) + Expect(unionSet.Has(paintA)).To(BeTrue()) + Expect(unionSet.Has(paintBCluster2)).To(BeTrue()) + Expect(unionSet.Has(paintC)).To(BeTrue()) Expect(unionSet).ToNot(BeIdenticalTo(setA)) Expect(unionSet).ToNot(BeIdenticalTo(setB)) }) It("should take the difference of two sets and return new set", func() { - setA.Insert(paintA, paintB) - setB.Insert(paintA, paintB, paintC) + setA.Insert(paintA, paintBCluster2) + setB.Insert(paintA, paintBCluster2, paintC) differenceA := setA.Difference(setB) - Expect(differenceA.List()).To(BeEmpty()) + Expect(differenceA.Len()).To(Equal(0)) Expect(differenceA.Map()).To(BeEmpty()) Expect(differenceA).ToNot(BeIdenticalTo(setA)) differenceB := setB.Difference(setA) - Expect(differenceB.List()).To(ConsistOf(paintC)) + Expect(differenceB.Has(paintC)).To(BeTrue()) Expect(differenceB.Map()).To(HaveKeyWithValue(sets.Key(paintC), paintC)) Expect(differenceB).ToNot(BeIdenticalTo(setB)) }) It("should take the intersection of two sets and return new set", func() { - setA.Insert(paintA, paintB) - setB.Insert(paintA, paintB, paintC) + setA.Insert(paintA, paintBCluster2) + setB.Insert(paintA, paintBCluster2, paintC) intersectionA := setA.Intersection(setB) - Expect(intersectionA.List()).To(ConsistOf(paintA, paintB)) + Expect(intersectionA.Has(paintA)).To(BeTrue()) + Expect(intersectionA.Has(paintBCluster2)).To(BeTrue()) + Expect(intersectionA.Len()).To(Equal(2)) Expect(intersectionA.Map()).To(HaveKeyWithValue(sets.Key(paintA), paintA)) - Expect(intersectionA.Map()).To(HaveKeyWithValue(sets.Key(paintB), paintB)) + Expect(intersectionA.Map()).To(HaveKeyWithValue(sets.Key(paintBCluster2), paintBCluster2)) Expect(intersectionA).ToNot(BeIdenticalTo(setA)) }) It("should correctly match two sets", func() { - setA.Insert(paintA, paintB) - setB.Insert(paintA, paintB) - Expect(setA).To(Equal(setB)) + setA.Insert(paintA, paintBCluster2) + setB.Insert(paintA, paintBCluster2) + Expect(setA.Equal(setB)).To(BeTrue()) setB.Insert(paintC) - Expect(setA).ToNot(Equal(setB)) + Expect(setA.Equal(setB)).To(BeFalse()) }) It("should return corrent length", func() { - setA.Insert(paintA, paintB) - Expect(setA.Length()).To(Equal(2)) + setA.Insert(paintA, paintBCluster2) + Expect(setA.Len()).To(Equal(2)) }) - It("should return set deltas", func() { - // update + It("should find resources", func() { oldPaintA := &v1.Paint{ - ObjectMeta: metav1.ObjectMeta{Name: "background", Namespace: "color"}, - Spec: v1.PaintSpec{Color: &v1.PaintColor{Hue: "orange"}}, + ObjectMeta: metav1.ObjectMeta{ + Name: "background", + Namespace: "color", + Annotations: map[string]string{ezkube.ClusterAnnotation: "orange"}, + }, } newPaintA := &v1.Paint{ - ObjectMeta: metav1.ObjectMeta{Name: "background", Namespace: "color"}, - Spec: v1.PaintSpec{Color: &v1.PaintColor{Hue: "green"}}, + ObjectMeta: metav1.ObjectMeta{ + Name: "background", + Namespace: "color", + 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"}, } - // no change - oldPaintD := &v1.Paint{ - ObjectMeta: metav1.ObjectMeta{Name: "decent", Namespace: "color"}, + paintList := []*v1.Paint{oldPaintA, newPaintA, oldPaintB, newPaintC, newPaintC} + expectedPaintList := []*v1.Paint{newPaintC, oldPaintB, newPaintA} + setA.Insert(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)) + } + // find based on something that implemented ClusterObjectRef + found, err := setA.Find(ezkube.MakeClusterObjectRef(newPaintC)) + Expect(Expect(err).NotTo(HaveOccurred())) + Expect(found).To(Equal(newPaintC)) + }) + + 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: "1"}, + }, + } + paintAB1 := &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "b", + Annotations: map[string]string{ezkube.ClusterAnnotation: "1"}, + }, } - newPaintD := &v1.Paint{ - ObjectMeta: metav1.ObjectMeta{Name: "decent", Namespace: "color"}, + paintBB1 := &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{ + Name: "b", + Namespace: "b", + Annotations: map[string]string{ezkube.ClusterAnnotation: "1"}, + }, + } + paintAC2 := &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{ + Name: "a", + Namespace: "c", + Annotations: map[string]string{ezkube.ClusterAnnotation: "2"}, + }, + } + paintBC2 := &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{ + Name: "b", + Namespace: "c", + Annotations: map[string]string{ezkube.ClusterAnnotation: "2"}, + }, + } + 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 + var paintList2 []*v1.Paint + for _, paint := range setA.FilterOutAndCreateList() { + paintList = append(paintList, paint) + } + + setA.Iter(func(_ int, paint *v1.Paint) bool { + paintList2 = append(paintList2, paint) + return true + }) + + for i, paint := range expectedOrder { + Expect(paintList[i]).To(Equal(paint)) + Expect(paintList2[i]).To(Equal(paint)) } - setA.Insert(oldPaintA, oldPaintB, oldPaintD) - setB.Insert(newPaintA, newPaintC, newPaintD) - Expect(setA.Delta(setB)).To(Equal(sets.ResourceDelta{ - Inserted: sets.NewResourceSet(newPaintA, newPaintC), - Removed: sets.NewResourceSet(oldPaintB), - })) }) }) diff --git a/pkg/ezkube/creationtimestamp.go b/pkg/ezkube/creationtimestamp.go new file mode 100644 index 000000000..2723c6a65 --- /dev/null +++ b/pkg/ezkube/creationtimestamp.go @@ -0,0 +1,15 @@ +package ezkube + +import "sigs.k8s.io/controller-runtime/pkg/client" + +// CreationTimestampCompare returns an integer comparing two resources lexicographically. +// The result will be 0 if a == b, -1 if a < b, and +1 if a > b. +func CreationTimestampCompare(a, b client.Object) int { + if a.GetCreationTimestamp().Time.Equal(b.GetCreationTimestamp().Time) { + return 0 + } + if a.GetCreationTimestamp().Time.Before(b.GetCreationTimestamp().Time) { + return -1 + } + return 1 +} diff --git a/pkg/ezkube/resource_id.go b/pkg/ezkube/resource_id.go index b070e006e..3b1d8207d 100644 --- a/pkg/ezkube/resource_id.go +++ b/pkg/ezkube/resource_id.go @@ -173,3 +173,39 @@ func ResourceIdFromKeyWithSeparator(key string, separator string) (ResourceId, e return nil, eris.Errorf("could not convert key %s with separator %s into resource id; unexpected number of parts: %d", key, separator, len(parts)) } } + +// ResourceIdsCompare returns an integer comparing two ResourceIds lexicographically. +// The comparison is based on name, then namespace, then cluster name. +// The result will be 0 if a == b, -1 if a < b, and +1 if a > b. +func ResourceIdsCompare(a, b ResourceId) int { + // compare names + if cmp := strings.Compare(a.GetName(), b.GetName()); cmp != 0 { + return cmp + } + + // compare namespaces + if cmp := strings.Compare(a.GetNamespace(), b.GetNamespace()); cmp != 0 { + return cmp + } + + // compare cluster names + // attempt to cast to ClusterResourceId + // if fails, attempt cast to deprecatedClusterResourceId since we might be working with a ClusterObjectRef + var ( + aCluster, bCluster string + ) + + if a_cri, ok := a.(ClusterResourceId); ok { + aCluster = GetClusterName(a_cri) + } else { + aCluster = getDeprecatedClusterName(a) + } + + if b_cri, ok := b.(ClusterResourceId); ok { + bCluster = GetClusterName(b_cri) + } else { + bCluster = getDeprecatedClusterName(b) + } + + return strings.Compare(aCluster, bCluster) +}