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

fix(object-cache): improve reliability of object cache management (#508)

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>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
This commit is contained in:
Marco Nenciarini 2025-09-12 11:31:59 +02:00 committed by GitHub
parent 3c0d8c3a33
commit 8c3db955ef
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 64 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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
}

78
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,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"))
})
})