Skip to content

feat: add spec.additionalVolumeMounts for the main container - #565

Open
IgorOhrimenko wants to merge 1 commit into
dragonflydb:mainfrom
IgorOhrimenko:feat/additional-volume-mounts
Open

feat: add spec.additionalVolumeMounts for the main container#565
IgorOhrimenko wants to merge 1 commit into
dragonflydb:mainfrom
IgorOhrimenko:feat/additional-volume-mounts

Conversation

@IgorOhrimenko

Copy link
Copy Markdown

What

Adds spec.additionalVolumeMounts — extra volume mounts for the Dragonfly main container. It is the counterpart to the existing spec.additionalVolumes, which adds volumes to the pod but does not mount them anywhere.

Why

additionalVolumes on its own is half a feature: you can add a volume to the pod but cannot mount it into the Dragonfly container. Pairing the two lets users compose several patterns without any snapshot-specific API, e.g.:

  • Back the snapshot dir (spec.snapshot.dir) with an emptyDir. Dragonfly saves on SIGTERM and reloads on start, so an emptyDir snapshot survives in-place container restarts (e.g. a kubelet minor upgrade that changes the container hash) without pinning the pod to a node the way a local-path PVC does.
  • Read-only warm-up cache — mount a golden snapshot read-only; every start loads it, runtime writes never touch the on-disk copy.
  • Combine an init-seeded emptyDir for warm-up + crash protection.

Usage examples (all validated end-to-end on a cluster): https://gist.github.com/IgorOhrimenko/d679e593637e0fbe62525f165fd17d78

Implementation

  • Merged into the main container by name (a user mount replaces the operator's default on collision), mirroring how additionalVolumes / additionalContainers already merge.
  • Applied before the additionalContainers merge, since that merge prepends additional containers and would otherwise shift the main container off index 0.

Testing

  • Unit tests in internal/resources, including one asserting mounts land on the main container only and never on additionalContainers.
  • Added a config/samples example (v1alpha1_dragonfly_emptydir_snapshot.yaml).
  • Verified on a live cluster: emptyDir survives simultaneous both-pod container restarts (data preserved); a read-only seed stays immutable across restarts; the init-seed + emptyDir combo warms from a baseline and keeps runtime growth across in-place restarts.

Backward compatible — the field is optional and existing specs are unaffected.

Note: I regenerated config/crd/bases (make manifests) but intentionally left the manifests/ bundle untouched — it is already out of sync with config/ on main (missing some recent upstream API-type updates), so regenerating it here would add unrelated churn. Happy to include a bundle regeneration if you prefer.

Complements spec.additionalVolumes, which only adds volumes to the pod
without mounting them. additionalVolumeMounts mounts them into the
Dragonfly main container, so users can e.g. back the snapshot dir
(spec.snapshot.dir) with an emptyDir — surviving in-place container
restarts without pinning the pod to a node like a local-path PVC does.

Mounts are merged into the main container before additionalContainers
are merged in, since that merge can prepend sidecars and shift the main
container off index 0. Merged by name (user mount replaces default on
collision), mirroring additionalVolumes/additionalContainers.

Adds a config/samples example and unit tests, including one asserting the
mounts land on the main container only, never on additional containers.

Signed-off-by: Igor Ohrimenko <igor.ohrimenko@travelata.ru>
@IgorOhrimenko
IgorOhrimenko force-pushed the feat/additional-volume-mounts branch from be445b4 to 2a2b906 Compare July 18, 2026 07:50
@IgorOhrimenko

Copy link
Copy Markdown
Author

The build-operator failure looks like pre-existing e2e flakiness, not related to this change. The 3 failing specs (PVC single-replica, lifecycle connectivity, readiness-gate) don't use additionalVolumeMounts, and the new merge is a no-op when the field is unset (mergeNamedSlices(base, nil) returns the mounts unchanged), so the generated StatefulSet is identical to main for those tests. Failures are timing/infra (System should be stable … Timed out 60s, redis connection refused …, 4 flaked). The same suite intermittently fails on main too.

@Abhra303
Abhra303 self-requested a review August 12, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant