From 0f05e0cc7d83abae8a5b2ccf2ec7416d42b3ecaf Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Tue, 27 May 2025 09:39:01 +0200 Subject: [PATCH] fix: do not add barman-certificates projection if not needed Closes #347 Signed-off-by: Marco Nenciarini --- internal/cnpgi/operator/lifecycle.go | 75 +++++++++++++---------- internal/cnpgi/operator/lifecycle_test.go | 41 +++++++++++++ 2 files changed, 84 insertions(+), 32 deletions(-) diff --git a/internal/cnpgi/operator/lifecycle.go b/internal/cnpgi/operator/lifecycle.go index 308c9e7..406332d 100644 --- a/internal/cnpgi/operator/lifecycle.go +++ b/internal/cnpgi/operator/lifecycle.go @@ -333,6 +333,7 @@ func reconcilePodSpec( Drop: []corev1.Capability{"ALL"}, }, } + sidecarTemplate.RestartPolicy = ptr.To(corev1.ContainerRestartPolicyAlways) sidecarTemplate.Resources = config.resources // merge the main container envs if they aren't already set @@ -368,13 +369,15 @@ func reconcilePodSpec( } } - if err := injectPluginSidecarPodSpec(spec, &sidecarTemplate, mainContainerName); err != nil { - return err - } + if len(config.certificates) > 0 { + sidecarTemplate.VolumeMounts = append( + sidecarTemplate.VolumeMounts, + corev1.VolumeMount{ + Name: barmanCertificatesVolumeName, + MountPath: metadata.BarmanCertificatesPath, + }) - // inject the volume containing the certificates if needed - if !volumeListHasVolume(spec.Volumes, barmanCertificatesVolumeName) { - spec.Volumes = append(spec.Volumes, corev1.Volume{ + spec.Volumes = volumeListEnsureVolume(spec.Volumes, corev1.Volume{ Name: barmanCertificatesVolumeName, VolumeSource: corev1.VolumeSource{ Projected: &corev1.ProjectedVolumeSource{ @@ -382,6 +385,12 @@ func reconcilePodSpec( }, }, }) + } else { + spec.Volumes = volumeListRemoveVolume(spec.Volumes, barmanCertificatesVolumeName) + } + + if err := injectPluginSidecarPodSpec(spec, &sidecarTemplate, mainContainerName); err != nil { + return err } return nil @@ -428,10 +437,6 @@ func InjectPluginVolumePodSpec(spec *corev1.PodSpec, mainContainerName string) { } // injectPluginSidecarPodSpec injects a plugin sidecar into a CNPG Pod spec. -// -// If the "injectMainContainerVolumes" flag is true, this will append all the volume -// mounts that are used in the instance manager Pod to the passed sidecar -// container, granting it superuser access to the PostgreSQL instance. func injectPluginSidecarPodSpec( spec *corev1.PodSpec, sidecar *corev1.Container, @@ -440,12 +445,11 @@ func injectPluginSidecarPodSpec( sidecar = sidecar.DeepCopy() InjectPluginVolumePodSpec(spec, mainContainerName) - var volumeMounts []corev1.VolumeMount sidecarContainerFound := false mainContainerFound := false for i := range spec.Containers { if spec.Containers[i].Name == mainContainerName { - volumeMounts = spec.Containers[i].VolumeMounts + sidecar.VolumeMounts = append(sidecar.VolumeMounts, spec.Containers[i].VolumeMounts...) mainContainerFound = true } } @@ -457,38 +461,45 @@ func injectPluginSidecarPodSpec( for i := range spec.InitContainers { if spec.InitContainers[i].Name == sidecar.Name { sidecarContainerFound = true + spec.InitContainers[i] = *sidecar } } - if sidecarContainerFound { - // The sidecar container was already added - return nil + if !sidecarContainerFound { + spec.InitContainers = append(spec.InitContainers, *sidecar) } - // Do not modify the passed sidecar definition - sidecar.VolumeMounts = append( - sidecar.VolumeMounts, - corev1.VolumeMount{ - Name: barmanCertificatesVolumeName, - MountPath: metadata.BarmanCertificatesPath, - }) - sidecar.VolumeMounts = append(sidecar.VolumeMounts, volumeMounts...) - sidecar.RestartPolicy = ptr.To(corev1.ContainerRestartPolicyAlways) - spec.InitContainers = append(spec.InitContainers, *sidecar) - return nil } -// volumeListHasVolume check if a volume with a known name exists -// in the volume list -func volumeListHasVolume(volumes []corev1.Volume, name string) bool { +// volumeListEnsureVolume makes sure the passed volume is present in the list of volumes. +// If the volume is already present, it is updated. +func volumeListEnsureVolume(volumes []corev1.Volume, volume corev1.Volume) []corev1.Volume { + volumeFound := false for i := range volumes { - if volumes[i].Name == name { - return true + if volumes[i].Name == volume.Name { + volumeFound = true + volumes[i] = volume } } - return false + if !volumeFound { + volumes = append(volumes, volume) + } + + return volumes +} + +// volumeListRemoveVolume removes a volume with a known name from a list of volumes. +func volumeListRemoveVolume(volumes []corev1.Volume, name string) []corev1.Volume { + j := 0 + for _, volume := range volumes { + if volume.Name != name { + volumes[j] = volume + j++ + } + } + return volumes[:j] } // getCNPGJobRole gets the role associated to a CNPG job diff --git a/internal/cnpgi/operator/lifecycle_test.go b/internal/cnpgi/operator/lifecycle_test.go index 3d94a3b..40c20ed 100644 --- a/internal/cnpgi/operator/lifecycle_test.go +++ b/internal/cnpgi/operator/lifecycle_test.go @@ -209,3 +209,44 @@ var _ = Describe("LifecycleImplementation", func() { }) }) }) + +var _ = Describe("Volume utilities", func() { + Describe("volumeListEnsureVolume", func() { + It("adds a new volume if not present", func() { + volumes := []corev1.Volume{{Name: "vol1"}} + newVol := corev1.Volume{Name: "vol2"} + result := volumeListEnsureVolume(volumes, newVol) + Expect(result).To(HaveLen(2)) + Expect(result[1]).To(Equal(newVol)) + }) + + It("updates an existing volume", func() { + volumes := []corev1.Volume{{Name: "vol1"}} + updatedVol := corev1.Volume{Name: "vol1", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}} + result := volumeListEnsureVolume(volumes, updatedVol) + Expect(result).To(HaveLen(1)) + Expect(result[0]).To(Equal(updatedVol)) + }) + }) + + Describe("volumeListRemoveVolume", func() { + It("removes the specified volume", func() { + volumes := []corev1.Volume{ + {Name: "vol1"}, + {Name: "vol2"}, + {Name: "vol3"}, + } + result := volumeListRemoveVolume(volumes, "vol2") + Expect(result).To(HaveLen(2)) + for _, v := range result { + Expect(v.Name).NotTo(Equal("vol2")) + } + }) + + It("returns the same list if the volume is not present", func() { + volumes := []corev1.Volume{{Name: "vol1"}, {Name: "vol2"}} + result := volumeListRemoveVolume(volumes, "vol3") + Expect(result).To(Equal(volumes)) + }) + }) +})