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

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.IsInitialized() so it stops being added once the cluster is
up.

This makes the operator's own drift-check see the already-running
pod's spec as outdated right after initialization completes, 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 4c089ce55f
commit e95b91dee9
No known key found for this signature in database
GPG Key ID: 589F03F01BA55038
2 changed files with 56 additions and 6 deletions

View File

@ -322,6 +322,38 @@ func (impl LifecycleImplementation) collectAdditionalInstanceArgs(
return nil, nil 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.IsInitialized() instead.
//
// Once the cluster finishes initializing, 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.IsInitialized()
}
func reconcileInstancePod( func reconcileInstancePod(
ctx context.Context, ctx context.Context,
cluster *cnpgv1.Cluster, cluster *cnpgv1.Cluster,
@ -339,11 +371,7 @@ func reconcileInstancePod(
mutatedPod := pod.DeepCopy() mutatedPod := pod.DeepCopy()
// A recovery-only cluster (only RecoveryBarmanObjectName set) still needs the if shouldInjectBarmanSidecar(cluster, pluginConfiguration) {
// 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 err := reconcilePodSpec( if err := reconcilePodSpec(
cluster, cluster,
&mutatedPod.Spec, &mutatedPod.Spec,
@ -356,7 +384,7 @@ func reconcileInstancePod(
return nil, fmt.Errorf("while reconciling pod spec for pod: %w", err) return nil, fmt.Errorf("while reconciling pod spec for pod: %w", err)
} }
} else { } 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) patch, err := object.CreatePatch(mutatedPod, pod)

View File

@ -265,6 +265,28 @@ var _ = Describe("LifecycleImplementation", func() {
Expect(patch).To(ContainElement(HaveKeyWithValue("path", "/spec/initContainers"))) 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.LatestGeneratedNode = 1
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) { It("does not mutate the pod when no object store is configured", func(ctx SpecContext) {
emptyConfig := &config.PluginConfiguration{} emptyConfig := &config.PluginConfiguration{}
pod := &corev1.Pod{ pod := &corev1.Pod{