Skip to content

Commit 2e6dfb4

Browse files
conradhansonconradhanson
committed
add cluster name checks to compare func; add unit tests to sets v2; update Delete to take ResourceId
1 parent d4fd8ee commit 2e6dfb4

File tree

3 files changed

+99
-94
lines changed

3 files changed

+99
-94
lines changed

‎contrib/pkg/sets/v2/sets.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ type ResourceSet[T client.Object] interface {
2525
Insert(resource ...T)
2626
// Compare the equality of the keys in two sets (not the resources themselves)
2727
Equal(set ResourceSet[T]) bool
28-
// Check if the set contains the resource. Uses the compare func to determine equality.
28+
// Check if the set contains the resource.
2929
Has(resource T) bool
30-
// Delete the matching resource. Uses the compare func to determine equality.
31-
Delete(resource T)
30+
// Delete the matching resource.
31+
Delete(resource ezkube.ResourceId)
3232
// Return the union with the provided set
3333
Union(set ResourceSet[T]) ResourceSet[T]
3434
// Return the difference with the provided set
@@ -157,13 +157,16 @@ func (s *resourceSet[T]) Equal(
157157
return s.Generic().Equal(set.Generic())
158158
}
159159

160-
func (s *resourceSet[T]) Delete(resource T) {
160+
func (s *resourceSet[T]) Delete(resource ezkube.ResourceId) {
161161
s.lock.Lock()
162162
defer s.lock.Unlock()
163+
164+
desired_key := sk_sets.Key(resource)
163165
i := sort.Search(len(s.set), func(i int) bool {
164-
return s.compareFunc(s.set[i], resource) >= 0
166+
return sk_sets.Key(s.set[i]) >= desired_key
165167
})
166-
if i != len(s.set) && s.compareFunc(s.set[i], resource) == 0 {
168+
found := i < len(s.set) && sk_sets.Key(s.set[i]) == desired_key
169+
if found {
167170
s.set = slices.Delete(s.set, i, i+1)
168171
}
169172
}

‎contrib/tests/set_v2_test.go

Lines changed: 78 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import (
1212

1313
var _ = FDescribe("PaintSetV2", func() {
1414
var (
15-
setA, setB sets_v2.ResourceSet[*v1.Paint]
16-
paintA, paintB, paintC *v1.Paint
15+
setA, setB sets_v2.ResourceSet[*v1.Paint]
16+
paintA, paintBCluster2, paintC *v1.Paint
1717
)
1818

1919
BeforeEach(func() {
@@ -22,7 +22,7 @@ var _ = FDescribe("PaintSetV2", func() {
2222
paintA = &v1.Paint{
2323
ObjectMeta: metav1.ObjectMeta{Name: "nameA", Namespace: "nsA"},
2424
}
25-
paintB = &v1.Paint{
25+
paintBCluster2 = &v1.Paint{
2626
ObjectMeta: metav1.ObjectMeta{Name: "nameB", Namespace: "nsB"},
2727
}
2828
paintC = &v1.Paint{
@@ -33,44 +33,44 @@ var _ = FDescribe("PaintSetV2", func() {
3333
It("should insert", func() {
3434
setA.Insert(paintA)
3535
Expect(setA.Has(paintA)).To(BeTrue())
36-
setA.Insert(paintB, paintC)
37-
Expect(setA.Has(paintB)).To(BeTrue())
36+
setA.Insert(paintBCluster2, paintC)
37+
Expect(setA.Has(paintBCluster2)).To(BeTrue())
3838
Expect(setA.Has(paintC)).To(BeTrue())
3939
Expect(setA.Len()).To(Equal(3))
4040
})
4141

4242
It("should return set existence", func() {
4343
setA.Insert(paintA)
4444
Expect(setA.Has(paintA)).To(BeTrue())
45-
Expect(setA.Has(paintB)).To(BeFalse())
46-
setA.Insert(paintB, paintC)
45+
Expect(setA.Has(paintBCluster2)).To(BeFalse())
46+
setA.Insert(paintBCluster2, paintC)
4747
Expect(setA.Has(paintA)).To(BeTrue())
48-
Expect(setA.Has(paintB)).To(BeTrue())
48+
Expect(setA.Has(paintBCluster2)).To(BeTrue())
4949
Expect(setA.Has(paintC)).To(BeTrue())
5050
})
5151

5252
It("should return set equality", func() {
53-
setB.Insert(paintA, paintB, paintC)
53+
setB.Insert(paintA, paintBCluster2, paintC)
5454
setA.Insert(paintA)
5555
Expect(setA.Equal(setB)).To(BeFalse())
56-
setA.Insert(paintC, paintB)
56+
setA.Insert(paintC, paintBCluster2)
5757
Expect(setA.Equal(setB)).To(BeTrue())
5858
})
5959

6060
It("should delete", func() {
61-
setA.Insert(paintA, paintB, paintC)
61+
setA.Insert(paintA, paintBCluster2, paintC)
6262
Expect(setA.Has(paintA)).To(BeTrue())
6363
setA.Delete(paintA)
6464
Expect(setA.Has(paintA)).To(BeFalse())
6565
})
6666

6767
It("should filter List", func() {
68-
setA.Insert(paintA, paintB, paintC)
68+
setA.Insert(paintA, paintBCluster2, paintC)
6969
Expect(setA.Has(paintA)).To(BeTrue())
7070

7171
for i, filtered := range setA.List(func(p *v1.Paint) bool { return p.GetName() == "nameA" }) {
7272
if i == 1 {
73-
Expect(filtered).To(Equal(paintB))
73+
Expect(filtered).To(Equal(paintBCluster2))
7474
}
7575
if i == 2 {
7676
Expect(filtered).To(Equal(paintC))
@@ -79,7 +79,7 @@ var _ = FDescribe("PaintSetV2", func() {
7979
})
8080

8181
It("should double filter List", func() {
82-
setA.Insert(paintA, paintB, paintC)
82+
setA.Insert(paintA, paintBCluster2, paintC)
8383
Expect(setA.Has(paintA)).To(BeTrue())
8484
for _, filtered := range setA.List(func(p *v1.Paint) bool {
8585
return p.Name == "nameA"
@@ -91,20 +91,20 @@ var _ = FDescribe("PaintSetV2", func() {
9191
})
9292

9393
It("should union two sets and return new set", func() {
94-
setA.Insert(paintA, paintB)
95-
setB.Insert(paintA, paintB, paintC)
94+
setA.Insert(paintA, paintBCluster2)
95+
setB.Insert(paintA, paintBCluster2, paintC)
9696
unionSet := setA.Union(setB)
9797
Expect(unionSet.Len()).To(Equal(3))
9898
Expect(unionSet.Has(paintA)).To(BeTrue())
99-
Expect(unionSet.Has(paintB)).To(BeTrue())
99+
Expect(unionSet.Has(paintBCluster2)).To(BeTrue())
100100
Expect(unionSet.Has(paintC)).To(BeTrue())
101101
Expect(unionSet).ToNot(BeIdenticalTo(setA))
102102
Expect(unionSet).ToNot(BeIdenticalTo(setB))
103103
})
104104

105105
It("should take the difference of two sets and return new set", func() {
106-
setA.Insert(paintA, paintB)
107-
setB.Insert(paintA, paintB, paintC)
106+
setA.Insert(paintA, paintBCluster2)
107+
setB.Insert(paintA, paintBCluster2, paintC)
108108
differenceA := setA.Difference(setB)
109109
Expect(differenceA.Len()).To(Equal(0))
110110
Expect(differenceA.Map()).To(BeEmpty())
@@ -117,27 +117,27 @@ var _ = FDescribe("PaintSetV2", func() {
117117
})
118118

119119
It("should take the intersection of two sets and return new set", func() {
120-
setA.Insert(paintA, paintB)
121-
setB.Insert(paintA, paintB, paintC)
120+
setA.Insert(paintA, paintBCluster2)
121+
setB.Insert(paintA, paintBCluster2, paintC)
122122
intersectionA := setA.Intersection(setB)
123123
Expect(intersectionA.Has(paintA)).To(BeTrue())
124-
Expect(intersectionA.Has(paintB)).To(BeTrue())
124+
Expect(intersectionA.Has(paintBCluster2)).To(BeTrue())
125125
Expect(intersectionA.Len()).To(Equal(2))
126126
Expect(intersectionA.Map()).To(HaveKeyWithValue(sets.Key(paintA), paintA))
127-
Expect(intersectionA.Map()).To(HaveKeyWithValue(sets.Key(paintB), paintB))
127+
Expect(intersectionA.Map()).To(HaveKeyWithValue(sets.Key(paintBCluster2), paintBCluster2))
128128
Expect(intersectionA).ToNot(BeIdenticalTo(setA))
129129
})
130130

131131
// It("should correctly match two sets", func() {
132-
// setA.Insert(paintA, paintB)
133-
// setB.Insert(paintA, paintB)
132+
// setA.Insert(paintA, paintBCluster2)
133+
// setB.Insert(paintA, paintBCluster2)
134134
// Expect(setA).To(Equal(setB))
135135
// setB.Insert(paintC)
136136
// Expect(setA).ToNot(Equal(setB))
137137
// })
138138

139139
It("should return corrent length", func() {
140-
setA.Insert(paintA, paintB)
140+
setA.Insert(paintA, paintBCluster2)
141141
Expect(setA.Len()).To(Equal(2))
142142
})
143143

@@ -224,4 +224,56 @@ var _ = FDescribe("PaintSetV2", func() {
224224
Expect(found).To(Equal(paint))
225225
}
226226
})
227+
228+
It("should sort resources first by cluster, then by namespace, then by name", func() {
229+
paintAAcluster1 := &v1.Paint{
230+
ObjectMeta: metav1.ObjectMeta{
231+
Name: "a",
232+
Namespace: "a",
233+
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-1"},
234+
},
235+
}
236+
paintABcluster1 := &v1.Paint{
237+
ObjectMeta: metav1.ObjectMeta{
238+
Name: "a",
239+
Namespace: "b",
240+
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-1"},
241+
},
242+
}
243+
paintBBcluster1 := &v1.Paint{
244+
ObjectMeta: metav1.ObjectMeta{
245+
Name: "b",
246+
Namespace: "b",
247+
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-1"},
248+
},
249+
}
250+
paintACluster2 := &v1.Paint{
251+
ObjectMeta: metav1.ObjectMeta{
252+
Name: "a",
253+
Namespace: "c",
254+
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-2"},
255+
},
256+
}
257+
paintBCluster2 := &v1.Paint{
258+
ObjectMeta: metav1.ObjectMeta{
259+
Name: "b",
260+
Namespace: "c",
261+
Annotations: map[string]string{ezkube.ClusterAnnotation: "cluster-2"},
262+
},
263+
}
264+
expectedOrder := []*v1.Paint{
265+
paintAAcluster1, paintABcluster1, paintBBcluster1,
266+
paintACluster2, paintBCluster2,
267+
}
268+
setA.Insert(expectedOrder...)
269+
270+
var paintList []*v1.Paint
271+
for _, paint := range setA.List() {
272+
paintList = append(paintList, paint)
273+
}
274+
275+
for i, paint := range expectedOrder {
276+
Expect(paintList[i]).To(Equal(paint))
277+
}
278+
})
227279
})

‎pkg/ezkube/resource_id.go

Lines changed: 12 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"sync"
66

77
"github.com/rotisserie/eris"
8-
"github.com/solo-io/skv2/pkg/controllerutils"
98
"sigs.k8s.io/controller-runtime/pkg/client"
109
)
1110

@@ -178,81 +177,32 @@ func ResourceIdFromKeyWithSeparator(key string, separator string) (ResourceId, e
178177
// CompareResourceId returns an integer comparing two ResourceIds lexicographically.
179178
// The result will be 0 if a == b, -1 if a < b, and +1 if a > b.
180179
// a, b must be of type ResourceId
181-
func CompareResourceIds(a, b interface{}) int {
182-
aa, aok := a.(ResourceId)
183-
bb, bok := b.(ResourceId)
184-
if !aok {
185-
panic("a is not a ResourceId")
186-
}
187-
if !bok {
188-
panic("b is not a ResourceId")
189-
}
190-
if resourceIdsEqual(aa, bb) {
180+
func ResourceIdsCompare(a, b client.Object) int {
181+
if resourceIdsEqual(a, b) {
191182
return 0
192183
}
193-
if resourceIdsLessThan(aa, bb) {
184+
if resourceIdsLessThan(a, b) {
194185
return -1
195186
}
196187
return 1
197188
}
198189

199-
func ResourceIdsAscending(a, b interface{}) bool {
200-
aa, aok := a.(ResourceId)
201-
bb, bok := b.(ResourceId)
202-
if !aok {
203-
panic("a is not a ResourceId")
204-
}
205-
if !bok {
206-
panic("b is not a ResourceId")
207-
}
208-
return resourceIdsLessThan(aa, bb)
209-
}
210-
211-
func resourceIdsEqual(a, b ResourceId) bool {
212-
return a.GetName() == b.GetName() && a.GetNamespace() == b.GetNamespace()
213-
}
214-
215-
func resourceIdsLessThan(a, b ResourceId) bool {
216-
// namespace is the primary sort key
217-
if a.GetNamespace() > b.GetNamespace() {
218-
return false
219-
}
220-
// name is the secondary sort key
221-
if a.GetName() < b.GetName() {
222-
return true
223-
}
224-
return false
190+
func resourceIdsEqual(a, b client.Object) bool {
191+
return a.GetName() == b.GetName() && a.GetNamespace() == b.GetNamespace() && GetClusterName(a) == GetClusterName(b)
225192
}
226193

227-
// CompareResourceId returns an integer comparing two ResourceIds lexicographically.
228-
// The result will be 0 if a == b, -1 if a < b, and +1 if a > b.
229-
// a, b must be of type ResourceId
230-
func CompareObjects(a, b client.Object) int {
231-
if controllerutils.ObjectsEqual(a, b) {
232-
return 0
233-
}
234-
if objectsLessThan(a, b) {
235-
return -1
236-
}
237-
return 1
238-
}
239-
240-
func ObjectsAscending(a, b client.Object) bool {
241-
return objectsLessThan(a, b)
242-
}
243-
244-
func objectsLessThan(a, b client.Object) bool {
194+
func resourceIdsLessThan(a, b client.Object) bool {
245195
// cluster name is the primary sort key
246196
if GetClusterName(a) > GetClusterName(b) {
247197
return false
248198
}
249199
// namespace is the secondary sort key
250-
if a.GetNamespace() < b.GetNamespace() {
251-
return true
200+
if a.GetNamespace() > b.GetNamespace() {
201+
return false
252202
}
253-
// name is the secondary sort key
254-
if a.GetName() < b.GetName() {
255-
return true
203+
// name is the tertiary sort key
204+
if a.GetName() > b.GetName() {
205+
return false
256206
}
257-
return false
207+
return true
258208
}

0 commit comments

Comments
 (0)