From a504ce06d0ee8288809872c5c0224e14fbf60043 Mon Sep 17 00:00:00 2001 From: Armando Ruocco Date: Tue, 27 May 2025 15:28:36 +0200 Subject: [PATCH] chore: review Signed-off-by: Armando Ruocco --- internal/cnpgi/operator/lifecycle.go | 30 +++++++++++------ internal/cnpgi/operator/lifecycle_test.go | 41 +++++++++++++++++++---- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/internal/cnpgi/operator/lifecycle.go b/internal/cnpgi/operator/lifecycle.go index 406332d..055b99a 100644 --- a/internal/cnpgi/operator/lifecycle.go +++ b/internal/cnpgi/operator/lifecycle.go @@ -377,7 +377,7 @@ func reconcilePodSpec( MountPath: metadata.BarmanCertificatesPath, }) - spec.Volumes = volumeListEnsureVolume(spec.Volumes, corev1.Volume{ + spec.Volumes = ensureVolume(spec.Volumes, corev1.Volume{ Name: barmanCertificatesVolumeName, VolumeSource: corev1.VolumeSource{ Projected: &corev1.ProjectedVolumeSource{ @@ -386,7 +386,8 @@ func reconcilePodSpec( }, }) } else { - spec.Volumes = volumeListRemoveVolume(spec.Volumes, barmanCertificatesVolumeName) + sidecarTemplate.VolumeMounts = removeVolumeMount(sidecarTemplate.VolumeMounts, barmanCertificatesVolumeName) + spec.Volumes = removeVolume(spec.Volumes, barmanCertificatesVolumeName) } if err := injectPluginSidecarPodSpec(spec, &sidecarTemplate, mainContainerName); err != nil { @@ -472,9 +473,9 @@ func injectPluginSidecarPodSpec( return nil } -// volumeListEnsureVolume makes sure the passed volume is present in the list of volumes. +// ensureVolume 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 { +func ensureVolume(volumes []corev1.Volume, volume corev1.Volume) []corev1.Volume { volumeFound := false for i := range volumes { if volumes[i].Name == volume.Name { @@ -490,16 +491,25 @@ func volumeListEnsureVolume(volumes []corev1.Volume, volume corev1.Volume) []cor 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 +// removeVolume removes a volume with a known name from a list of volumes. +func removeVolume(volumes []corev1.Volume, name string) []corev1.Volume { + var filteredVolumes []corev1.Volume for _, volume := range volumes { if volume.Name != name { - volumes[j] = volume - j++ + filteredVolumes = append(filteredVolumes, volume) } } - return volumes[:j] + return filteredVolumes +} + +func removeVolumeMount(mounts []corev1.VolumeMount, name string) []corev1.VolumeMount { + var filteredMounts []corev1.VolumeMount + for _, mount := range mounts { + if mount.Name != name { + filteredMounts = append(filteredMounts, mount) + } + } + return filteredMounts } // 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 40c20ed..4f9877e 100644 --- a/internal/cnpgi/operator/lifecycle_test.go +++ b/internal/cnpgi/operator/lifecycle_test.go @@ -211,11 +211,11 @@ var _ = Describe("LifecycleImplementation", func() { }) var _ = Describe("Volume utilities", func() { - Describe("volumeListEnsureVolume", func() { + Describe("ensureVolume", func() { It("adds a new volume if not present", func() { volumes := []corev1.Volume{{Name: "vol1"}} newVol := corev1.Volume{Name: "vol2"} - result := volumeListEnsureVolume(volumes, newVol) + result := ensureVolume(volumes, newVol) Expect(result).To(HaveLen(2)) Expect(result[1]).To(Equal(newVol)) }) @@ -223,20 +223,20 @@ var _ = Describe("Volume utilities", func() { 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) + result := ensureVolume(volumes, updatedVol) Expect(result).To(HaveLen(1)) Expect(result[0]).To(Equal(updatedVol)) }) }) - Describe("volumeListRemoveVolume", func() { + Describe("removeVolume", func() { It("removes the specified volume", func() { volumes := []corev1.Volume{ {Name: "vol1"}, {Name: "vol2"}, {Name: "vol3"}, } - result := volumeListRemoveVolume(volumes, "vol2") + result := removeVolume(volumes, "vol2") Expect(result).To(HaveLen(2)) for _, v := range result { Expect(v.Name).NotTo(Equal("vol2")) @@ -245,8 +245,37 @@ var _ = Describe("Volume utilities", func() { It("returns the same list if the volume is not present", func() { volumes := []corev1.Volume{{Name: "vol1"}, {Name: "vol2"}} - result := volumeListRemoveVolume(volumes, "vol3") + result := removeVolume(volumes, "vol3") Expect(result).To(Equal(volumes)) }) }) }) + +var _ = Describe("removeVolumeMount", func() { + It("removes the specified volume mount", func() { + mounts := []corev1.VolumeMount{ + {Name: "mount1"}, + {Name: "mount2"}, + {Name: "mount3"}, + } + result := removeVolumeMount(mounts, "mount2") + Expect(result).To(HaveLen(2)) + for _, m := range result { + Expect(m.Name).NotTo(Equal("mount2")) + } + }) + + It("returns the same list if the volume mount is not present", func() { + mounts := []corev1.VolumeMount{{Name: "mount1"}, {Name: "mount2"}} + result := removeVolumeMount(mounts, "mount3") + Expect(result).To(HaveLen(2)) + Expect(result[0].Name).To(Equal("mount1")) + Expect(result[1].Name).To(Equal("mount2")) + }) + + It("handles empty input slice", func() { + var mounts []corev1.VolumeMount + result := removeVolumeMount(mounts, "mount1") + Expect(result).To(BeEmpty()) + }) +})