From 8b48609a8279c9634a0596596512abb7fe6acf5f Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Thu, 18 Jun 2026 14:50:01 +0200 Subject: [PATCH] fix: honor replica-source object store sidecar config for pg_basebackup replicas The sidecar injected into pg_basebackup replica clusters ignored the instanceSidecarConfiguration (resources, logLevel, additionalContainerArgs) of the replica-source ObjectStore: collectSidecarResourcesForPod and collectAdditionalInstanceArgs only handled the cluster and recovery object stores. Log-shipping replicas masked this because their recovery and replica source object stores coincide; pg_basebackup replicas only set ReplicaSourceBarmanObjectName, so the configuration was silently dropped (the sidecar ran with no additional args and the default log level). Handle ReplicaSourceBarmanObjectName in both helpers, documenting the store precedence (Barman > Recovery > ReplicaSource), and fix the misleading comment on the recovery branch. Signed-off-by: Armando Ruocco --- internal/cnpgi/operator/lifecycle.go | 20 ++- .../cnpgi/operator/lifecycle_resources.go | 24 +++- internal/cnpgi/operator/lifecycle_test.go | 130 ++++++++++++++++++ 3 files changed, 168 insertions(+), 6 deletions(-) diff --git a/internal/cnpgi/operator/lifecycle.go b/internal/cnpgi/operator/lifecycle.go index b25af55..776b301 100644 --- a/internal/cnpgi/operator/lifecycle.go +++ b/internal/cnpgi/operator/lifecycle.go @@ -273,8 +273,10 @@ func (impl LifecycleImplementation) collectAdditionalInstanceArgs( return args } - // Prefer the cluster object store (backup/archive). If not set, fallback to the recovery object store. - // If neither is configured, no additional args are provided. + // Only one object store provides the sidecar arguments, with precedence: + // BarmanObjectName (backup/archive) > RecoveryBarmanObjectName (recovery + // bootstrap) > ReplicaSourceBarmanObjectName (pg_basebackup replica). + // If none is configured, no additional args are provided. if len(pluginConfiguration.BarmanObjectName) > 0 { var barmanObjectStore barmancloudv1.ObjectStore if err := impl.Client.Get(ctx, pluginConfiguration.GetBarmanObjectKey(), &barmanObjectStore); err != nil { @@ -303,6 +305,20 @@ func (impl LifecycleImplementation) collectAdditionalInstanceArgs( return args, nil } + if len(pluginConfiguration.ReplicaSourceBarmanObjectName) > 0 { + key := pluginConfiguration.GetReplicaSourceBarmanObjectKey() + var barmanObjectStore barmancloudv1.ObjectStore + if err := impl.Client.Get(ctx, key, &barmanObjectStore); err != nil { + return nil, fmt.Errorf("while getting replica source barman object store %s: %w", key.String(), err) + } + args := barmanObjectStore.Spec.InstanceSidecarConfiguration.AdditionalContainerArgs + args = append( + args, + collectTypedAdditionalArgs(&barmanObjectStore)..., + ) + return args, nil + } + return nil, nil } diff --git a/internal/cnpgi/operator/lifecycle_resources.go b/internal/cnpgi/operator/lifecycle_resources.go index 12498ee..02b7b43 100644 --- a/internal/cnpgi/operator/lifecycle_resources.go +++ b/internal/cnpgi/operator/lifecycle_resources.go @@ -48,6 +48,9 @@ func (impl LifecycleImplementation) collectSidecarResourcesForPod( ctx context.Context, configuration *config.PluginConfiguration, ) (corev1.ResourceRequirements, error) { + // Only one object store provides the sidecar resources, with precedence: + // BarmanObjectName (backup/archive) > RecoveryBarmanObjectName (recovery + // bootstrap) > ReplicaSourceBarmanObjectName (pg_basebackup replica). if len(configuration.BarmanObjectName) > 0 { // On a replica cluster that also archives, the designated primary // will use both the replica source object store and the object store @@ -64,10 +67,10 @@ func (impl LifecycleImplementation) collectSidecarResourcesForPod( } if len(configuration.RecoveryBarmanObjectName) > 0 { - // On a replica cluster that doesn't archive, the designated primary - // uses only the replica source object store. - // In this case, we use the replica source object store for configuring - // the resources of the sidecar container. + // On a cluster recovering from an object store (including log-shipping + // replica clusters, where the recovery and replica source object stores + // coincide), we use the recovery object store for configuring the + // resources of the sidecar container. var barmanObjectStore barmancloudv1.ObjectStore if err := impl.Client.Get(ctx, configuration.GetRecoveryBarmanObjectKey(), &barmanObjectStore); err != nil { return corev1.ResourceRequirements{}, err @@ -76,5 +79,18 @@ func (impl LifecycleImplementation) collectSidecarResourcesForPod( return barmanObjectStore.Spec.InstanceSidecarConfiguration.Resources, nil } + if len(configuration.ReplicaSourceBarmanObjectName) > 0 { + // On a replica cluster bootstrapped via pg_basebackup, the designated + // primary uses only the replica source object store. + // In this case, we use the replica source object store for configuring + // the resources of the sidecar container. + var barmanObjectStore barmancloudv1.ObjectStore + if err := impl.Client.Get(ctx, configuration.GetReplicaSourceBarmanObjectKey(), &barmanObjectStore); err != nil { + return corev1.ResourceRequirements{}, err + } + + return barmanObjectStore.Spec.InstanceSidecarConfiguration.Resources, nil + } + return corev1.ResourceRequirements{}, nil } diff --git a/internal/cnpgi/operator/lifecycle_test.go b/internal/cnpgi/operator/lifecycle_test.go index d2755ed..a4851be 100644 --- a/internal/cnpgi/operator/lifecycle_test.go +++ b/internal/cnpgi/operator/lifecycle_test.go @@ -28,6 +28,7 @@ import ( barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -293,6 +294,24 @@ var _ = Describe("LifecycleImplementation", func() { Expect(args).To(Equal(recoveryArgs)) }) + It("falls back to replica source object store when primary and recovery not set", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{ + Cluster: cluster, + ReplicaSourceBarmanObjectName: "replica-source-store", + } + replicaArgs := []string{"--replica-a", "--replica-b"} + cli := buildClientFunc( + makeStoreFunc(ns, pc.ReplicaSourceBarmanObjectName, replicaArgs), + ).Build() + + impl := LifecycleImplementation{Client: cli} + args, err := impl.collectAdditionalInstanceArgs(ctx, pc) + Expect(err).NotTo(HaveOccurred()) + Expect(args).To(Equal(replicaArgs)) + }) + It("returns nil when neither object name is configured", func(ctx SpecContext) { ns := "test-ns" cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} @@ -386,6 +405,117 @@ var _ = Describe("LifecycleImplementation", func() { Expect(err).NotTo(HaveOccurred()) Expect(args).To(Equal([]string{"--log-level=info"})) }) + + It("includes --log-level from replica source object store when only replica source set", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{ + Cluster: cluster, + ReplicaSourceBarmanObjectName: "replica-source-store", + } + store := &barmancloudv1.ObjectStore{ + TypeMeta: metav1.TypeMeta{Kind: "ObjectStore", APIVersion: barmancloudv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Name: pc.ReplicaSourceBarmanObjectName, Namespace: ns}, + Spec: barmancloudv1.ObjectStoreSpec{ + InstanceSidecarConfiguration: barmancloudv1.InstanceSidecarConfiguration{ + LogLevel: "warning", + }, + }, + } + s := runtime.NewScheme() + barmancloudv1.AddKnownTypes(s) + cli := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(store).Build() + + impl := LifecycleImplementation{Client: cli} + args, err := impl.collectAdditionalInstanceArgs(ctx, pc) + Expect(err).NotTo(HaveOccurred()) + Expect(args).To(Equal([]string{"--log-level=warning"})) + }) + }) + + Describe("collectSidecarResourcesForPod", func() { + makeStoreWithResourcesFunc := func(ns, name string, res corev1.ResourceRequirements) *barmancloudv1.ObjectStore { + return &barmancloudv1.ObjectStore{ + TypeMeta: metav1.TypeMeta{Kind: "ObjectStore", APIVersion: barmancloudv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns}, + Spec: barmancloudv1.ObjectStoreSpec{ + InstanceSidecarConfiguration: barmancloudv1.InstanceSidecarConfiguration{ + Resources: res, + }, + }, + } + } + + It("uses the cluster object store resources when set", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{Cluster: cluster, BarmanObjectName: "primary-store"} + res := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("64Mi")}, + } + cli := buildClientFunc(makeStoreWithResourcesFunc(ns, pc.BarmanObjectName, res)).Build() + + impl := LifecycleImplementation{Client: cli} + got, err := impl.collectSidecarResourcesForPod(ctx, pc) + Expect(err).NotTo(HaveOccurred()) + gotMem := got.Requests[corev1.ResourceMemory] + Expect(gotMem.String()).To(Equal("64Mi")) + }) + + It("uses the recovery object store resources when only recovery is set", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{Cluster: cluster, RecoveryBarmanObjectName: "recovery-store"} + res := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("128Mi")}, + } + cli := buildClientFunc(makeStoreWithResourcesFunc(ns, pc.RecoveryBarmanObjectName, res)).Build() + + impl := LifecycleImplementation{Client: cli} + got, err := impl.collectSidecarResourcesForPod(ctx, pc) + Expect(err).NotTo(HaveOccurred()) + gotMem := got.Requests[corev1.ResourceMemory] + Expect(gotMem.String()).To(Equal("128Mi")) + }) + + It("uses the replica source object store resources when only replica source is set", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{Cluster: cluster, ReplicaSourceBarmanObjectName: "replica-source-store"} + res := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("256Mi")}, + } + cli := buildClientFunc(makeStoreWithResourcesFunc(ns, pc.ReplicaSourceBarmanObjectName, res)).Build() + + impl := LifecycleImplementation{Client: cli} + got, err := impl.collectSidecarResourcesForPod(ctx, pc) + Expect(err).NotTo(HaveOccurred()) + gotMem := got.Requests[corev1.ResourceMemory] + Expect(gotMem.String()).To(Equal("256Mi")) + }) + + It("returns empty resources when no object store is configured", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{Cluster: cluster} + cli := buildClientFunc().Build() + + impl := LifecycleImplementation{Client: cli} + got, err := impl.collectSidecarResourcesForPod(ctx, pc) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(corev1.ResourceRequirements{})) + }) + + It("returns an error if the replica source object store cannot be retrieved", func(ctx SpecContext) { + ns := "test-ns" + cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}} + pc := &config.PluginConfiguration{Cluster: cluster, ReplicaSourceBarmanObjectName: "missing-store"} + cli := buildClientFunc().Build() + + impl := LifecycleImplementation{Client: cli} + _, err := impl.collectSidecarResourcesForPod(ctx, pc) + Expect(err).To(HaveOccurred()) + }) }) })