Skip to content

Commit 749ade0

Browse files
mvasilenkomvasilenko
committed
refactor as per comments
1 parent a0d4647 commit 749ade0

7 files changed

Lines changed: 131 additions & 173 deletions

File tree

‎config/crd/bases/dragonflydb.io_dragonflies.yaml‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4462,11 +4462,8 @@ spec:
44624462
type: object
44634463
type: object
44644464
customLivenessProbeConfigMap:
4465-
description: |-
4466-
(Optional) Custom ConfigMap for the liveness probe script.
4467-
Must contain key "liveness-check.sh". Overrides the operator-embedded default.
4468-
Note: if spec.additionalVolumes also contains a volume named "liveness-probe",
4469-
it takes precedence over this field. Do not use both for the same probe.
4465+
description: (Optional) Custom ConfigMap with key "liveness-check.sh"
4466+
to override the default liveness probe.
44704467
properties:
44714468
name:
44724469
default: ""
@@ -4480,11 +4477,8 @@ spec:
44804477
type: object
44814478
x-kubernetes-map-type: atomic
44824479
customReadinessProbeConfigMap:
4483-
description: |-
4484-
(Optional) Custom ConfigMap for the readiness probe script.
4485-
Must contain key "readiness-check.sh". Overrides the operator-embedded default.
4486-
Note: if spec.additionalVolumes also contains a volume named "readiness-probe",
4487-
it takes precedence over this field. Do not use both for the same probe.
4480+
description: (Optional) Custom ConfigMap with key "readiness-check.sh"
4481+
to override the default readiness probe.
44884482
properties:
44894483
name:
44904484
default: ""
@@ -4498,11 +4492,8 @@ spec:
44984492
type: object
44994493
x-kubernetes-map-type: atomic
45004494
customStartupProbeConfigMap:
4501-
description: |-
4502-
(Optional) Custom ConfigMap for the startup probe script.
4503-
Must contain key "startup-check.sh". Overrides the operator-embedded default.
4504-
Note: if spec.additionalVolumes also contains a volume named "startup-probe",
4505-
it takes precedence over this field. Do not use both for the same probe.
4495+
description: (Optional) Custom ConfigMap with key "startup-check.sh"
4496+
to override the default startup probe.
45064497
properties:
45074498
name:
45084499
default: ""

‎e2e/dragonfly_controller_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,10 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func()
136136
err := k8sClient.Create(ctx, &df)
137137
Expect(err).To(BeNil())
138138

139-
// Wait for master election, then for all replicas to be Ready
140-
waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseReady, 3*time.Minute)
141-
waitForStatefulSetReady(ctx, k8sClient, name, namespace, 3*time.Minute)
139+
// Wait until Dragonfly object is marked initialized
140+
waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseResourcesCreated, 2*time.Minute)
141+
waitForStatefulSetReady(ctx, k8sClient, name, namespace, 2*time.Minute)
142+
142143
})
143144

144145
var ss appsv1.StatefulSet

‎internal/controller/dragonfly_controller.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ func (r *DragonflyReconciler) SetupWithManager(mgr ctrl.Manager) error {
159159
For(&dfv1alpha1.Dragonfly{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
160160
Owns(&appsv1.StatefulSet{}, builder.MatchEveryOwner).
161161
Owns(&corev1.Service{}, builder.MatchEveryOwner).
162+
Owns(&corev1.ConfigMap{}, builder.MatchEveryOwner).
162163
Owns(&networkingv1.NetworkPolicy{}, builder.MatchEveryOwner).
163164
Named("Dragonfly").
164165
Complete(r)

‎internal/resources/resources.go‎

Lines changed: 53 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,52 @@ var (
3333
dflyUserGroup int64 = 999
3434
)
3535

36-
func generateProbeConfigMap(df *resourcesv1.Dragonfly, suffix, key, script string) *corev1.ConfigMap {
36+
// returns the custom ConfigMap name if set, or "<dfName>-<suffix>" otherwise
37+
func probeConfigMapName(dfName, suffix string, custom *corev1.LocalObjectReference) string {
38+
if custom != nil && custom.Name != "" {
39+
return custom.Name
40+
}
41+
return fmt.Sprintf("%s-%s", dfName, suffix)
42+
}
43+
44+
// append probe ConfigMap volumes and mounts into the StatefulSet
45+
func appendProbeVolumesAndMounts(sts *appsv1.StatefulSet, livenessCM, readinessCM, startupCM string) {
46+
probes := []struct {
47+
volumeName string
48+
configMapName string
49+
scriptKey string
50+
}{
51+
{LivenessProbeVolumeName, livenessCM, LivenessScriptKey},
52+
{ReadinessProbeVolumeName, readinessCM, ReadinessScriptKey},
53+
{StartupProbeVolumeName, startupCM, StartupScriptKey},
54+
}
55+
for _, p := range probes {
56+
sts.Spec.Template.Spec.Volumes = append(sts.Spec.Template.Spec.Volumes,
57+
corev1.Volume{
58+
Name: p.volumeName,
59+
VolumeSource: corev1.VolumeSource{
60+
ConfigMap: &corev1.ConfigMapVolumeSource{
61+
LocalObjectReference: corev1.LocalObjectReference{Name: p.configMapName},
62+
},
63+
},
64+
},
65+
)
66+
sts.Spec.Template.Spec.Containers[0].VolumeMounts = append(
67+
sts.Spec.Template.Spec.Containers[0].VolumeMounts,
68+
corev1.VolumeMount{
69+
Name: p.volumeName,
70+
MountPath: ProbeMountPath + "/" + p.scriptKey,
71+
SubPath: p.scriptKey,
72+
},
73+
)
74+
}
75+
}
76+
77+
// generateProbeConfigMap builds a ConfigMap owned by the Dragonfly instance for a single probe script.
78+
func generateProbeConfigMap(df *resourcesv1.Dragonfly, name, key, script string) *corev1.ConfigMap {
3779
return &corev1.ConfigMap{
3880
ObjectMeta: metav1.ObjectMeta{
39-
Name: fmt.Sprintf("%s-%s", df.Name, suffix),
81+
Name: name,
4082
Namespace: df.Namespace,
4183
OwnerReferences: []metav1.OwnerReference{
4284
{
@@ -428,77 +470,19 @@ func GenerateDragonflyResources(df *resourcesv1.Dragonfly, defaultDragonflyImage
428470
}
429471
}
430472

431-
// Probe script volumes — the corresponding ConfigMaps are generated and appended to resources below.
432-
livenessConfigMapName := fmt.Sprintf("%s-%s", df.Name, LivenessProbeConfigMapSuffix)
433-
if df.Spec.CustomLivenessProbeConfigMap != nil && df.Spec.CustomLivenessProbeConfigMap.Name != "" {
434-
livenessConfigMapName = df.Spec.CustomLivenessProbeConfigMap.Name
435-
}
436-
readinessConfigMapName := fmt.Sprintf("%s-%s", df.Name, ReadinessProbeConfigMapSuffix)
437-
if df.Spec.CustomReadinessProbeConfigMap != nil && df.Spec.CustomReadinessProbeConfigMap.Name != "" {
438-
readinessConfigMapName = df.Spec.CustomReadinessProbeConfigMap.Name
439-
}
440-
startupConfigMapName := fmt.Sprintf("%s-%s", df.Name, StartupProbeConfigMapSuffix)
441-
if df.Spec.CustomStartupProbeConfigMap != nil && df.Spec.CustomStartupProbeConfigMap.Name != "" {
442-
startupConfigMapName = df.Spec.CustomStartupProbeConfigMap.Name
443-
}
473+
// Wire probe ConfigMap volumes and mounts into the StatefulSet.
474+
livenessConfigMapName := probeConfigMapName(df.Name, LivenessProbeConfigMapSuffix, df.Spec.CustomLivenessProbeConfigMap)
475+
readinessConfigMapName := probeConfigMapName(df.Name, ReadinessProbeConfigMapSuffix, df.Spec.CustomReadinessProbeConfigMap)
476+
startupConfigMapName := probeConfigMapName(df.Name, StartupProbeConfigMapSuffix, df.Spec.CustomStartupProbeConfigMap)
444477

445-
statefulset.Spec.Template.Spec.Volumes = append(statefulset.Spec.Template.Spec.Volumes,
446-
corev1.Volume{
447-
Name: LivenessProbeVolumeName,
448-
VolumeSource: corev1.VolumeSource{
449-
ConfigMap: &corev1.ConfigMapVolumeSource{
450-
LocalObjectReference: corev1.LocalObjectReference{Name: livenessConfigMapName},
451-
},
452-
},
453-
},
454-
corev1.Volume{
455-
Name: ReadinessProbeVolumeName,
456-
VolumeSource: corev1.VolumeSource{
457-
ConfigMap: &corev1.ConfigMapVolumeSource{
458-
LocalObjectReference: corev1.LocalObjectReference{Name: readinessConfigMapName},
459-
},
460-
},
461-
},
462-
corev1.Volume{
463-
Name: StartupProbeVolumeName,
464-
VolumeSource: corev1.VolumeSource{
465-
ConfigMap: &corev1.ConfigMapVolumeSource{
466-
LocalObjectReference: corev1.LocalObjectReference{Name: startupConfigMapName},
467-
},
468-
},
469-
},
470-
)
471-
472-
statefulset.Spec.Template.Spec.Containers[0].VolumeMounts = append(
473-
statefulset.Spec.Template.Spec.Containers[0].VolumeMounts,
474-
corev1.VolumeMount{
475-
Name: LivenessProbeVolumeName,
476-
MountPath: ProbeMountPath + "/" + LivenessScriptKey,
477-
SubPath: LivenessScriptKey,
478-
},
479-
corev1.VolumeMount{
480-
Name: ReadinessProbeVolumeName,
481-
MountPath: ProbeMountPath + "/" + ReadinessScriptKey,
482-
SubPath: ReadinessScriptKey,
483-
},
484-
corev1.VolumeMount{
485-
Name: StartupProbeVolumeName,
486-
MountPath: ProbeMountPath + "/" + StartupScriptKey,
487-
SubPath: StartupScriptKey,
488-
},
478+
appendProbeVolumesAndMounts(&statefulset,
479+
livenessConfigMapName, readinessConfigMapName, startupConfigMapName,
489480
)
490481

491482
statefulset.Spec.Template.Spec.Containers = mergeNamedSlices(
492483
statefulset.Spec.Template.Spec.Containers, df.Spec.AdditionalContainers,
493484
func(c corev1.Container) string { return c.Name })
494485

495-
// There are two ways to override probe scripts:
496-
// 1. spec.customLivenessProbeConfigMap / customReadinessProbeConfigMap / customStartupProbeConfigMap
497-
// — replaces the generated default ConfigMap for a specific probe.
498-
// 2. spec.additionalVolumes with a matching volume name ("liveness-probe", "readiness-probe",
499-
// "startup-probe") — wins over both the default and custom ConfigMap volumes because
500-
// mergeNamedSlices appends additionalVolumes last, and Kubernetes uses the last definition.
501-
// Do not use both mechanisms for the same probe simultaneously.
502486
statefulset.Spec.Template.Spec.Volumes = mergeNamedSlices(
503487
statefulset.Spec.Template.Spec.Volumes, df.Spec.AdditionalVolumes,
504488
func(v corev1.Volume) string { return v.Name })
@@ -507,13 +491,13 @@ func GenerateDragonflyResources(df *resourcesv1.Dragonfly, defaultDragonflyImage
507491
// and can mount the probe script volumes without getting stuck in Pending.
508492
// Skip generating a default when the user has pointed to their own ConfigMap.
509493
if df.Spec.CustomLivenessProbeConfigMap == nil || df.Spec.CustomLivenessProbeConfigMap.Name == "" {
510-
resources = append(resources, generateProbeConfigMap(df, LivenessProbeConfigMapSuffix, LivenessScriptKey, defaultLivenessScript))
494+
resources = append(resources, generateProbeConfigMap(df, livenessConfigMapName, LivenessScriptKey, defaultLivenessScript))
511495
}
512496
if df.Spec.CustomReadinessProbeConfigMap == nil || df.Spec.CustomReadinessProbeConfigMap.Name == "" {
513-
resources = append(resources, generateProbeConfigMap(df, ReadinessProbeConfigMapSuffix, ReadinessScriptKey, defaultReadinessScript))
497+
resources = append(resources, generateProbeConfigMap(df, readinessConfigMapName, ReadinessScriptKey, defaultReadinessScript))
514498
}
515499
if df.Spec.CustomStartupProbeConfigMap == nil || df.Spec.CustomStartupProbeConfigMap.Name == "" {
516-
resources = append(resources, generateProbeConfigMap(df, StartupProbeConfigMapSuffix, StartupScriptKey, defaultStartupScript))
500+
resources = append(resources, generateProbeConfigMap(df, startupConfigMapName, StartupScriptKey, defaultStartupScript))
517501
}
518502

519503
resources = append(resources, &statefulset)

‎internal/resources/resources_test.go‎

Lines changed: 55 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -380,63 +380,62 @@ func TestProbes_PointToMountedScripts(t *testing.T) {
380380
}
381381

382382
func TestProbeVolumes_CustomConfigMapOverride(t *testing.T) {
383-
df := newTestDragonfly(1)
384-
df.Spec.CustomReadinessProbeConfigMap = &corev1.LocalObjectReference{Name: "my-readiness-check"}
385-
386-
objs, err := GenerateDragonflyResources(df, "")
387-
require.NoError(t, err)
388-
389-
sts := findStatefulSet(objs)
390-
require.NotNil(t, sts)
391-
392-
volumes := sts.Spec.Template.Spec.Volumes
393-
for _, v := range volumes {
394-
if v.Name == ReadinessProbeVolumeName {
395-
require.NotNil(t, v.ConfigMap)
396-
assert.Equal(t, "my-readiness-check", v.ConfigMap.Name,
397-
"custom ConfigMap should be mounted for readiness probe")
398-
return
399-
}
400-
}
401-
t.Fatal("readiness-probe volume not found")
402-
}
403-
404-
func TestProbeVolumes_CustomLivenessConfigMapOverride(t *testing.T) {
405-
df := newTestDragonfly(1)
406-
df.Spec.CustomLivenessProbeConfigMap = &corev1.LocalObjectReference{Name: "my-liveness-check"}
407-
408-
objs, err := GenerateDragonflyResources(df, "")
409-
require.NoError(t, err)
410-
411-
sts := findStatefulSet(objs)
412-
require.NotNil(t, sts)
413-
414-
for _, v := range sts.Spec.Template.Spec.Volumes {
415-
if v.Name == LivenessProbeVolumeName {
416-
require.NotNil(t, v.ConfigMap)
417-
assert.Equal(t, "my-liveness-check", v.ConfigMap.Name)
418-
return
419-
}
383+
tests := []struct {
384+
name string
385+
setup func(df *resourcesv1.Dragonfly)
386+
volumeName string
387+
wantCM string
388+
defaultCMName string // should NOT appear in generated resources
389+
}{
390+
{
391+
name: "custom liveness",
392+
setup: func(df *resourcesv1.Dragonfly) { df.Spec.CustomLivenessProbeConfigMap = &corev1.LocalObjectReference{Name: "my-liveness"} },
393+
volumeName: LivenessProbeVolumeName,
394+
wantCM: "my-liveness",
395+
defaultCMName: "test-df-" + LivenessProbeConfigMapSuffix,
396+
},
397+
{
398+
name: "custom readiness",
399+
setup: func(df *resourcesv1.Dragonfly) { df.Spec.CustomReadinessProbeConfigMap = &corev1.LocalObjectReference{Name: "my-readiness"} },
400+
volumeName: ReadinessProbeVolumeName,
401+
wantCM: "my-readiness",
402+
defaultCMName: "test-df-" + ReadinessProbeConfigMapSuffix,
403+
},
404+
{
405+
name: "custom startup",
406+
setup: func(df *resourcesv1.Dragonfly) { df.Spec.CustomStartupProbeConfigMap = &corev1.LocalObjectReference{Name: "my-startup"} },
407+
volumeName: StartupProbeVolumeName,
408+
wantCM: "my-startup",
409+
defaultCMName: "test-df-" + StartupProbeConfigMapSuffix,
410+
},
420411
}
421-
t.Fatal("liveness-probe volume not found")
422-
}
423412

424-
func TestProbeVolumes_CustomStartupConfigMapOverride(t *testing.T) {
425-
df := newTestDragonfly(1)
426-
df.Spec.CustomStartupProbeConfigMap = &corev1.LocalObjectReference{Name: "my-startup-check"}
427-
428-
objs, err := GenerateDragonflyResources(df, "")
429-
require.NoError(t, err)
430-
431-
sts := findStatefulSet(objs)
432-
require.NotNil(t, sts)
433-
434-
for _, v := range sts.Spec.Template.Spec.Volumes {
435-
if v.Name == StartupProbeVolumeName {
436-
require.NotNil(t, v.ConfigMap)
437-
assert.Equal(t, "my-startup-check", v.ConfigMap.Name)
438-
return
439-
}
413+
for _, tc := range tests {
414+
t.Run(tc.name, func(t *testing.T) {
415+
df := newTestDragonfly(1)
416+
tc.setup(df)
417+
418+
objs, err := GenerateDragonflyResources(df, "")
419+
require.NoError(t, err)
420+
421+
// volume should reference the custom ConfigMap
422+
sts := findStatefulSet(objs)
423+
require.NotNil(t, sts)
424+
425+
var found bool
426+
for _, v := range sts.Spec.Template.Spec.Volumes {
427+
if v.Name == tc.volumeName {
428+
require.NotNil(t, v.ConfigMap)
429+
assert.Equal(t, tc.wantCM, v.ConfigMap.Name)
430+
found = true
431+
break
432+
}
433+
}
434+
assert.True(t, found, "volume %q not found", tc.volumeName)
435+
436+
// default ConfigMap should NOT be generated
437+
assert.Nil(t, findConfigMap(objs, tc.defaultCMName),
438+
"default ConfigMap %q should not be generated when custom is set", tc.defaultCMName)
439+
})
440440
}
441-
t.Fatal("startup-probe volume not found")
442441
}

‎manifests/crd.yaml‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4461,11 +4461,8 @@ spec:
44614461
type: object
44624462
type: object
44634463
customLivenessProbeConfigMap:
4464-
description: |-
4465-
(Optional) Custom ConfigMap for the liveness probe script.
4466-
Must contain key "liveness-check.sh". Overrides the operator-embedded default.
4467-
Note: if spec.additionalVolumes also contains a volume named "liveness-probe",
4468-
it takes precedence over this field. Do not use both for the same probe.
4464+
description: (Optional) Custom ConfigMap with key "liveness-check.sh"
4465+
to override the default liveness probe.
44694466
properties:
44704467
name:
44714468
default: ""
@@ -4479,11 +4476,8 @@ spec:
44794476
type: object
44804477
x-kubernetes-map-type: atomic
44814478
customReadinessProbeConfigMap:
4482-
description: |-
4483-
(Optional) Custom ConfigMap for the readiness probe script.
4484-
Must contain key "readiness-check.sh". Overrides the operator-embedded default.
4485-
Note: if spec.additionalVolumes also contains a volume named "readiness-probe",
4486-
it takes precedence over this field. Do not use both for the same probe.
4479+
description: (Optional) Custom ConfigMap with key "readiness-check.sh"
4480+
to override the default readiness probe.
44874481
properties:
44884482
name:
44894483
default: ""
@@ -4497,11 +4491,8 @@ spec:
44974491
type: object
44984492
x-kubernetes-map-type: atomic
44994493
customStartupProbeConfigMap:
4500-
description: |-
4501-
(Optional) Custom ConfigMap for the startup probe script.
4502-
Must contain key "startup-check.sh". Overrides the operator-embedded default.
4503-
Note: if spec.additionalVolumes also contains a volume named "startup-probe",
4504-
it takes precedence over this field. Do not use both for the same probe.
4494+
description: (Optional) Custom ConfigMap with key "startup-check.sh"
4495+
to override the default startup probe.
45054496
properties:
45064497
name:
45074498
default: ""

0 commit comments

Comments
 (0)