marco/plugin-barman-cloud
1
0
Fork 0
mirror of https://github.com/cloudnative-pg/plugin-barman-cloud.git synced 2026-01-11 21:23:12 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: improve object cache reliability

Browse Source 
* make sure objects expire after DefaultTTLSeconds
* make cached objects have GKV information
* fix cache retrieval and removal logic

Closes #502

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
This commit is contained in:
Marco Nenciarini 2025-09-02 19:15:30 +02:00 committed by Leonardo Cecchi
parent 3c0d8c3a33
commit 0f4b8a70de
2 changed files with 96 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
internal/cnpgi/instance/internal/client/client.go
View File

@ -21,11 +21,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
@ -71,6 +71,28 @@ func (e *ExtendedClient) Get(
return e.Client.Get(ctx, key, obj, opts...)
}
// addTypeInformationToObject adds TypeMeta information to a client.Object based upon the client Scheme
// inspired by: https://github.com/kubernetes/cli-runtime/blob/v0.19.2/pkg/printers/typesetter.go#L41
func (e *ExtendedClient) addTypeInformationToObject(obj client.Object) error {
gvks, _, err := e.Client.Scheme().ObjectKinds(obj)
if err != nil {
return fmt.Errorf("missing apiVersion or kind and cannot assign it; %w", err)
}
for _, gvk := range gvks {
if len(gvk.Kind) == 0 {
continue
}
if len(gvk.Version) == 0 || gvk.Version == runtime.APIVersionInternal {
continue
}
obj.GetObjectKind().SetGroupVersionKind(gvk)
break
}
return nil
}
func (e *ExtendedClient) getCachedObject(
ctx context.Context,
key client.ObjectKey,
@ -81,6 +103,12 @@ func (e *ExtendedClient) getCachedObject(
WithName("extended_client").
WithValues("name", key.Name, "namespace", key.Namespace)
// Make sure the object has GVK information
// This is needed to compare the object type with the cached one
if err := e.addTypeInformationToObject(obj); err != nil {
return fmt.Errorf("cannot add type metadata to object of type %T: %w", obj, err)
}
contextLogger.Trace("locking the cache")
e.mux.Lock()
defer e.mux.Unlock()
@ -120,9 +148,16 @@ func (e *ExtendedClient) getCachedObject(
return err
}
// Populate the GKV information again, as the client.Get() may have
// returned an object without this information set
if err := e.addTypeInformationToObject(obj); err != nil {
return fmt.Errorf("cannot add type metadata to object of type %T: %w", obj, err)
}
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 +178,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() {
cache.entry.GetObjectKind().GroupVersionKind() == object.GetObjectKind().GroupVersionKind() {
e.cachedObjects = append(e.cachedObjects[:i], e.cachedObjects[i+1:]...)
return
}

77
internal/cnpgi/instance/internal/client/client_test.go
View File

@ -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,28 @@ 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")
err := c.addTypeInformationToObject(ce.entry)
Expect(err).ToNot(HaveOccurred())
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 +51,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 +73,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 +117,30 @@ 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())
})
})