[Design] Persistent Volumes for ValkeyNode

[jdheyburn]

Previous discussions:

• # Design Proposal: Persistent Volume Support for Valkey Operator #88
• feat: Add persistent volume support for ValkeyCluster #85

The user should be able to allow for volumes to be attached to ValkeyNodes. This can be used for:

• persisting memory to disk
  • allow for a partial resync on pod roll
• persisting nodes.conf (when in Cluster mode)
  • to avoid having to rediscover the cluster on pod rolls
• other data durability needs

When a volume is enabled on the ValkeyCluster, it should traverse this config to the ValkeyNode, which should create the volume prior to creating the workload (Deployment/StatefulSet). Then attach the volume to the workload (Deployment/StatefulSet) so that it can be read.

The volume should be independently managed by the ValkeyNode controller so that we can dynamically resize it (should the storage class support it) in either:

• a user request
• automatically increasing via a future feature

Note: cluster node reconciliation should continue to happen regardless of whether the volume is created or not.

Proposed specs:

apiVersion: valkey.io/v1alpha1
kind: ValkeyCluster
metadata:
  name: my-cluster
spec:
  shards: 3
  replicas: 1
  persistence:
    size: 10Gi
    storageClassName: gp3

This then is traversed to ValkeyNodes:

kind: ValkeyNode
metadata:
  name: my-cluster-0-0
spec:
  # ... other attributes
  persistence:
    size: 10Gi
    storageClassName: gp3

We can use presence-based enabling (if persistence is defined, create the PVC).

Go structs and usage

type PersistenceSpec struct {
    // Size of the PVC. Required when mode is enabled.
    // +optional
    Size *resource.Quantity `json:"size,omitempty"`

    // StorageClassName for the PVC. Uses cluster default if omitted.
    // +optional
    StorageClassName *string `json:"storageClassName,omitempty"`
}

type ValkeyNodeSpec struct {
    // ... existing fields ...

    // Persistent storage configuration.
    // When nil, no PVC is created (ephemeral storage).
    // +optional
    Persistence *PersistenceSpec `json:"persistence,omitempty"`
}

type ValkeyClusterSpec struct {
    // ... existing fields ...

    // Persistent storage configuration passed through to all ValkeyNodes.
    // +optional
    Persistence *PersistenceSpec `json:"persistence,omitempty"`
}

Consumption

The volume should be attached to the pod and then mounted on the container:

volumes:
  - name: data
    persistentVolumeClaim:
      claimName: valkey-<valkeynode-name>-data

containers:
  - name: server
    volumeMounts:
      - name: data
        mountPath: /data

Then the config will need to specify dir:

dir /data
cluster-config-file /data/config/nodes.conf

I'm suggesting to put nodes.conf at /data/configso that there is some tidiness within the /data dir.

Future work, potentially out of scope for this iteration

• Native RDB/AOF configuration
• External PVCs
  • is this something we want? Would users like this for their own data migration?
• Reclaim policy
• Volume expansion

cc @umar-riswan-ap @deepakpunjabi

[deepakpunjabi]

Does it makes sense to simplify this design by

• Using volumeClaimTemplates instead of persistentVolumeClaim.
• Disallowing persistence for WorkloadTypeDeployment.

Doing this, externalizes the lifecycle of PVC outside operator and ties it to WorkloadTypeStatefulSet. Adding persistence support to Deployment requires the operator to manually manage PVC lifecycle, handle cleanup, set Recreate strategy to avoid mount contention, which is directly available via StatefulSet .

Can we set up convention that WorkloadTypeDeployment option exists for stateless use cases. If a user wants persistence, they should be using a StatefulSet.

Validation with CEL is also straightforward.

  // +kubebuilder:validation:XValidation:rule="!(has(self.persistence) && self.workloadType == 'Deployment')",message="persistence requires workloadType StatefulSet"
  type ValkeyClusterSpec struct {
      // ...
  }

Let me know your view.

[jdheyburn]

@deepakpunjabi volumeClaimTemplates are immutable after creation. If the pod scales vertically and persists to disk, the operator would have to manually increase the volume anyway - or follow a destructive process of recreating the StatefulSet by orphaning the pods. That is a process that can be avoided by having the operator manage the volumes from day 1.

We can disable Volumes for Deployments initially to simplify the solution. At least this way, we have the option of adding it at a later date.

[deepakpunjabi]

Fair, it makes sense to own lifecycle of PVC if dynamic resize is a first class requirement.
Could you please shed some light on few other questions in the initial design proposed?

1. Should Size be a mandatory field?

type PersistenceSpec struct {
    // Size of the PVC. Required when mode is enabled.
    // +optional
    Size *resource.Quantity `json:"size,omitempty"`

2. Are you planning to support persistence only for statefulsets in the initial implementation?
3. Will there be OwnerReferences on the PVC? Will it be ValkeyNode, ValkeyCluster, both?
4. [Migration] Does enabling persistence on previously persistence disabled cluster just create PVCs with empty data or triggers a rolling restart of the existing cluster?
5. Who owns the creation of /data/config subdirectory? Is this supposed to be done by an init container?
6. Do we define PersistenceReady for ValkeyNodeStatus?
7. Are you planning to pick this up or is it open to pick for implementation?

[jdheyburn]

Thanks for the questions!

1. I am hoping that we can use the presence of PersistenceSpec as a means of enabling persistence. i.e. if persistence exists, it is used. Instead of us requiring a persistence.enabled field. If setting Size to mandatory doesn't break this, then that I'm fine for that.

2. If it helps simplify the initial implementation, then yes. We can always add it to Deployments later. The consensus in the k8s ecosystem is if you need persistence then use StatefulSets. Although in our design where we want volume expansion to be dynamic, it means that we should have Volumes managed outside of volumeClaimTemplate. This then means there's nothing stopping us adding it to Deployment. But we can leave the Deployment implementation out for this first iteration.

3. ValkeyNode would be the owner reference, as it is already for any resource it creates already. This then means if someone switches from Deployment <-> StatefulSet for WorkloadType, then the volume still exists.

4. The PVCs would be created with empty directories - but this should then trigger a rolling restart. That aspect is blocked on feat: Safer cluster node rolls #120 though.

5. If the creation of nodes.conf does not create this, then we may want an init container in its place. If its simpler to just have nodes.conf at /data, then I am in favour of that instead of introducing an init container.

6. That's optional, we can include it and decide how useful it is after the fact.

7. Open for implementation, if you would like to take it that would be a big help :)

[daanvinken]

Some of my 2cs:

1. If ValkeyNode owns the PVC via ownerReference, any time the operator deletes a ValkeyNode (scale-in, recovery, even a bug), Kubernetes garbage collection deletes the PVC and all data with it. I took a peek how other operators handle this; CNPG avoids this by managing PVCs independently (associated by naming convention/labels, no ownerReference), so PVCs survive instance deletion and are only cleaned up explicitly after confirming the instance is fully decommissioned.
   Opstree's redis-operator takes a middle ground with a KeepAfterDelete flag and Kubernetes' PersistentVolumeClaimRetentionPolicy, giving users explicit control. Either way, the current design of tying PVC lifecycle to ValkeyNode via ownerReference feels risky for production use.

2. A pod could have been down for an unknown amount of time. If it comes back up with a stale nodes.conf, it rejoins the cluster with outdated topolgy . The operator would then have to detect and reconcile that stale state, which adds complexity.

How expensive is fetching the topology fresh on startup really? The operator already does full cluster discovery via CLUSTER NODES and CLUSTER INFO on every reconcile. A clean start with an empty nodes.conf and letting the operator reintroduce the node via CLUSTER MEET seems simpler and more predictable than trusting whatever is on disk.

3. During scale-in, slots are drained first, then ValkeyNode is deleted. But what if the drain didn't fully complete? A finalizer (e.g. valkey.io/pvc-protection on ValKeyNode that checks slot count = 0 before allowing PVC deletion would prevent data loss.

4. Volume resizing is never easy. Just a note that we should take failures into account. The operator needs to handle pending/failed expansion gracefully without blocking the reconciliation loop itself.

[jdheyburn]

Thanks for the comments @daanvinken!

1. Good point, we can then omit the ownerReference here.

2. I don't know enough about cluster semantics to comment. If stale nodes.conf is an issue, then we might want to think about this edge case for when PVs are introduced - because this would cause an issue.

3. That is a good suggestion - how does that work? I've not used them before.

4. That is something to take into account when resizing feature is introduced - right now we want to ensure that the design we make for adding PVs allows for this feature.

[daanvinken]

2. Revisiting my earlier point on nodes.conf. For simple pod rolls (e.g. rolling updates, OOM restarts) with a persistent volume, keeping nodes.conf on the same PV avoids unnecessary FORGET + MEET churn since the node comes back with the same ID and reconnects automatically.
   I think is already handled by the operator's normal reconcile loop through CLUSTER NODES discovery and FORGET/MEET. So persisting nodes.conf alongside the data is probably the right call, it's free and avoids unnecessary work in the common case. Perhaps we can just iterate on it when it causes issues.

3. On finalizers: when you add a string to metadata.finalizers on a ValkeyNode, Kubernetes blocks deletion and keeps the object in Terminating state until the finalizer is removed (by the thing owning it). The operator's reconciler can then check preconditions (e.g. slots drained) before removing the finalizer and allowing deletion to proceed. It's a fairly standard pattern for operators that need cleanup guarantees (think ArgoCD, CNPG etc).

[jdheyburn]

Implemented in #149