[RBD] Unrecognized VolumeAttributesClass parameter names silently clear all cgroup QoS metadata

[yogananth-subramanian]

Describe the bug

When a VolumeAttributesClass contains parameter names that no QoS handler recognizes (e.g., readIOPS instead of maxReadIops), ControllerModifyVolume succeeds without error and silently deletes all existing cgroup QoS metadata from the RBD image. On the next pod mount, NodePublishVolume finds no QoS metadata and the pod starts with no io.max limits.

The root cause is that validateQoSParameters() only validates parameters it recognizes — it returns nil for unknown keys. The subsequent modifyVolumeAttributes() loop asks each handler if it recognizes the parameters; when none do, the code falls through to a "clear all QoS" path that was designed for the case where a user intentionally removes QoS from a PVC.

Unrecognized parameter names are indistinguishable from "no QoS parameters" in the current code, so the driver interprets wrong names as "user wants to remove QoS" and actively deletes all stored metadata.

Environment details

• Image/version of Ceph CSI driver: quay.io/cephcsi/cephcsi:canary (includes PR #6274: Add cgroup v2 QoS support for RBD volumes)
• Helm chart version: N/A (deployed via ODF 4.23)
• Kernel version: 6.12+ (RHCOS 9.8)
• Mounter: krbd (cgroup v2 QoS path)
• Kubernetes cluster version: 1.35 (VolumeAttributesClass GA since 1.31)
• Ceph cluster version: Reef (deployed via ODF 4.23)

Steps to reproduce

1. Create a VolumeAttributesClass with correct parameter names and a PVC + pod using it:

   apiVersion: storage.k8s.io/v1
   kind: VolumeAttributesClass
   metadata:
     name: correct-qos
   driverName: openshift-storage.rbd.csi.ceph.com
   parameters:
     maxReadIops: "500"
     maxWriteIops: "500"
     maxReadBps: "5242880"
     maxWriteBps: "10485760"

2. Verify QoS is applied — pod's cgroup shows io.max:

   252:0 rbps=5242880 wbps=10485760 riops=500 wiops=500
   

3. Verify RBD image metadata contains QoS keys:

   rbd image-meta list <pool>/<image>
   # Shows:
   # .rbd.csi.ceph.com/cgroup_qos_max_read_iops   500
   # .rbd.csi.ceph.com/cgroup_qos_max_write_iops   500
   # .rbd.csi.ceph.com/cgroup_qos_max_read_bps     5242880
   # .rbd.csi.ceph.com/cgroup_qos_max_write_bps     10485760

4. Create a VolumeAttributesClass with wrong parameter names:

   apiVersion: storage.k8s.io/v1
   kind: VolumeAttributesClass
   metadata:
     name: wrong-qos
   driverName: openshift-storage.rbd.csi.ceph.com
   parameters:
     readIOPS: "500"       # wrong — should be maxReadIops
     writeIOPS: "500"      # wrong — should be maxWriteIops
     readBPS: "5242880"    # wrong — should be maxReadBps
     writeBPS: "10485760"  # wrong — should be maxWriteBps

5. Patch the PVC to the wrong-named VAC:

   kubectl patch pvc my-pvc --type=merge \
     -p '{"spec":{"volumeAttributesClassName":"wrong-qos"}}'

6. Observe: ControllerModifyVolume returns success — no error, no event, no warning.

7. Check RBD image metadata again:

   rbd image-meta list <pool>/<image>
   # All .rbd.csi.ceph.com/cgroup_qos_* keys are GONE

8. Delete and recreate the pod.

9. Check io.max on the new pod:

   # EMPTY — no io.max limits applied
   

Actual results

• ControllerModifyVolume returns gRPC success (no error)
• No Kubernetes event or warning is emitted
• PVC .status.currentVolumeAttributesClassName updates to wrong-qos
• All RBD image QoS metadata keys are silently deleted
• Running pods are unaffected (their cgroup io.max persists in-kernel)
• On next pod mount (recreation, rollout, node drain), NodePublishVolume finds no metadata and the pod starts unthrottled

The delayed-effect nature makes this especially dangerous — everything appears correct until the next pod lifecycle event.

Expected behavior

ControllerModifyVolume should return a gRPC INVALID_ARGUMENT error when the request contains mutable parameters that no QoS handler recognizes. The CSI spec defines this error code for exactly this situation:

INVALID_ARGUMENT: ... Indicates that a required parameter or value is missing or invalid.

Existing QoS metadata on the RBD image should not be modified when the request parameters are unrecognized.

Code path analysis

The call chain that leads to silent clearing:

1. validateQoSParameters() does not reject unknown params

File: internal/rbd/controllerserver.go, function validateQoSParameters()

For the krbd mounter, the function checks:

• HasQoSParams(mutableParams) — looks for NBD params (baseIops, baseReadIops, etc.). Wrong names like readIOPS are not in this set → returns false.
• hasCgroupQoSParams(mutableParams) — looks for exactly maxReadIops, maxWriteIops, maxReadBps, maxWriteBps. Wrong names not in this set → returns false.
• Returns nil — no error. Unknown params pass through silently.

2. modifyVolumeAttributes() fallthrough to clearing

File: internal/rbd/rbd_util.go, function modifyVolumeAttributes()

Two handlers are created: cgroupQoSHandler and nbdQoSHandler. The code iterates:

for _, handler := range handlers {
    if handler.HasParams(newMutableParameters) {
        // validate + apply + return
    }
}

Neither handler recognizes the wrong parameter names → the loop completes without returning.

The critical fallthrough:

// No QoS parameters provided - clear any existing QoS of all types.
// This happens when VolumeAttributesClass is removed from the PVC.
log.DebugLog(ctx, "no QoS parameters in request, clearing existing QoS for volume %s", rv.VolID)

for _, handler := range handlers {
    if err := handler.Clear(ctx); err != nil { ...

3. hasCgroupQoSParams() — the four recognized names

File: internal/rbd/cgroup_qos.go, function hasCgroupQoSParams()

func hasCgroupQoSParams(params map[string]string) bool {
    cgroupParams := []string{maxReadIops, maxWriteIops, maxReadBps, maxWriteBps}
    for _, param := range cgroupParams {
        if val, ok := params[param]; ok && val != "" {
            return true
        }
    }
    return false
}

The constants are defined in internal/rbd/qos.go:

maxReadIops  = "maxReadIops"
maxWriteIops = "maxWriteIops"
maxReadBps   = "maxReadBps"
maxWriteBps  = "maxWriteBps"

4. Clear() deletes all metadata

File: internal/rbd/cgroup_qos.go, function Clear() → saveCgroupQoS()

Clear() calls saveCgroupQoS(ctx, map[string]string{}) with an empty map. saveCgroupQoS() detects no cgroup params and iterates over qosParamToMetadataKey, calling rv.RemoveMetadata(metadataKey) for each of the four RBD image metadata keys.

5. Next mount finds nothing

File: internal/rbd/cgroup_qos.go, function applyCgroupQoSForVolume()

Called during NodePublishVolume. Reads metadata via getCgroupQoS() — now empty. Logs "no cgroup QoS configured for volume" at DEBUG level and returns without writing io.max.

Summary diagram

ControllerModifyVolume(params: {readIOPS: "500", writeIOPS: "500", ...})
  │
  ├─ validateQoSParameters() → nil (unknown params not rejected)
  │
  └─ modifyVolumeAttributes()
       ├─ cgroupQoSHandler.HasParams() → false (no maxReadIops etc.)
       ├─ nbdQoSHandler.HasParams()    → false (no baseIops etc.)
       │
       └─ FALLTHROUGH: "no QoS parameters in request"
            ├─ cgroupQoSHandler.Clear() → RemoveMetadata(all 4 keys)
            └─ nbdQoSHandler.Clear()    → RemoveMetadata(all NBD keys)

Next NodePublishVolume:
  applyCgroupQoSForVolume()
    └─ getCgroupQoS() → empty map → "no cgroup QoS configured" → no io.max

Proposed fix

Add a check in modifyVolumeAttributes() (or validateQoSParameters()) that distinguishes between:

1. Empty mutable parameters = user removed VAC from PVC → clear QoS (current behavior, correct)
2. Non-empty mutable parameters but no handler recognizes them = wrong parameter names → return codes.InvalidArgument

For example, in modifyVolumeAttributes() after the handler loop:

// Current: falls through to Clear()
// Proposed:
if len(newMutableParameters) > 0 {
    return status.Errorf(codes.InvalidArgument,
        "mutable parameters contain unrecognized keys: %v — "+
            "valid cgroup QoS parameters are: maxReadIops, maxWriteIops, maxReadBps, maxWriteBps",
        maps.Keys(newMutableParameters))
}
// Only clear when parameters are genuinely empty

This preserves the intentional "clear on VAC removal" behavior while preventing wrong parameter names from silently destroying QoS metadata.

Impact

Affected scenario	Detail
Initial setup	Users following early documentation or prototypes that used different parameter naming conventions (e.g., readIOPS, writeBPS, maxReadIOPS) will get no QoS with no error. The HackMD design document used capitalized maxReadIOPS — the PR implemented lowercase maxReadIops.
VAC migration	If a ceph-csi upgrade changes expected parameter names, patching PVCs to a VAC with old-style names silently clears QoS on the next ControllerModifyVolume.
Delayed-effect danger	Running pods are unaffected (cgroup io.max persists in-kernel), so the problem is invisible until the next pod recreation — which could be days or weeks later during a routine rollout or node drain.
Multi-tenant risk	When QoS is silently cleared and a pod is recreated without limits, it competes for I/O bandwidth unthrottled while other tenants' pods are still throttled — a noisy-neighbor scenario.
Diagnosability	Everything looks correct from the Kubernetes API: PVC shows the VAC name, ControllerModifyVolume returned success, no events or warnings. Only inspecting RBD image metadata (rbd image-meta list) or the pod's io.max reveals the failure.

Logs

The clearing happens at DEBUG log level:

"no QoS parameters in request, clearing existing QoS for volume <volID>"

And on next mount:

"no cgroup QoS configured for volume <volID>"

Both are DEBUG, not WARN or ERROR — no indication that something unexpected happened.

[Madhu-1]

@yogananth-subramanian Thanks for the details. This is the existing behaviour for any classes in the csi, we dont validate the unknown parameters rather we consider only the known parameters. The classes are admin created objects and there is no way to validate the parameters during the creation, they all come in to picture only during the PVC operations for example Create/Update operations. As the parameters are well established its admin responsibility to create the classes with the known required parameters.

one good example for this one is encryption storageclass, if the encryption key is wrong in the parameters, we just go ahead and create the non encrypted pvc and return success.

I agree we can have such validation to find the errors but note they will come in to picture only when user try to update the VAC in the PVC object not until that time.

[Madhu-1]

@nixpanic @Rakshith-R do you guys think we should have such validation to identify the unrecognized parameters?

[nixpanic]

Returning INVALID_ARGUMENT for unknown parameters would be nice, I think. That is also documented in the ControllerModify errors.