marco/plugin-barman-cloud
1
0
Fork 0
mirror of https://github.com/cloudnative-pg/plugin-barman-cloud.git synced 2026-09-04 22:22:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: stop keeping the sidecar forever on recovery-only clusters

Browse Source 
A cluster bootstrapped only via RecoveryBarmanObjectName, with no
continuing archiving or replica source, only ever needs the sidecar
for its one-time bootstrap restore. Gate its injection on
cluster.Status.CurrentPrimary being unset, so it stops being added
once that instance is up.

CurrentPrimary is set by the instance manager itself, from inside the
primary pod, only once it has completed its own bootstrap - unlike
cluster.Status.Instances / IsInitialized(), which flips as soon as the
instance's PVC exists, well before the pod is even created. Gating on
IsInitialized() would mean the sidecar is never added to the one pod
that needs it to perform its restore.

Once CurrentPrimary is set, this makes the operator's own drift-check
see the already-running pod's spec as outdated and roll it out to drop
the sidecar. That is deliberately accepted rather than engineered
around: it is one deterministic rollout using the same machinery the
operator already uses for every other pod-spec change (a switchover if
a replica is available, an in-place restart otherwise), not a new or
fragile risk.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
This commit is contained in:
Marco Nenciarini 2026-07-27 17:47:05 +02:00
parent 4f5f259ee7
commit fb3397386c
2 changed files with 63 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
internal/cnpgi/operator/lifecycle.go
View File

@ -322,6 +322,45 @@ func (impl LifecycleImplementation) collectAdditionalInstanceArgs(
return nil, nil
}
// shouldInjectBarmanSidecar decides whether an instance pod needs the
// plugin-barman-cloud sidecar.
//
// A cluster doing backup/archiving or serving as a replica source needs the
// sidecar in every instance pod for as long as the cluster exists, so those
// two cases always inject it. A recovery-only cluster (only
// RecoveryBarmanObjectName set, mirroring what pluginConfiguration.Validate()
// accepts) only ever needs the sidecar for its one-time bootstrap restore, so
// it's gated on cluster.Status.CurrentPrimary instead.
//
// CurrentPrimary is set by the instance manager itself, from inside the
// primary pod, only once it's up and has completed its own bootstrap (see
// instance_startup.go in cloudnative-pg) - unlike cluster.Status.Instances /
// IsInitialized(), which flips as soon as the instance's PVC exists, well
// before the pod is even created. Using IsInitialized() here would mean the
// sidecar is never added to the one pod that needs it to perform its restore.
//
// Once CurrentPrimary is set, this makes the operator's own drift-check
// (checkPodSpecIsOutdated) see the running pod's spec as outdated and roll it
// out to drop the sidecar. That's deliberately accepted rather than
// engineered around: it's one deterministic rollout using the same machinery
// the operator already uses for every other pod-spec change (a switchover if
// a replica is available, an in-place restart otherwise), not a new or
// fragile risk.
func shouldInjectBarmanSidecar(
cluster *cnpgv1.Cluster,
pluginConfiguration *config.PluginConfiguration,
) bool {
if len(pluginConfiguration.BarmanObjectName) != 0 || len(pluginConfiguration.ReplicaSourceBarmanObjectName) != 0 {
return true
}
if len(pluginConfiguration.RecoveryBarmanObjectName) == 0 {
return false
}
return cluster.Status.CurrentPrimary == ""
}
func reconcileInstancePod(
ctx context.Context,
cluster *cnpgv1.Cluster,
@ -339,11 +378,7 @@ func reconcileInstancePod(
mutatedPod := pod.DeepCopy()
// A recovery-only cluster (only RecoveryBarmanObjectName set) still needs the
// sidecar in its instance pods: the phase-0 bootstrap restore and the WAL
// replay that follows both run inside the instance and rely on it. This
// condition therefore mirrors what pluginConfiguration.Validate() accepts.
if pluginConfiguration.HasAnyBarmanObjectStore() {
if shouldInjectBarmanSidecar(cluster, pluginConfiguration) {
if err := reconcilePodSpec(
cluster,
&mutatedPod.Spec,
@ -356,7 +391,7 @@ func reconcileInstancePod(
return nil, fmt.Errorf("while reconciling pod spec for pod: %w", err)
}
} else {
contextLogger.Debug("No need to mutate instance with no barman object store configuration")
contextLogger.Debug("No need to mutate instance, sidecar not required for this configuration and pod")
}
patch, err := object.CreatePatch(mutatedPod, pod)

22
internal/cnpgi/operator/lifecycle_test.go
View File

@ -265,6 +265,28 @@ var _ = Describe("LifecycleImplementation", func() {
Expect(patch).To(ContainElement(HaveKeyWithValue("path", "/spec/initContainers")))
})
It("does not inject the sidecar for a recovery-only cluster that has "+
"already completed its initial bootstrap", func(ctx SpecContext) {
recoveryOnlyConfig := &config.PluginConfiguration{
RecoveryBarmanObjectName: "minio-store-recovery",
}
cluster.Status.CurrentPrimary = "test-pod"
pod := &corev1.Pod{
TypeMeta: podTypeMeta,
ObjectMeta: metav1.ObjectMeta{Name: "test-pod"},
Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "postgres"}}},
}
podJSON, _ := json.Marshal(pod)
request := &lifecycle.OperatorLifecycleRequest{
ObjectDefinition: podJSON,
}
response, err := reconcileInstancePod(ctx, cluster, request, recoveryOnlyConfig, sidecarConfiguration{})
Expect(err).NotTo(HaveOccurred())
Expect(response).NotTo(BeNil())
Expect(response.JsonPatch).To(BeEmpty())
})
It("does not mutate the pod when no object store is configured", func(ctx SpecContext) {
emptyConfig := &config.PluginConfiguration{}
pod := &corev1.Pod{