Skip to content

Commit c56a53c

Browse files
authored
Merge branch 'master' into chore/replace-golint
2 parents bf191d7 + 51ecc11 commit c56a53c

File tree

6 files changed

+59
-47
lines changed

6 files changed

+59
-47
lines changed

go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ require (
3636
github.com/beorn7/perks v1.0.1 // indirect
3737
github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect
3838
github.com/cespare/xxhash/v2 v2.2.0 // indirect
39-
github.com/containerd/containerd v1.7.11 // indirect
39+
github.com/containerd/containerd v1.7.12 // indirect
4040
github.com/containerd/log v0.1.0 // indirect
4141
github.com/cyphar/filepath-securejoin v0.2.4 // indirect
4242
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
@@ -144,7 +144,7 @@ require (
144144
gopkg.in/ini.v1 v1.67.0 // indirect
145145
gopkg.in/yaml.v2 v2.4.0 // indirect
146146
gopkg.in/yaml.v3 v3.0.1 // indirect
147-
helm.sh/helm/v3 v3.14.2 // indirect
147+
helm.sh/helm/v3 v3.14.3 // indirect
148148
k8s.io/apiextensions-apiserver v0.29.0 // indirect
149149
k8s.io/cli-runtime v0.29.0 // indirect
150150
k8s.io/component-base v0.29.0 // indirect

go.sum

+4
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ github.com/containerd/cgroups v1.1.0 h1:v8rEWFl6EoqHB+swVNjVoCJE8o3jX7e8nqBGPLaD
9292
github.com/containerd/cgroups v1.1.0/go.mod h1:6ppBcbh/NOOUU+dMKrykgaBnK9lCIBxHqJDGwsa1mIw=
9393
github.com/containerd/containerd v1.7.11 h1:lfGKw3eU35sjV0aG2eYZTiwFEY1pCzxdzicHP3SZILw=
9494
github.com/containerd/containerd v1.7.11/go.mod h1:5UluHxHTX2rdvYuZ5OJTC5m/KJNs0Zs9wVoJm9zf5ZE=
95+
github.com/containerd/containerd v1.7.12 h1:+KQsnv4VnzyxWcfO9mlxxELaoztsDEjOuCMPAuPqgU0=
96+
github.com/containerd/containerd v1.7.12/go.mod h1:/5OMpE1p0ylxtEUGY8kuCYkDRzJm9NO1TFMWjUpdevk=
9597
github.com/containerd/continuity v0.4.2 h1:v3y/4Yz5jwnvqPKJJ+7Wf93fyWoCB3F5EclWG023MDM=
9698
github.com/containerd/continuity v0.4.2/go.mod h1:F6PTNCKepoxEaXLQp3wDAjygEnImnZ/7o4JzpodfroQ=
9799
github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I=
@@ -869,6 +871,8 @@ gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o=
869871
gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g=
870872
helm.sh/helm/v3 v3.14.2 h1:V71fv+NGZv0icBlr+in1MJXuUIHCiPG1hW9gEBISTIA=
871873
helm.sh/helm/v3 v3.14.2/go.mod h1:2itvvDv2WSZXTllknfQo6j7u3VVgMAvm8POCDgYH424=
874+
helm.sh/helm/v3 v3.14.3 h1:HmvRJlwyyt9HjgmAuxHbHv3PhMz9ir/XNWHyXfmnOP4=
875+
helm.sh/helm/v3 v3.14.3/go.mod h1:v6myVbyseSBJTzhmeE39UcPLNv6cQK6qss3dvgAySaE=
872876
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
873877
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
874878
honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

pkg/controller/generic_reconciler.go

+8-10
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ func (gr *GenericReconciler) processNamespacedResources(
295295
gr.logger.Info("reconcileNamespaceResources",
296296
"Reconciling group of", len(objects), "objects with labels", label,
297297
"in the namespace", ns.name)
298-
err := gr.reconcileGroupOfObjects(ctx, objects, ns.name)
298+
err := gr.reconcileGroupOfObjects(objects, ns.uid)
299299
if err != nil {
300300
return fmt.Errorf(
301301
"reconciling related objects with labels '%s': %w", label, err,
@@ -307,15 +307,13 @@ func (gr *GenericReconciler) processNamespacedResources(
307307
return nil
308308
}
309309

310-
func (gr *GenericReconciler) reconcileGroupOfObjects(_ context.Context,
311-
objs []*unstructured.Unstructured, namespace string) error {
310+
func (gr *GenericReconciler) reconcileGroupOfObjects(objs []*unstructured.Unstructured, namespaceUID string) error {
312311

313-
if gr.allObjectsValidated(objs) {
312+
if gr.allObjectsValidated(objs, namespaceUID) {
314313
gr.logger.Info("reconcileGroupOfObjects", "All objects are validated", "Nothing to do")
315314
return nil
316315
}
317316

318-
namespaceUID := gr.watchNamespaces.getNamespaceUID(namespace)
319317
cliObjects := make([]client.Object, 0, len(objs))
320318
for _, o := range objs {
321319
typedClientObject, err := gr.unstructuredToTyped(o)
@@ -330,21 +328,21 @@ func (gr *GenericReconciler) reconcileGroupOfObjects(_ context.Context,
330328
return fmt.Errorf("running validations: %w", err)
331329
}
332330
for _, o := range objs {
333-
gr.objectValidationCache.store(o, outcome)
331+
gr.objectValidationCache.store(o, namespaceUID, outcome)
334332
}
335333

336334
return nil
337335
}
338336

339337
// allObjectsValidated checks whether all unstructured objects passed as argument are validated
340338
// and thus present in the cache
341-
func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructured) bool {
339+
func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructured, namespaceID string) bool {
342340
allObjectsValidated := true
343341
// we must be sure that all objects in the given group are cached (validated)
344342
// see DVO-103
345343
for _, o := range objs {
346-
gr.currentObjects.store(o, "")
347-
if !gr.objectValidationCache.objectAlreadyValidated(o) {
344+
gr.currentObjects.store(o, namespaceID, "")
345+
if !gr.objectValidationCache.objectAlreadyValidated(o, namespaceID) {
348346
allObjectsValidated = false
349347
}
350348
}
@@ -384,7 +382,7 @@ func (gr *GenericReconciler) handleResourceDeletions() {
384382
Kind: k.kind,
385383
Name: k.name,
386384
Namespace: k.namespace,
387-
NamespaceUID: gr.watchNamespaces.getNamespaceUID(k.namespace),
385+
NamespaceUID: k.nsID,
388386
UID: v.uid,
389387
}
390388

pkg/controller/generic_reconciler_test.go

+15-7
Original file line numberDiff line numberDiff line change
@@ -869,11 +869,12 @@ func TestProcessNamespacedResources(t *testing.T) {
869869
err = testReconciler.processNamespacedResources(context.Background(), tt.gvks, tt.namespaces)
870870
assert.NoError(t, err)
871871
for _, o := range tt.objects {
872-
vr, ok := testReconciler.objectValidationCache.retrieve(o)
872+
namespaceID := testReconciler.watchNamespaces.getNamespaceUID(o.GetNamespace())
873+
vr, ok := testReconciler.objectValidationCache.retrieve(o, namespaceID)
873874
assert.True(t, ok, "can't find object %v in the validation cache", o)
874875
assert.Equal(t, string(o.GetUID()), vr.uid)
875876

876-
co, ok := testReconciler.currentObjects.retrieve(o)
877+
co, ok := testReconciler.currentObjects.retrieve(o, namespaceID)
877878
assert.True(t, ok, "can't find object %v in the current objects", o)
878879
assert.Equal(t, string(o.GetUID()), co.uid)
879880
}
@@ -987,26 +988,33 @@ func TestHandleResourceDeletions(t *testing.T) {
987988

988989
// store the test objects in the caches
989990
for _, co := range tt.testCurrentObjects {
990-
testReconciler.currentObjects.store(co, validations.ObjectNeedsImprovement)
991+
testReconciler.currentObjects.store(co,
992+
testReconciler.watchNamespaces.getNamespaceUID(co.GetNamespace()),
993+
validations.ObjectNeedsImprovement)
991994
}
992995
for _, co := range tt.testValidatedObjects {
993-
testReconciler.objectValidationCache.store(co, validations.ObjectNeedsImprovement)
996+
testReconciler.objectValidationCache.store(co,
997+
testReconciler.watchNamespaces.getNamespaceUID(co.GetNamespace()),
998+
validations.ObjectNeedsImprovement)
994999
}
9951000
testReconciler.handleResourceDeletions()
9961001
// currentObjects should be always empty after calling handleResourceDeletions
9971002
for _, co := range tt.testCurrentObjects {
998-
_, ok := testReconciler.currentObjects.retrieve(co)
1003+
_, ok := testReconciler.currentObjects.retrieve(co,
1004+
testReconciler.watchNamespaces.getNamespaceUID(co.GetNamespace()))
9991005
assert.False(t, ok)
10001006
}
10011007

10021008
if tt.expectedValidatedObjects == nil {
10031009
for _, vo := range tt.testValidatedObjects {
1004-
_, ok := testReconciler.objectValidationCache.retrieve(vo)
1010+
_, ok := testReconciler.objectValidationCache.retrieve(vo,
1011+
testReconciler.watchNamespaces.getNamespaceUID(vo.GetNamespace()))
10051012
assert.False(t, ok)
10061013
}
10071014
} else {
10081015
for _, vo := range tt.expectedValidatedObjects {
1009-
_, ok := testReconciler.objectValidationCache.retrieve(vo)
1016+
_, ok := testReconciler.objectValidationCache.retrieve(vo,
1017+
testReconciler.watchNamespaces.getNamespaceUID(vo.GetNamespace()))
10101018
assert.True(t, ok)
10111019
}
10121020
}

pkg/controller/validationscache.go

+15-13
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import (
77
)
88

99
type validationKey struct {
10-
group, version, kind, namespace, name string
11-
uid types.UID
10+
group, version, kind string
11+
name, namespace, nsID string
12+
uid types.UID
1213
}
1314

1415
type resourceVersion string
@@ -19,14 +20,15 @@ func newResourceversionVal(str string) resourceVersion {
1920

2021
// newValidationKey returns a unique identifier for the given
2122
// object suitable for hashing.
22-
func newValidationKey(obj client.Object) validationKey {
23+
func newValidationKey(obj client.Object, nsID string) validationKey {
2324
gvk := obj.GetObjectKind().GroupVersionKind()
2425
return validationKey{
2526
group: gvk.Group,
2627
version: gvk.Version,
2728
kind: gvk.Kind,
28-
namespace: obj.GetNamespace(),
2929
name: obj.GetName(),
30+
namespace: obj.GetNamespace(),
31+
nsID: nsID,
3032
uid: obj.GetUID(),
3133
}
3234
}
@@ -67,8 +69,8 @@ func (vc *validationCache) has(key validationKey) bool {
6769
// store caches a 'ValidationOutcome' for the given 'Object'.
6870
// constraint: cached outcomes will be updated in-place for a given object and
6971
// consecutive updates will not preserve previous state.
70-
func (vc *validationCache) store(obj client.Object, outcome validations.ValidationOutcome) {
71-
key := newValidationKey(obj)
72+
func (vc *validationCache) store(obj client.Object, nsID string, outcome validations.ValidationOutcome) {
73+
key := newValidationKey(obj, nsID)
7274
(*vc)[key] = newValidationResource(
7375
newResourceversionVal(obj.GetResourceVersion()),
7476
string(obj.GetUID()),
@@ -85,8 +87,8 @@ func (vc *validationCache) drain() {
8587
// remove uncaches the 'ValidationOutcome' for the
8688
// given object if it exists and performs a noop
8789
// if it does not.
88-
func (vc *validationCache) remove(obj client.Object) {
89-
key := newValidationKey(obj)
90+
func (vc *validationCache) remove(obj client.Object, nsID string) {
91+
key := newValidationKey(obj, nsID)
9092
vc.removeKey(key)
9193
}
9294

@@ -98,8 +100,8 @@ func (vc *validationCache) removeKey(key validationKey) {
98100
// retrieve returns a tuple of 'validationResource' (if present)
99101
// and 'ok' which returns 'true' if a 'validationResource' exists
100102
// for the given 'Object' and 'false' otherwise.
101-
func (vc *validationCache) retrieve(obj client.Object) (*validationResource, bool) {
102-
key := newValidationKey(obj)
103+
func (vc *validationCache) retrieve(obj client.Object, nsID string) (*validationResource, bool) {
104+
key := newValidationKey(obj, nsID)
103105
val, exists := (*vc)[key]
104106
return val, exists
105107
}
@@ -110,15 +112,15 @@ func (vc *validationCache) retrieve(obj client.Object) (*validationResource, boo
110112
// If the 'ResourceVersion' of an existing 'Object' is stale the cached
111113
// 'ValidationOutcome' is removed and 'false' is returned. In all other
112114
// cases 'false' is returned.
113-
func (vc *validationCache) objectAlreadyValidated(obj client.Object) bool {
114-
validationOutcome, ok := vc.retrieve(obj)
115+
func (vc *validationCache) objectAlreadyValidated(obj client.Object, nsID string) bool {
116+
validationOutcome, ok := vc.retrieve(obj, nsID)
115117
if !ok {
116118
return false
117119
}
118120
storedResourceVersion := validationOutcome.version
119121
currentResourceVersion := obj.GetResourceVersion()
120122
if string(storedResourceVersion) != currentResourceVersion {
121-
vc.remove(obj)
123+
vc.remove(obj, nsID)
122124
return false
123125
}
124126
return true

pkg/controller/validationscache_test.go

+15-15
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,11 @@ func TestValidationsCache(t *testing.T) {
2929
}}
3030

3131
// When
32-
mock.store(&mockClientObject, "mock_outcome")
32+
mock.store(&mockClientObject, "", "mock_outcome")
3333

3434
// Assert
3535
expected := newValidationResource(newResourceversionVal("mock_version"), "mock_uid", "mock_outcome")
36-
assert.Equal(t, expected, (*mock)[newValidationKey(&mockClientObject)])
36+
assert.Equal(t, expected, (*mock)[newValidationKey(&mockClientObject, "")])
3737
})
3838

3939
t.Run("objectAlreadyValidated : key does not exist", func(t *testing.T) {
@@ -45,7 +45,7 @@ func TestValidationsCache(t *testing.T) {
4545
}}
4646

4747
// When
48-
test := mock.objectAlreadyValidated(&mockClientObject)
48+
test := mock.objectAlreadyValidated(&mockClientObject, "")
4949

5050
// Assert
5151
assert.False(t, test)
@@ -58,12 +58,12 @@ func TestValidationsCache(t *testing.T) {
5858
ResourceVersion: "mock_version",
5959
UID: "mock_uid",
6060
}}
61-
mock.store(&mockClientObject, "mock_outcome")
62-
toBeRemovedKey := newValidationKey(&mockClientObject)
61+
mock.store(&mockClientObject, "", "mock_outcome")
62+
toBeRemovedKey := newValidationKey(&mockClientObject, "")
6363

6464
// When
6565
mockClientObject.ResourceVersion = "mock_new_version"
66-
test := mock.objectAlreadyValidated(&mockClientObject)
66+
test := mock.objectAlreadyValidated(&mockClientObject, "")
6767

6868
// Assert
6969
assert.False(t, test)
@@ -77,10 +77,10 @@ func TestValidationsCache(t *testing.T) {
7777
ResourceVersion: "mock_version",
7878
UID: "mock_uid",
7979
}}
80-
mock.store(&mockClientObject, "mock_outcome")
80+
mock.store(&mockClientObject, "", "mock_outcome")
8181

8282
// When
83-
test := mock.objectAlreadyValidated(&mockClientObject)
83+
test := mock.objectAlreadyValidated(&mockClientObject, "")
8484

8585
// Assert
8686
assert.True(t, test)
@@ -103,14 +103,14 @@ func TestValidationsCache(t *testing.T) {
103103
UID: "bar345",
104104
},
105105
}
106-
testCache.store(&dep1, validations.ObjectNeedsImprovement)
107-
testCache.store(&dep2, validations.ObjectValid)
106+
testCache.store(&dep1, "", validations.ObjectNeedsImprovement)
107+
testCache.store(&dep2, "", validations.ObjectValid)
108108

109-
resource1, exists := testCache.retrieve(&dep1)
109+
resource1, exists := testCache.retrieve(&dep1, "")
110110
assert.True(t, exists)
111111
assert.Equal(t, validations.ObjectNeedsImprovement, resource1.outcome)
112112

113-
resource2, exists := testCache.retrieve(&dep2)
113+
resource2, exists := testCache.retrieve(&dep2, "")
114114
assert.True(t, exists)
115115
assert.Equal(t, validations.ObjectValid, resource2.outcome)
116116
})
@@ -128,19 +128,19 @@ func Benchmark_ValidationCache(b *testing.B) {
128128

129129
for i := 0; i < b.N; i++ {
130130
name := fmt.Sprintf("test-%d", i)
131-
vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, validations.ObjectValid)
131+
vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, "", validations.ObjectValid)
132132
}
133133
printMemoryInfo(fmt.Sprintf("Memory consumption after storing %d items in the cache", b.N))
134134
for i := 0; i < b.N; i++ {
135135
name := fmt.Sprintf("test-%d", i)
136-
vc.remove(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}})
136+
vc.remove(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, "")
137137
}
138138
runtime.GC()
139139
printMemoryInfo("Memory consumption after removing the items ")
140140

141141
for i := 0; i < b.N; i++ {
142142
name := fmt.Sprintf("test-%d", i)
143-
vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, validations.ObjectValid)
143+
vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, "", validations.ObjectValid)
144144
}
145145
printMemoryInfo(fmt.Sprintf("Memory consumption after storing %d items again", b.N))
146146
}

0 commit comments

Comments
 (0)