Merge 9fd7ba5b5e into a95197e648

The current configuration derives the sidecar image from configuration paramter `sidecar-image`: 1e13020fe5/internal/cnpgi/operator/lifecycle.go (L418)

However, this is challenging for multi-tenant Kubernetes clusters, as the plugin deployment defines the target sidecar image for all CNPG clusters within a given Kubernetes cluster.

In this PR, we add an optional `sidecarImage` field to `ObjectStore.spec.instanceSidecarConfiguration`.

This allows workloads using a specific ObjectStore to override the Barman Cloud sidecar image configured globally on the plugin deployment.

Signed-off-by: Chandon Pierre <cpierre@coreweave.com>

api/v1/objectstore_types.go

@@ -27,6 +27,11 @@ import (
 
 // InstanceSidecarConfiguration defines the configuration for the sidecar that runs in the instance pods.
 type InstanceSidecarConfiguration struct {
+	// SidecarImage overrides the plugin sidecar image for workloads that use this ObjectStore.
+	// When omitted, the image configured on the plugin deployment is used.
+	// +optional
+	SidecarImage string `json:"sidecarImage,omitempty"`
+
 	// The environment to be explicitly passed to the sidecar
 	// +optional
 	Env []corev1.EnvVar `json:"env,omitempty"`

config/crd/bases/barmancloud.cnpg.io_objectstores.yaml

@@ -666,6 +666,11 @@ spec:
                       The retentionCheckInterval defines the frequency at which the
                       system checks and enforces retention policies.
                     type: integer
+                  sidecarImage:
+                    description: |-
+                      SidecarImage overrides the plugin sidecar image for workloads that use this ObjectStore.
+                      When omitted, the image configured on the plugin deployment is used.
+                    type: string
                 type: object
               retentionPolicy:
                 description: |-

internal/cnpgi/operator/lifecycle.go

@@ -150,10 +150,16 @@ func (impl LifecycleImplementation) reconcileJob(
 		return nil, err
 	}
 
+	image, err := impl.collectSidecarImageForRecoveryJob(ctx, pluginConfiguration)
+	if err != nil {
+		return nil, err
+	}
+
 	return reconcileJob(ctx, cluster, request, sidecarConfiguration{
 		env:          env,
 		certificates: certificates,
 		resources:    resources,
+		image:        image,
 	})
 }
 
@@ -162,6 +168,7 @@ type sidecarConfiguration struct {
 	certificates   []corev1.VolumeProjection
 	resources      corev1.ResourceRequirements
 	additionalArgs []string
+	image          string
 }
 
 func reconcileJob(
@@ -248,11 +255,17 @@ func (impl LifecycleImplementation) reconcilePod(
 		return nil, err
 	}
 
+	image, err := impl.collectSidecarImageForPod(ctx, pluginConfiguration)
+	if err != nil {
+		return nil, err
+	}
+
 	return reconcileInstancePod(ctx, cluster, request, pluginConfiguration, sidecarConfiguration{
 		env:            env,
 		certificates:   certificates,
 		resources:      resources,
 		additionalArgs: additionalArgs,
+		image:          image,
 	})
 }
 
@@ -450,7 +463,10 @@ func reconcilePodSpec(
 
 	// fixed values
 	sidecarTemplate.Name = "plugin-barman-cloud"
-	sidecarTemplate.Image = viper.GetString("sidecar-image")
+	sidecarTemplate.Image = config.image
+	if sidecarTemplate.Image == "" {
+		sidecarTemplate.Image = viper.GetString("sidecar-image")
+	}
 	sidecarTemplate.ImagePullPolicy = cluster.Spec.ImagePullPolicy
 	sidecarTemplate.StartupProbe = baseProbe.DeepCopy()
 	sidecarTemplate.SecurityContext = &corev1.SecurityContext{

internal/cnpgi/operator/lifecycle_image.go

@@ -0,0 +1,75 @@
+/*
+Copyright © contributors to CloudNativePG, established as
+CloudNativePG a Series of LF Projects, LLC.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package operator
+
+import (
+	"context"
+
+	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+	"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config"
+)
+
+func (impl LifecycleImplementation) collectSidecarImageForRecoveryJob(
+	ctx context.Context,
+	configuration *config.PluginConfiguration,
+) (string, error) {
+	if len(configuration.RecoveryBarmanObjectName) == 0 {
+		return "", nil
+	}
+
+	var objectStore barmancloudv1.ObjectStore
+	if err := impl.Client.Get(ctx, configuration.GetRecoveryBarmanObjectKey(), &objectStore); err != nil {
+		return "", err
+	}
+
+	return objectStore.Spec.InstanceSidecarConfiguration.SidecarImage, nil
+}
+
+func (impl LifecycleImplementation) collectSidecarImageForPod(
+	ctx context.Context,
+	configuration *config.PluginConfiguration,
+) (string, error) {
+	// Keep the same precedence used for sidecar resources and arguments.
+	switch {
+	case len(configuration.BarmanObjectName) > 0:
+		var objectStore barmancloudv1.ObjectStore
+		if err := impl.Client.Get(ctx, configuration.GetBarmanObjectKey(), &objectStore); err != nil {
+			return "", err
+		}
+		return objectStore.Spec.InstanceSidecarConfiguration.SidecarImage, nil
+
+	case len(configuration.RecoveryBarmanObjectName) > 0:
+		var objectStore barmancloudv1.ObjectStore
+		if err := impl.Client.Get(ctx, configuration.GetRecoveryBarmanObjectKey(), &objectStore); err != nil {
+			return "", err
+		}
+		return objectStore.Spec.InstanceSidecarConfiguration.SidecarImage, nil
+
+	case len(configuration.ReplicaSourceBarmanObjectName) > 0:
+		var objectStore barmancloudv1.ObjectStore
+		if err := impl.Client.Get(ctx, configuration.GetReplicaSourceBarmanObjectKey(), &objectStore); err != nil {
+			return "", err
+		}
+		return objectStore.Spec.InstanceSidecarConfiguration.SidecarImage, nil
+
+	default:
+		return "", nil
+	}
+}

internal/cnpgi/operator/lifecycle_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
 	"github.com/cloudnative-pg/cnpg-i/pkg/lifecycle"
 	barmancloudv1 "github.com/cloudnative-pg/plugin-barman-cloud/api/v1"
+	"github.com/spf13/viper"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -580,6 +581,101 @@ var _ = Describe("LifecycleImplementation", func() {
 			Expect(err).To(HaveOccurred())
 		})
 	})
+
+	Describe("collectSidecarImage", func() {
+		makeStoreWithImageFunc := func(ns, name, image string) *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{
+						SidecarImage: image,
+					},
+				},
+			}
+		}
+
+		It("uses the cluster object store image when multiple stores are configured", func(ctx SpecContext) {
+			ns := "test-ns"
+			cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}}
+			pc := &config.PluginConfiguration{
+				Cluster:                       cluster,
+				BarmanObjectName:              "primary-store",
+				RecoveryBarmanObjectName:      "recovery-store",
+				ReplicaSourceBarmanObjectName: "replica-store",
+			}
+			cli := buildClientFunc(
+				makeStoreWithImageFunc(ns, pc.BarmanObjectName, "example.com/primary:v1"),
+				makeStoreWithImageFunc(ns, pc.RecoveryBarmanObjectName, "example.com/recovery:v1"),
+				makeStoreWithImageFunc(ns, pc.ReplicaSourceBarmanObjectName, "example.com/replica:v1"),
+			).Build()
+
+			impl := LifecycleImplementation{Client: cli}
+			image, err := impl.collectSidecarImageForPod(ctx, pc)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(image).To(Equal("example.com/primary:v1"))
+		})
+
+		It("uses the recovery object store image for recovery jobs", func(ctx SpecContext) {
+			ns := "test-ns"
+			cluster := &cnpgv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: ns}}
+			pc := &config.PluginConfiguration{
+				Cluster:                  cluster,
+				RecoveryBarmanObjectName: "recovery-store",
+			}
+			cli := buildClientFunc(
+				makeStoreWithImageFunc(ns, pc.RecoveryBarmanObjectName, "example.com/recovery:v1"),
+			).Build()
+
+			impl := LifecycleImplementation{Client: cli}
+			image, err := impl.collectSidecarImageForRecoveryJob(ctx, pc)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(image).To(Equal("example.com/recovery:v1"))
+		})
+
+		It("returns an empty override when no object store is configured", func(ctx SpecContext) {
+			pc := &config.PluginConfiguration{Cluster: &cnpgv1.Cluster{}}
+			impl := LifecycleImplementation{Client: buildClientFunc().Build()}
+
+			image, err := impl.collectSidecarImageForPod(ctx, pc)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(image).To(BeEmpty())
+		})
+	})
+
+	Describe("sidecar image selection", func() {
+		It("prefers the ObjectStore image override", func() {
+			spec := corev1.PodSpec{Containers: []corev1.Container{{Name: "postgres"}}}
+			err := reconcilePodSpec(
+				cluster,
+				&spec,
+				"postgres",
+				corev1.Container{Args: []string{"instance"}},
+				sidecarConfiguration{image: "example.com/override:v1"},
+			)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(spec.InitContainers).To(HaveLen(1))
+			Expect(spec.InitContainers[0].Image).To(Equal("example.com/override:v1"))
+		})
+
+		It("falls back to the deployment sidecar image", func() {
+			previousImage := viper.GetString("sidecar-image")
+			viper.Set("sidecar-image", "example.com/global:v1")
+			DeferCleanup(viper.Set, "sidecar-image", previousImage)
+
+			spec := corev1.PodSpec{Containers: []corev1.Container{{Name: "postgres"}}}
+			err := reconcilePodSpec(
+				cluster,
+				&spec,
+				"postgres",
+				corev1.Container{Args: []string{"instance"}},
+				sidecarConfiguration{},
+			)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(spec.InitContainers).To(HaveLen(1))
+			Expect(spec.InitContainers[0].Image).To(Equal("example.com/global:v1"))
+		})
+	})
 })
 
 var _ = Describe("Volume utilities", func() {