diff --git a/internal/cnpgi/operator/lifecycle.go b/internal/cnpgi/operator/lifecycle.go index 8938f63..04e3c9a 100644 --- a/internal/cnpgi/operator/lifecycle.go +++ b/internal/cnpgi/operator/lifecycle.go @@ -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) diff --git a/internal/cnpgi/operator/lifecycle_test.go b/internal/cnpgi/operator/lifecycle_test.go index fb605a7..2aedf44 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.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{