diff --git a/internal/cnpgi/instance/internal/client/client.go b/internal/cnpgi/instance/internal/client/client.go index e634532..290e132 100644 --- a/internal/cnpgi/instance/internal/client/client.go +++ b/internal/cnpgi/instance/internal/client/client.go @@ -9,7 +9,6 @@ import ( "github.com/cloudnative-pg/machinery/pkg/log" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" pluginBarman "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" @@ -21,11 +20,11 @@ const DefaultTTLSeconds = 10 type cachedEntry struct { entry client.Object fetchUnixTime int64 - ttl time.Duration + ttlSeconds int64 } func (e *cachedEntry) isExpired() bool { - return time.Now().Unix()-e.fetchUnixTime > int64(e.ttl) + return time.Now().Unix()-e.fetchUnixTime > e.ttlSeconds } // ExtendedClient is an extended client that is capable of caching multiple secrets without relying on informers @@ -91,7 +90,7 @@ func (e *ExtendedClient) getCachedObject( if cacheEntry.entry.GetNamespace() != key.Namespace || cacheEntry.entry.GetName() != key.Name { continue } - if cacheEntry.entry.GetObjectKind().GroupVersionKind() != obj.GetObjectKind().GroupVersionKind() { + if reflect.TypeOf(cacheEntry.entry) != reflect.TypeOf(obj) { continue } if cacheEntry.isExpired() { @@ -121,8 +120,9 @@ func (e *ExtendedClient) getCachedObject( } cs := cachedEntry{ - entry: obj.(runtime.Object).DeepCopyObject().(client.Object), + entry: obj.DeepCopyObject().(client.Object), fetchUnixTime: time.Now().Unix(), + ttlSeconds: DefaultTTLSeconds, } contextLogger.Debug("setting object in the cache") @@ -143,7 +143,7 @@ func (e *ExtendedClient) removeObject(object client.Object) { for i, cache := range e.cachedObjects { if cache.entry.GetNamespace() == object.GetNamespace() && cache.entry.GetName() == object.GetName() && - cache.entry.GetObjectKind().GroupVersionKind() != object.GetObjectKind().GroupVersionKind() { + reflect.TypeOf(cache.entry) == reflect.TypeOf(object) { e.cachedObjects = append(e.cachedObjects[:i], e.cachedObjects[i+1:]...) return } diff --git a/internal/cnpgi/instance/internal/client/client_test.go b/internal/cnpgi/instance/internal/client/client_test.go index 70568f4..ee84aba 100644 --- a/internal/cnpgi/instance/internal/client/client_test.go +++ b/internal/cnpgi/instance/internal/client/client_test.go @@ -9,7 +9,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - v1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" + barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -20,16 +20,26 @@ var scheme = buildScheme() func buildScheme() *runtime.Scheme { scheme := runtime.NewScheme() _ = corev1.AddToScheme(scheme) - _ = v1.AddToScheme(scheme) + _ = barmancloudv1.AddToScheme(scheme) return scheme } +func addToCache(c *ExtendedClient, obj client.Object, fetchUnixTime int64) { + ce := cachedEntry{ + entry: obj.DeepCopyObject().(client.Object), + fetchUnixTime: fetchUnixTime, + ttlSeconds: DefaultTTLSeconds, + } + ce.entry.SetResourceVersion("from cache") + c.cachedObjects = append(c.cachedObjects, ce) +} + var _ = Describe("ExtendedClient Get", func() { var ( extendedClient *ExtendedClient secretInClient *corev1.Secret - objectStore *v1.ObjectStore + objectStore *barmancloudv1.ObjectStore ) BeforeEach(func() { @@ -39,12 +49,12 @@ var _ = Describe("ExtendedClient Get", func() { Name: "test-secret", }, } - objectStore = &v1.ObjectStore{ + objectStore = &barmancloudv1.ObjectStore{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "test-object-store", }, - Spec: v1.ObjectStoreSpec{}, + Spec: barmancloudv1.ObjectStoreSpec{}, } baseClient := fake.NewClientBuilder(). @@ -61,35 +71,34 @@ var _ = Describe("ExtendedClient Get", func() { }, } - // manually add the secret to the cache, this is not present in the fake client so we are sure it is from the - // cache - extendedClient.cachedObjects = []cachedEntry{ - { - entry: secretNotInClient, - fetchUnixTime: time.Now().Unix(), - }, - } + // manually add the secret to the cache, this is not present in the fake client, + // so we are sure it is from the cache + addToCache(extendedClient, secretNotInClient, time.Now().Unix()) err := extendedClient.Get(ctx, client.ObjectKeyFromObject(secretNotInClient), secretInClient) Expect(err).NotTo(HaveOccurred()) - Expect(secretNotInClient).To(Equal(extendedClient.cachedObjects[0].entry)) + Expect(secretInClient).To(Equal(extendedClient.cachedObjects[0].entry)) + Expect(secretInClient.GetResourceVersion()).To(Equal("from cache")) }) It("fetches secret from base client if cache is expired", func(ctx SpecContext) { - extendedClient.cachedObjects = []cachedEntry{ - { - entry: secretInClient.DeepCopy(), - fetchUnixTime: time.Now().Add(-2 * time.Minute).Unix(), - }, - } + addToCache(extendedClient, secretInClient, time.Now().Add(-2*time.Minute).Unix()) err := extendedClient.Get(ctx, client.ObjectKeyFromObject(secretInClient), secretInClient) Expect(err).NotTo(HaveOccurred()) + Expect(secretInClient.GetResourceVersion()).NotTo(Equal("from cache")) + + // the cache is updated with the new value + Expect(extendedClient.cachedObjects).To(HaveLen(1)) + Expect(extendedClient.cachedObjects[0].entry.GetResourceVersion()).NotTo(Equal("from cache")) }) It("fetches secret from base client if not in cache", func(ctx SpecContext) { err := extendedClient.Get(ctx, client.ObjectKeyFromObject(secretInClient), secretInClient) Expect(err).NotTo(HaveOccurred()) + + // the cache is updated with the new value + Expect(extendedClient.cachedObjects).To(HaveLen(1)) }) It("does not cache non-secret objects", func(ctx SpecContext) { @@ -106,4 +115,33 @@ var _ = Describe("ExtendedClient Get", func() { Expect(err).NotTo(HaveOccurred()) Expect(extendedClient.cachedObjects).To(BeEmpty()) }) + + It("returns the correct object from cache when multiple objects with the same object key are cached", + func(ctx SpecContext) { + secretNotInClient := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "common-name", + }, + } + objectStoreNotInClient := &barmancloudv1.ObjectStore{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "common-name", + }, + } + + // manually add the objects to the cache, these are not present in the fake client, + // so we are sure they are from the cache + addToCache(extendedClient, secretNotInClient, time.Now().Unix()) + addToCache(extendedClient, objectStoreNotInClient, time.Now().Unix()) + + err := extendedClient.Get(ctx, client.ObjectKeyFromObject(secretNotInClient), secretInClient) + Expect(err).NotTo(HaveOccurred()) + err = extendedClient.Get(ctx, client.ObjectKeyFromObject(objectStoreNotInClient), objectStore) + Expect(err).NotTo(HaveOccurred()) + + Expect(secretInClient.GetResourceVersion()).To(Equal("from cache")) + Expect(objectStore.GetResourceVersion()).To(Equal("from cache")) + }) })