Skip to content

Commit 52c47cd

Browse files
authored
Ensured volume mounts are sorted consistently (#300)
1 parent ea9d932 commit 52c47cd

File tree

2 files changed

+12
-23
lines changed

2 files changed

+12
-23
lines changed

pkg/util/merge/merge.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package merge
22

33
import (
44
"sort"
5+
"strings"
56

67
"github.com/mongodb/mongodb-kubernetes-operator/pkg/util/contains"
78
corev1 "k8s.io/api/core/v1"
@@ -430,16 +431,21 @@ func VolumeMounts(original, override []corev1.VolumeMount) []corev1.VolumeMount
430431
}
431432

432433
sort.SliceStable(mergedMounts, func(i, j int) bool {
433-
return mergedMounts[i].Name < mergedMounts[j].Name
434+
return volumeMountToString(mergedMounts[i]) < volumeMountToString(mergedMounts[j])
434435
})
435436

436437
return mergedMounts
437438
}
438439

440+
// volumeMountToString returns a string consisting of all components of the given VolumeMount.
441+
func volumeMountToString(v corev1.VolumeMount) string {
442+
return strings.Join([]string{v.Name, v.MountPath, v.SubPath}, "_")
443+
}
444+
439445
func createVolumeMountMap(mounts []corev1.VolumeMount) map[string]corev1.VolumeMount {
440446
m := make(map[string]corev1.VolumeMount)
441447
for _, v := range mounts {
442-
m[v.Name] = v
448+
m[volumeMountToString(v)] = v
443449
}
444450
return m
445451
}

pkg/util/merge/merge_test.go

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ func TestMergeContainer(t *testing.T) {
239239

240240
t.Run("Volume Mounts are overridden", func(t *testing.T) {
241241
volumeMounts := mergedContainer.VolumeMounts
242-
assert.Len(t, volumeMounts, 2)
242+
assert.Len(t, volumeMounts, 3, "volume mounts can have the same name, the uniqueness is the combination of name, path and subpath")
243243
t.Run("First VolumeMount is still present", func(t *testing.T) {
244244
vm0 := volumeMounts[0]
245245
assert.Equal(t, "volume-mount-0", vm0.Name)
@@ -249,12 +249,9 @@ func TestMergeContainer(t *testing.T) {
249249
assert.Equal(t, "original-sub-path-expr", vm0.SubPathExpr)
250250
})
251251
t.Run("Second VolumeMount has merged values", func(t *testing.T) {
252-
vm1 := volumeMounts[1]
253-
assert.Equal(t, "volume-mount-1", vm1.Name)
254-
assert.True(t, vm1.ReadOnly)
255-
assert.Equal(t, "override-mount-path-1", vm1.MountPath)
256-
assert.Equal(t, "override-sub-path-1", vm1.SubPath)
257-
assert.Equal(t, "override-sub-path-expr-1", vm1.SubPathExpr)
252+
assert.Equal(t, volumeMounts[0], defaultContainer.VolumeMounts[0])
253+
assert.Equal(t, volumeMounts[1], defaultContainer.VolumeMounts[1])
254+
assert.Equal(t, volumeMounts[2], overrideContainer.VolumeMounts[0])
258255
})
259256
})
260257
})
@@ -588,20 +585,6 @@ func TestMergeVolumeAddVolume(t *testing.T) {
588585
assert.Equal(t, corev1.EmptyDirVolumeSource{}, *volume1.EmptyDir)
589586
}
590587

591-
func TestMergeVolumeMounts(t *testing.T) {
592-
vol0 := corev1.VolumeMount{Name: "container-0.volume-mount-0"}
593-
vol1 := corev1.VolumeMount{Name: "another-mount"}
594-
volumeMounts := []corev1.VolumeMount{vol1, vol0}
595-
596-
mergedVolumeMounts := VolumeMounts(nil, volumeMounts)
597-
assert.Equal(t, []corev1.VolumeMount{vol1, vol0}, mergedVolumeMounts)
598-
599-
vol2 := vol1
600-
vol2.MountPath = "/somewhere"
601-
mergedVolumeMounts = VolumeMounts([]corev1.VolumeMount{vol2}, []corev1.VolumeMount{vol0, vol1})
602-
assert.Equal(t, []corev1.VolumeMount{vol2, vol0}, mergedVolumeMounts)
603-
}
604-
605588
func TestMergeHostAliases(t *testing.T) {
606589
ha0 := []corev1.HostAlias{
607590
{

0 commit comments

Comments
 (0)