From 4d16e79b89c878b8249e3b0081913b257ec98a97 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Wed, 3 Apr 2024 21:00:32 +0200 Subject: [PATCH 1/7] draft validations cache key rewriting to include namespace Id --- pkg/controller/generic_reconciler.go | 43 +++++++++++++++++++++------- pkg/controller/validationscache.go | 28 +++++++++--------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 87744e55..552284c0 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" "sync" + "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -295,27 +296,31 @@ func (gr *GenericReconciler) processNamespacedResources( gr.logger.Info("reconcileNamespaceResources", "Reconciling group of", len(objects), "objects with labels", label, "in the namespace", ns.name) - err := gr.reconcileGroupOfObjects(ctx, objects, ns.name) + err := gr.reconcileGroupOfObjects(ctx, objects, ns.name, ns.uid) if err != nil { return fmt.Errorf( "reconciling related objects with labels '%s': %w", label, err, ) } } + if ns.name == "test-dvo-5" { + gr.logger.Info("////// replacing namespace with id", "id", ns.uid) + time.Sleep(2 * time.Minute) + } } return nil } func (gr *GenericReconciler) reconcileGroupOfObjects(ctx context.Context, - objs []*unstructured.Unstructured, namespace string) error { + objs []*unstructured.Unstructured, namespace string, namespaceUID string) error { if gr.allObjectsValidated(objs) { gr.logger.Info("reconcileGroupOfObjects", "All objects are validated", "Nothing to do") return nil } - namespaceUID := gr.watchNamespaces.getNamespaceUID(namespace) + //namespaceUID := gr.watchNamespaces.getNamespaceUID(namespace) cliObjects := make([]client.Object, 0, len(objs)) for _, o := range objs { typedClientObject, err := gr.unstructuredToTyped(o) @@ -330,7 +335,7 @@ func (gr *GenericReconciler) reconcileGroupOfObjects(ctx context.Context, return fmt.Errorf("running validations: %w", err) } for _, o := range objs { - gr.objectValidationCache.store(o, outcome) + gr.objectValidationCache.store(o, namespaceUID, outcome) } return nil @@ -343,8 +348,9 @@ func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructur // we must be sure that all objects in the given group are cached (validated) // see DVO-103 for _, o := range objs { - gr.currentObjects.store(o, "") - if !gr.objectValidationCache.objectAlreadyValidated(o) { + namespaceId := gr.watchNamespaces.getNamespaceUID(o.GetNamespace()) + gr.currentObjects.store(o, namespaceId, "") + if !gr.objectValidationCache.objectAlreadyValidated(o, namespaceId) { allObjectsValidated = false } } @@ -380,14 +386,29 @@ func (gr *GenericReconciler) handleResourceDeletions() { continue } + // resets namespaces (once namespaces ID is part of the key) + // gr.watchNamespaces.resetCache() + // namespaces, err := gr.watchNamespaces.getWatchNamespaces(c, gr.client) + // if err != nil { + // return fmt.Errorf("getting watched namespaces: %w", err) + // } + + nsId := gr.watchNamespaces.getNamespaceUID(k.namespace) req := validations.Request{ - Kind: k.kind, - Name: k.name, - Namespace: k.namespace, - NamespaceUID: gr.watchNamespaces.getNamespaceUID(k.namespace), - UID: v.uid, + Kind: k.kind, + Name: k.name, + Namespace: k.namespace, + // NamespaceUID: gr.watchNamespaces.getNamespaceUID(k.namespace), + NamespaceUID: k.nsId, + //NamespaceUID: nsId, // use correct namespace ID coming from validations cache key + // try logging the correct namespace or a diff to check if it works without removing the metrics + //save this UID in the key for the cache + UID: v.uid, } + if k.namespace == "test-dvo-5" { + gr.logger.Info("////// removing metrics for resource", "resource", k.name, "namespace", k.namespace, "nsId", nsId) + } gr.validationEngine.DeleteMetrics(req.ToPromLabels()) gr.objectValidationCache.removeKey(k) diff --git a/pkg/controller/validationscache.go b/pkg/controller/validationscache.go index 153fe336..a1905a9a 100644 --- a/pkg/controller/validationscache.go +++ b/pkg/controller/validationscache.go @@ -7,8 +7,9 @@ import ( ) type validationKey struct { - group, version, kind, namespace, name string - uid types.UID + group, version, kind string + name, namespace, nsId string + uid types.UID } type resourceVersion string @@ -19,14 +20,15 @@ func newResourceversionVal(str string) resourceVersion { // newValidationKey returns a unique identifier for the given // object suitable for hashing. -func newValidationKey(obj client.Object) validationKey { +func newValidationKey(obj client.Object, nsId string) validationKey { gvk := obj.GetObjectKind().GroupVersionKind() return validationKey{ group: gvk.Group, version: gvk.Version, kind: gvk.Kind, - namespace: obj.GetNamespace(), name: obj.GetName(), + namespace: obj.GetNamespace(), + nsId: nsId, uid: obj.GetUID(), } } @@ -67,8 +69,8 @@ func (vc *validationCache) has(key validationKey) bool { // store caches a 'ValidationOutcome' for the given 'Object'. // constraint: cached outcomes will be updated in-place for a given object and // consecutive updates will not preserve previous state. -func (vc *validationCache) store(obj client.Object, outcome validations.ValidationOutcome) { - key := newValidationKey(obj) +func (vc *validationCache) store(obj client.Object, nsId string, outcome validations.ValidationOutcome) { + key := newValidationKey(obj, nsId) (*vc)[key] = newValidationResource( newResourceversionVal(obj.GetResourceVersion()), string(obj.GetUID()), @@ -85,8 +87,8 @@ func (vc *validationCache) drain() { // remove uncaches the 'ValidationOutcome' for the // given object if it exists and performs a noop // if it does not. -func (vc *validationCache) remove(obj client.Object) { - key := newValidationKey(obj) +func (vc *validationCache) remove(obj client.Object, nsId string) { + key := newValidationKey(obj, nsId) vc.removeKey(key) } @@ -98,8 +100,8 @@ func (vc *validationCache) removeKey(key validationKey) { // retrieve returns a tuple of 'validationResource' (if present) // and 'ok' which returns 'true' if a 'validationResource' exists // for the given 'Object' and 'false' otherwise. -func (vc *validationCache) retrieve(obj client.Object) (*validationResource, bool) { - key := newValidationKey(obj) +func (vc *validationCache) retrieve(obj client.Object, nsId string) (*validationResource, bool) { + key := newValidationKey(obj, nsId) val, exists := (*vc)[key] return val, exists } @@ -110,15 +112,15 @@ func (vc *validationCache) retrieve(obj client.Object) (*validationResource, boo // If the 'ResourceVersion' of an existing 'Object' is stale the cached // 'ValidationOutcome' is removed and 'false' is returned. In all other // cases 'false' is returned. -func (vc *validationCache) objectAlreadyValidated(obj client.Object) bool { - validationOutcome, ok := vc.retrieve(obj) +func (vc *validationCache) objectAlreadyValidated(obj client.Object, nsId string) bool { + validationOutcome, ok := vc.retrieve(obj, nsId) if !ok { return false } storedResourceVersion := validationOutcome.version currentResourceVersion := obj.GetResourceVersion() if string(storedResourceVersion) != currentResourceVersion { - vc.remove(obj) + vc.remove(obj, nsId) return false } return true From 739f310c6be889a6e4ff66a9fdee25b607a465a9 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 4 Apr 2024 11:32:43 +0200 Subject: [PATCH 2/7] cleanup error replication and logging lines --- pkg/controller/generic_reconciler.go | 30 +++++----------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 552284c0..63567a71 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -9,7 +9,6 @@ import ( "strconv" "strings" "sync" - "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -303,10 +302,6 @@ func (gr *GenericReconciler) processNamespacedResources( ) } } - if ns.name == "test-dvo-5" { - gr.logger.Info("////// replacing namespace with id", "id", ns.uid) - time.Sleep(2 * time.Minute) - } } return nil @@ -320,7 +315,6 @@ func (gr *GenericReconciler) reconcileGroupOfObjects(ctx context.Context, return nil } - //namespaceUID := gr.watchNamespaces.getNamespaceUID(namespace) cliObjects := make([]client.Object, 0, len(objs)) for _, o := range objs { typedClientObject, err := gr.unstructuredToTyped(o) @@ -349,6 +343,7 @@ func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructur // see DVO-103 for _, o := range objs { namespaceId := gr.watchNamespaces.getNamespaceUID(o.GetNamespace()) + gr.currentObjects.store(o, namespaceId, "") if !gr.objectValidationCache.objectAlreadyValidated(o, namespaceId) { allObjectsValidated = false @@ -386,29 +381,14 @@ func (gr *GenericReconciler) handleResourceDeletions() { continue } - // resets namespaces (once namespaces ID is part of the key) - // gr.watchNamespaces.resetCache() - // namespaces, err := gr.watchNamespaces.getWatchNamespaces(c, gr.client) - // if err != nil { - // return fmt.Errorf("getting watched namespaces: %w", err) - // } - - nsId := gr.watchNamespaces.getNamespaceUID(k.namespace) req := validations.Request{ - Kind: k.kind, - Name: k.name, - Namespace: k.namespace, - // NamespaceUID: gr.watchNamespaces.getNamespaceUID(k.namespace), + Kind: k.kind, + Name: k.name, + Namespace: k.namespace, NamespaceUID: k.nsId, - //NamespaceUID: nsId, // use correct namespace ID coming from validations cache key - // try logging the correct namespace or a diff to check if it works without removing the metrics - //save this UID in the key for the cache - UID: v.uid, + UID: v.uid, } - if k.namespace == "test-dvo-5" { - gr.logger.Info("////// removing metrics for resource", "resource", k.name, "namespace", k.namespace, "nsId", nsId) - } gr.validationEngine.DeleteMetrics(req.ToPromLabels()) gr.objectValidationCache.removeKey(k) From a7b3624d1f6c940fd7e29dea42299102b30a78e1 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 4 Apr 2024 13:33:03 +0200 Subject: [PATCH 3/7] Fix unused parameters in signature --- pkg/controller/generic_reconciler.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 63567a71..c816ecd6 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -295,7 +295,7 @@ func (gr *GenericReconciler) processNamespacedResources( gr.logger.Info("reconcileNamespaceResources", "Reconciling group of", len(objects), "objects with labels", label, "in the namespace", ns.name) - err := gr.reconcileGroupOfObjects(ctx, objects, ns.name, ns.uid) + err := gr.reconcileGroupOfObjects(objects, ns.uid) if err != nil { return fmt.Errorf( "reconciling related objects with labels '%s': %w", label, err, @@ -307,8 +307,7 @@ func (gr *GenericReconciler) processNamespacedResources( return nil } -func (gr *GenericReconciler) reconcileGroupOfObjects(ctx context.Context, - objs []*unstructured.Unstructured, namespace string, namespaceUID string) error { +func (gr *GenericReconciler) reconcileGroupOfObjects(objs []*unstructured.Unstructured, namespaceUID string) error { if gr.allObjectsValidated(objs) { gr.logger.Info("reconcileGroupOfObjects", "All objects are validated", "Nothing to do") From e78ce3ddcc20cb9c4e3392e2f447d16be672da4d Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 4 Apr 2024 13:33:21 +0200 Subject: [PATCH 4/7] Fix Unit tests after change in signature --- pkg/controller/generic_reconciler_test.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/controller/generic_reconciler_test.go b/pkg/controller/generic_reconciler_test.go index 675fe638..2cc1888b 100644 --- a/pkg/controller/generic_reconciler_test.go +++ b/pkg/controller/generic_reconciler_test.go @@ -869,11 +869,12 @@ func TestProcessNamespacedResources(t *testing.T) { err = testReconciler.processNamespacedResources(context.Background(), tt.gvks, tt.namespaces) assert.NoError(t, err) for _, o := range tt.objects { - vr, ok := testReconciler.objectValidationCache.retrieve(o) + namespaceId := testReconciler.watchNamespaces.getNamespaceUID(o.GetNamespace()) + vr, ok := testReconciler.objectValidationCache.retrieve(o, namespaceId) assert.True(t, ok, "can't find object %v in the validation cache", o) assert.Equal(t, string(o.GetUID()), vr.uid) - co, ok := testReconciler.currentObjects.retrieve(o) + co, ok := testReconciler.currentObjects.retrieve(o, namespaceId) assert.True(t, ok, "can't find object %v in the current objects", o) assert.Equal(t, string(o.GetUID()), co.uid) } @@ -987,26 +988,33 @@ func TestHandleResourceDeletions(t *testing.T) { // store the test objects in the caches for _, co := range tt.testCurrentObjects { - testReconciler.currentObjects.store(co, validations.ObjectNeedsImprovement) + testReconciler.currentObjects.store(co, + testReconciler.watchNamespaces.getNamespaceUID(co.GetNamespace()), + validations.ObjectNeedsImprovement) } for _, co := range tt.testValidatedObjects { - testReconciler.objectValidationCache.store(co, validations.ObjectNeedsImprovement) + testReconciler.objectValidationCache.store(co, + testReconciler.watchNamespaces.getNamespaceUID(co.GetNamespace()), + validations.ObjectNeedsImprovement) } testReconciler.handleResourceDeletions() // currentObjects should be always empty after calling handleResourceDeletions for _, co := range tt.testCurrentObjects { - _, ok := testReconciler.currentObjects.retrieve(co) + _, ok := testReconciler.currentObjects.retrieve(co, + testReconciler.watchNamespaces.getNamespaceUID(co.GetNamespace())) assert.False(t, ok) } if tt.expectedValidatedObjects == nil { for _, vo := range tt.testValidatedObjects { - _, ok := testReconciler.objectValidationCache.retrieve(vo) + _, ok := testReconciler.objectValidationCache.retrieve(vo, + testReconciler.watchNamespaces.getNamespaceUID(vo.GetNamespace())) assert.False(t, ok) } } else { for _, vo := range tt.expectedValidatedObjects { - _, ok := testReconciler.objectValidationCache.retrieve(vo) + _, ok := testReconciler.objectValidationCache.retrieve(vo, + testReconciler.watchNamespaces.getNamespaceUID(vo.GetNamespace())) assert.True(t, ok) } } From 3d3dc1ce5d546e152bf66a496cce6464af3df042 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 4 Apr 2024 13:42:24 +0200 Subject: [PATCH 5/7] Fix Unit tests after change in signature --- pkg/controller/validationscache_test.go | 30 ++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/controller/validationscache_test.go b/pkg/controller/validationscache_test.go index d6781a77..5c958eb2 100644 --- a/pkg/controller/validationscache_test.go +++ b/pkg/controller/validationscache_test.go @@ -29,11 +29,11 @@ func TestValidationsCache(t *testing.T) { }} // When - mock.store(&mockClientObject, "mock_outcome") + mock.store(&mockClientObject, "", "mock_outcome") // Assert expected := newValidationResource(newResourceversionVal("mock_version"), "mock_uid", "mock_outcome") - assert.Equal(t, expected, (*mock)[newValidationKey(&mockClientObject)]) + assert.Equal(t, expected, (*mock)[newValidationKey(&mockClientObject, "")]) }) t.Run("objectAlreadyValidated : key does not exist", func(t *testing.T) { @@ -45,7 +45,7 @@ func TestValidationsCache(t *testing.T) { }} // When - test := mock.objectAlreadyValidated(&mockClientObject) + test := mock.objectAlreadyValidated(&mockClientObject, "") // Assert assert.False(t, test) @@ -58,12 +58,12 @@ func TestValidationsCache(t *testing.T) { ResourceVersion: "mock_version", UID: "mock_uid", }} - mock.store(&mockClientObject, "mock_outcome") - toBeRemovedKey := newValidationKey(&mockClientObject) + mock.store(&mockClientObject, "", "mock_outcome") + toBeRemovedKey := newValidationKey(&mockClientObject, "") // When mockClientObject.ResourceVersion = "mock_new_version" - test := mock.objectAlreadyValidated(&mockClientObject) + test := mock.objectAlreadyValidated(&mockClientObject, "") // Assert assert.False(t, test) @@ -77,10 +77,10 @@ func TestValidationsCache(t *testing.T) { ResourceVersion: "mock_version", UID: "mock_uid", }} - mock.store(&mockClientObject, "mock_outcome") + mock.store(&mockClientObject, "", "mock_outcome") // When - test := mock.objectAlreadyValidated(&mockClientObject) + test := mock.objectAlreadyValidated(&mockClientObject, "") // Assert assert.True(t, test) @@ -103,14 +103,14 @@ func TestValidationsCache(t *testing.T) { UID: "bar345", }, } - testCache.store(&dep1, validations.ObjectNeedsImprovement) - testCache.store(&dep2, validations.ObjectValid) + testCache.store(&dep1, "", validations.ObjectNeedsImprovement) + testCache.store(&dep2, "", validations.ObjectValid) - resource1, exists := testCache.retrieve(&dep1) + resource1, exists := testCache.retrieve(&dep1, "") assert.True(t, exists) assert.Equal(t, validations.ObjectNeedsImprovement, resource1.outcome) - resource2, exists := testCache.retrieve(&dep2) + resource2, exists := testCache.retrieve(&dep2, "") assert.True(t, exists) assert.Equal(t, validations.ObjectValid, resource2.outcome) }) @@ -128,19 +128,19 @@ func Benchmark_ValidationCache(b *testing.B) { for i := 0; i < b.N; i++ { name := fmt.Sprintf("test-%d", i) - vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, validations.ObjectValid) + vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, "", validations.ObjectValid) } printMemoryInfo(fmt.Sprintf("Memory consumption after storing %d items in the cache", b.N)) for i := 0; i < b.N; i++ { name := fmt.Sprintf("test-%d", i) - vc.remove(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}) + vc.remove(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, "") } runtime.GC() printMemoryInfo("Memory consumption after removing the items ") for i := 0; i < b.N; i++ { name := fmt.Sprintf("test-%d", i) - vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, validations.ObjectValid) + vc.store(&appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: name}}, "", validations.ObjectValid) } printMemoryInfo(fmt.Sprintf("Memory consumption after storing %d items again", b.N)) } From d36c9f973a0d39eb848b1368c5d2dadad96f8652 Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Thu, 4 Apr 2024 15:41:46 +0200 Subject: [PATCH 6/7] Fix linting (golint) --- pkg/controller/generic_reconciler.go | 8 ++++---- pkg/controller/generic_reconciler_test.go | 6 +++--- pkg/controller/validationscache.go | 24 +++++++++++------------ 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index c816ecd6..8ff30616 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -341,10 +341,10 @@ func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructur // we must be sure that all objects in the given group are cached (validated) // see DVO-103 for _, o := range objs { - namespaceId := gr.watchNamespaces.getNamespaceUID(o.GetNamespace()) + namespaceID := gr.watchNamespaces.getNamespaceUID(o.GetNamespace()) - gr.currentObjects.store(o, namespaceId, "") - if !gr.objectValidationCache.objectAlreadyValidated(o, namespaceId) { + gr.currentObjects.store(o, namespaceID, "") + if !gr.objectValidationCache.objectAlreadyValidated(o, namespaceID) { allObjectsValidated = false } } @@ -384,7 +384,7 @@ func (gr *GenericReconciler) handleResourceDeletions() { Kind: k.kind, Name: k.name, Namespace: k.namespace, - NamespaceUID: k.nsId, + NamespaceUID: k.nsID, UID: v.uid, } diff --git a/pkg/controller/generic_reconciler_test.go b/pkg/controller/generic_reconciler_test.go index 2cc1888b..45ffa26d 100644 --- a/pkg/controller/generic_reconciler_test.go +++ b/pkg/controller/generic_reconciler_test.go @@ -869,12 +869,12 @@ func TestProcessNamespacedResources(t *testing.T) { err = testReconciler.processNamespacedResources(context.Background(), tt.gvks, tt.namespaces) assert.NoError(t, err) for _, o := range tt.objects { - namespaceId := testReconciler.watchNamespaces.getNamespaceUID(o.GetNamespace()) - vr, ok := testReconciler.objectValidationCache.retrieve(o, namespaceId) + namespaceID := testReconciler.watchNamespaces.getNamespaceUID(o.GetNamespace()) + vr, ok := testReconciler.objectValidationCache.retrieve(o, namespaceID) assert.True(t, ok, "can't find object %v in the validation cache", o) assert.Equal(t, string(o.GetUID()), vr.uid) - co, ok := testReconciler.currentObjects.retrieve(o, namespaceId) + co, ok := testReconciler.currentObjects.retrieve(o, namespaceID) assert.True(t, ok, "can't find object %v in the current objects", o) assert.Equal(t, string(o.GetUID()), co.uid) } diff --git a/pkg/controller/validationscache.go b/pkg/controller/validationscache.go index a1905a9a..6f26fa76 100644 --- a/pkg/controller/validationscache.go +++ b/pkg/controller/validationscache.go @@ -8,7 +8,7 @@ import ( type validationKey struct { group, version, kind string - name, namespace, nsId string + name, namespace, nsID string uid types.UID } @@ -20,7 +20,7 @@ func newResourceversionVal(str string) resourceVersion { // newValidationKey returns a unique identifier for the given // object suitable for hashing. -func newValidationKey(obj client.Object, nsId string) validationKey { +func newValidationKey(obj client.Object, nsID string) validationKey { gvk := obj.GetObjectKind().GroupVersionKind() return validationKey{ group: gvk.Group, @@ -28,7 +28,7 @@ func newValidationKey(obj client.Object, nsId string) validationKey { kind: gvk.Kind, name: obj.GetName(), namespace: obj.GetNamespace(), - nsId: nsId, + nsID: nsID, uid: obj.GetUID(), } } @@ -69,8 +69,8 @@ func (vc *validationCache) has(key validationKey) bool { // store caches a 'ValidationOutcome' for the given 'Object'. // constraint: cached outcomes will be updated in-place for a given object and // consecutive updates will not preserve previous state. -func (vc *validationCache) store(obj client.Object, nsId string, outcome validations.ValidationOutcome) { - key := newValidationKey(obj, nsId) +func (vc *validationCache) store(obj client.Object, nsID string, outcome validations.ValidationOutcome) { + key := newValidationKey(obj, nsID) (*vc)[key] = newValidationResource( newResourceversionVal(obj.GetResourceVersion()), string(obj.GetUID()), @@ -87,8 +87,8 @@ func (vc *validationCache) drain() { // remove uncaches the 'ValidationOutcome' for the // given object if it exists and performs a noop // if it does not. -func (vc *validationCache) remove(obj client.Object, nsId string) { - key := newValidationKey(obj, nsId) +func (vc *validationCache) remove(obj client.Object, nsID string) { + key := newValidationKey(obj, nsID) vc.removeKey(key) } @@ -100,8 +100,8 @@ func (vc *validationCache) removeKey(key validationKey) { // retrieve returns a tuple of 'validationResource' (if present) // and 'ok' which returns 'true' if a 'validationResource' exists // for the given 'Object' and 'false' otherwise. -func (vc *validationCache) retrieve(obj client.Object, nsId string) (*validationResource, bool) { - key := newValidationKey(obj, nsId) +func (vc *validationCache) retrieve(obj client.Object, nsID string) (*validationResource, bool) { + key := newValidationKey(obj, nsID) val, exists := (*vc)[key] return val, exists } @@ -112,15 +112,15 @@ func (vc *validationCache) retrieve(obj client.Object, nsId string) (*validation // If the 'ResourceVersion' of an existing 'Object' is stale the cached // 'ValidationOutcome' is removed and 'false' is returned. In all other // cases 'false' is returned. -func (vc *validationCache) objectAlreadyValidated(obj client.Object, nsId string) bool { - validationOutcome, ok := vc.retrieve(obj, nsId) +func (vc *validationCache) objectAlreadyValidated(obj client.Object, nsID string) bool { + validationOutcome, ok := vc.retrieve(obj, nsID) if !ok { return false } storedResourceVersion := validationOutcome.version currentResourceVersion := obj.GetResourceVersion() if string(storedResourceVersion) != currentResourceVersion { - vc.remove(obj, nsId) + vc.remove(obj, nsID) return false } return true From 4d88ff3afaedc09ec285e24f392e5b3c75812b1e Mon Sep 17 00:00:00 2001 From: Isaac Jimeno Date: Mon, 8 Apr 2024 19:20:17 +0200 Subject: [PATCH 7/7] Fix missing namespace ID access --- pkg/controller/generic_reconciler.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/controller/generic_reconciler.go b/pkg/controller/generic_reconciler.go index 8ff30616..6cc35be5 100644 --- a/pkg/controller/generic_reconciler.go +++ b/pkg/controller/generic_reconciler.go @@ -309,7 +309,7 @@ func (gr *GenericReconciler) processNamespacedResources( func (gr *GenericReconciler) reconcileGroupOfObjects(objs []*unstructured.Unstructured, namespaceUID string) error { - if gr.allObjectsValidated(objs) { + if gr.allObjectsValidated(objs, namespaceUID) { gr.logger.Info("reconcileGroupOfObjects", "All objects are validated", "Nothing to do") return nil } @@ -336,13 +336,11 @@ func (gr *GenericReconciler) reconcileGroupOfObjects(objs []*unstructured.Unstru // allObjectsValidated checks whether all unstructured objects passed as argument are validated // and thus present in the cache -func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructured) bool { +func (gr *GenericReconciler) allObjectsValidated(objs []*unstructured.Unstructured, namespaceID string) bool { allObjectsValidated := true // we must be sure that all objects in the given group are cached (validated) // see DVO-103 for _, o := range objs { - namespaceID := gr.watchNamespaces.getNamespaceUID(o.GetNamespace()) - gr.currentObjects.store(o, namespaceID, "") if !gr.objectValidationCache.objectAlreadyValidated(o, namespaceID) { allObjectsValidated = false