diff --git a/internal/cnpgi/operator/lifecycle.go b/internal/cnpgi/operator/lifecycle.go index 8938f63..9d8e8fe 100644 --- a/internal/cnpgi/operator/lifecycle.go +++ b/internal/cnpgi/operator/lifecycle.go @@ -322,6 +322,38 @@ 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.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( ctx context.Context, cluster *cnpgv1.Cluster, @@ -339,11 +371,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 +384,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) diff --git a/internal/cnpgi/operator/lifecycle_test.go b/internal/cnpgi/operator/lifecycle_test.go index fb605a7..d21ea93 100644 --- a/internal/cnpgi/operator/lifecycle_test.go +++ b/internal/cnpgi/operator/lifecycle_test.go @@ -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.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) { emptyConfig := &config.PluginConfiguration{} pod := &corev1.Pod{