Skip to content

Commit ef7b29c

Browse files
authored
fix: do not create empty PVC when storage has only volumeMount (#1653)
When spec.storage contains only volumeMount entries (e.g. emptyDir) without volumeClaimTemplate, the operator incorrectly set PersistenceEnabled=true, causing an empty volumeClaimTemplates entry in the StatefulSet. Check whether volumeClaimTemplate is actually populated before enabling persistence. Signed-off-by: Aleksandrov Aleksandr <aaleksandrov.cy@gmail.com>
1 parent 329deaf commit ef7b29c

File tree

4 files changed

+71
-4
lines changed

4 files changed

+71
-4
lines changed

internal/k8sutils/redis-replication.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func generateRedisReplicationContainerParams(cr *rrvb2.RedisReplication) contain
194194
if cr.Spec.LivenessProbe != nil {
195195
containerProp.LivenessProbe = cr.Spec.LivenessProbe
196196
}
197-
if cr.Spec.Storage != nil {
197+
if storageHasVolumeClaimTemplate(cr.Spec.Storage) {
198198
containerProp.PersistenceEnabled = &trueProperty
199199
}
200200
if cr.Spec.TLS != nil {
@@ -230,7 +230,7 @@ func generateRedisReplicationInitContainerParams(cr *rrvb2.RedisReplication) ini
230230
initcontainerProp.AdditionalVolume = cr.Spec.Storage.VolumeMount.Volume
231231
initcontainerProp.AdditionalMountPath = cr.Spec.Storage.VolumeMount.MountPath
232232
}
233-
if cr.Spec.Storage != nil {
233+
if storageHasVolumeClaimTemplate(cr.Spec.Storage) {
234234
initcontainerProp.PersistenceEnabled = &trueProperty
235235
}
236236
}

internal/k8sutils/redis-standalone.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func generateRedisStandaloneContainerParams(cr *rvb2.Redis) containerParameters
197197
if cr.Spec.LivenessProbe != nil {
198198
containerProp.LivenessProbe = cr.Spec.LivenessProbe
199199
}
200-
if cr.Spec.Storage != nil {
200+
if storageHasVolumeClaimTemplate(cr.Spec.Storage) {
201201
containerProp.PersistenceEnabled = &trueProperty
202202
}
203203
if cr.Spec.TLS != nil {
@@ -233,7 +233,7 @@ func generateRedisStandaloneInitContainerParams(cr *rvb2.Redis) initContainerPar
233233
initcontainerProp.AdditionalVolume = cr.Spec.Storage.VolumeMount.Volume
234234
initcontainerProp.AdditionalMountPath = cr.Spec.Storage.VolumeMount.MountPath
235235
}
236-
if cr.Spec.Storage != nil {
236+
if storageHasVolumeClaimTemplate(cr.Spec.Storage) {
237237
initcontainerProp.PersistenceEnabled = &trueProperty
238238
}
239239
}

internal/k8sutils/statefulset.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,17 @@ func syncManagedFields(stored, new *appsv1.StatefulSet) {
250250
new.ManagedFields = stored.ManagedFields
251251
}
252252

253+
// storageHasVolumeClaimTemplate checks if the Storage has a meaningful VolumeClaimTemplate defined
254+
// (i.e., not just the zero value). This distinguishes between storage configured with only
255+
// volumeMount (e.g. emptyDir) vs. storage that actually requests a PersistentVolumeClaim.
256+
func storageHasVolumeClaimTemplate(storage *commonapi.Storage) bool {
257+
if storage == nil {
258+
return false
259+
}
260+
vct := storage.VolumeClaimTemplate
261+
return len(vct.Spec.AccessModes) > 0 || vct.Spec.Resources.Requests != nil || vct.Spec.StorageClassName != nil || vct.Spec.VolumeName != ""
262+
}
263+
253264
// hasVolumeClaimTemplates checks if the StatefulSet has VolumeClaimTemplates and if their counts match.
254265
func hasVolumeClaimTemplates(new, stored *appsv1.StatefulSet) bool {
255266
return len(new.Spec.VolumeClaimTemplates) >= 1 && len(new.Spec.VolumeClaimTemplates) == len(stored.Spec.VolumeClaimTemplates)

internal/k8sutils/statefulset_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,62 @@ func TestGenerateAuthAndTLSArgs(t *testing.T) {
4848
}
4949
}
5050

51+
func TestStorageHasVolumeClaimTemplate(t *testing.T) {
52+
tests := []struct {
53+
name string
54+
storage *common.Storage
55+
want bool
56+
}{
57+
{"nil storage", nil, false},
58+
{"empty VolumeClaimTemplate", &common.Storage{}, false},
59+
{"only volumeMount", &common.Storage{
60+
VolumeMount: common.AdditionalVolume{
61+
Volume: []corev1.Volume{{Name: "data", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}},
62+
MountPath: []corev1.VolumeMount{{Name: "data", MountPath: "/data"}},
63+
},
64+
}, false},
65+
{"with AccessModes", &common.Storage{
66+
VolumeClaimTemplate: corev1.PersistentVolumeClaim{
67+
Spec: corev1.PersistentVolumeClaimSpec{
68+
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
69+
},
70+
},
71+
}, true},
72+
{"with Resources.Requests", &common.Storage{
73+
VolumeClaimTemplate: corev1.PersistentVolumeClaim{
74+
Spec: corev1.PersistentVolumeClaimSpec{
75+
Resources: corev1.VolumeResourceRequirements{
76+
Requests: corev1.ResourceList{
77+
corev1.ResourceStorage: resource.MustParse("1Gi"),
78+
},
79+
},
80+
},
81+
},
82+
}, true},
83+
{"with StorageClassName", &common.Storage{
84+
VolumeClaimTemplate: corev1.PersistentVolumeClaim{
85+
Spec: corev1.PersistentVolumeClaimSpec{
86+
StorageClassName: ptr.To("standard"),
87+
},
88+
},
89+
}, true},
90+
{"with VolumeName", &common.Storage{
91+
VolumeClaimTemplate: corev1.PersistentVolumeClaim{
92+
Spec: corev1.PersistentVolumeClaimSpec{
93+
VolumeName: "pv-data",
94+
},
95+
},
96+
}, true},
97+
}
98+
99+
for _, tt := range tests {
100+
t.Run(tt.name, func(t *testing.T) {
101+
got := storageHasVolumeClaimTemplate(tt.storage)
102+
assert.Equal(t, tt.want, got)
103+
})
104+
}
105+
}
106+
51107
func TestGeneratePreStopCommand(t *testing.T) {
52108
tests := []struct {
53109
name string

0 commit comments

Comments
 (0)