rbd: apply QoS during CreateVolume

When a PVC is created with VAC name
already set,Kubernetes does not call
ControllerModifyVolume. That RPC is only
invoked when the VolumeAttributesClass
is changed after creation.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Author: Madhu-1

internal/rbd/controllerserver.go

@@ -150,6 +150,35 @@ func validateStriping(parameters map[string]string) error {
 	return nil
 }
 
+// validateQoSParameters validates QoS parameters from VolumeAttributesClass.
+// This validation happens before creating the RBD image to fail early on invalid parameters.
+func validateQoSParameters(mutableParams map[string]string) error {
+	// Check if cgroup v2 QoS parameters and traditional NBD QoS parameters are mixed.
+	hasCgroupQoS := hasCgroupQoSParams(mutableParams)
+	hasNBDQoS := HasQoSParams(mutableParams)
+
+	if hasCgroupQoS && hasNBDQoS {
+		return errors.New("cannot specify both cgroup v2 QoS parameters " +
+			"and traditional NBD QoS parameters in the same request")
+	}
+
+	// Validate cgroup QoS parameters if present.
+	if hasCgroupQoS {
+		if err := validateCgroupQoSParams(mutableParams); err != nil {
+			return fmt.Errorf("invalid cgroup QoS parameters: %w", err)
+		}
+	}
+
+	// Validate traditional NBD QoS parameters if present.
+	if hasNBDQoS {
+		if err := validateNBDQoSParams(mutableParams); err != nil {
+			return fmt.Errorf("invalid NBD QoS parameters: %w", err)
+		}
+	}
+
+	return nil
+}
+
 // parseVolCreateRequest take create volume `request` argument and make use of the
 // request arguments for subsequent calls.
 func (cs *ControllerServer) parseVolCreateRequest(
@@ -230,10 +259,19 @@ func (cs *ControllerServer) parseVolCreateRequest(
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	// parse QOS parameters from mutable parameters
-	err = rbdVol.SetQOS(ctx, req.GetMutableParameters())
-	if err != nil {
-		return nil, status.Error(codes.InvalidArgument, err.Error())
+	// Validate QoS parameters from VolumeAttributesClass before creating the image.
+	// This ensures we fail early if parameters are invalid, avoiding orphaned resources.
+	mutableParams := req.GetMutableParameters()
+	if len(mutableParams) > 0 {
+		err = validateQoSParameters(mutableParams)
+		if err != nil {
+			return nil, status.Error(codes.InvalidArgument, err.Error())
+		}
+
+		// Store BaseVolSize for capacity-based QoS calculations.
+		if v, ok := mutableParams[baseVolSizeBytes]; ok && v != "" {
+			rbdVol.BaseVolSize = v
+		}
 	}
 
 	err = rbdVol.Connect(cr)
@@ -814,19 +852,18 @@ func (cs *ControllerServer) createBackingImage(
 		return status.Error(codes.Internal, err.Error())
 	}
 
-	// Apply Qos parameters to rbd image.
-	err = rbdVol.ApplyQOS(ctx)
-	if err != nil {
-		log.ErrorLog(ctx, "failed to apply QOS for rbd image: %s with error: %v", rbdVol, err)
+	// Apply QoS parameters from VolumeAttributesClass if present.
+	// This handles both traditional NBD QoS and cgroup v2 QoS.
+	if len(mutableParameters) > 0 {
+		// Set RequestedVolSize for QoS capacity-based calculations.
+		rbdVol.RequestedVolSize = rbdVol.VolSize
 
-		return status.Error(codes.Internal, err.Error())
-	}
-	// Save Qos parameters from mutable parameters in Image metadata, we will use it while resize volume.
-	err = rbdVol.SaveQOS(ctx, mutableParameters)
-	if err != nil {
-		log.ErrorLog(ctx, "failed to save QOS for rbd image: %s with error: %v", rbdVol, err)
+		err = rbdVol.modifyVolumeAttributes(ctx, mutableParameters)
+		if err != nil {
+			log.ErrorLog(ctx, "failed to apply QoS for rbd image: %s with error: %v", rbdVol, err)
 
-		return status.Error(codes.Internal, err.Error())
+			return status.Error(codes.Internal, err.Error())
+		}
 	}
 
 	return nil

internal/rbd/qos.go

@@ -19,6 +19,7 @@ package rbd
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strconv"
 
 	librbd "github.com/ceph/go-ceph/rbd"
@@ -117,9 +118,35 @@ func (h *nbdQoSHandler) HasParams(params map[string]string) bool {
 }
 
 // Validate validates traditional NBD QoS parameters.
-// Currently returns nil as NBD QoS validation is done during SetQOS.
 func (h *nbdQoSHandler) Validate(params map[string]string) error {
-	// NBD QoS validation is performed in SetQOS via calcQosBasedOnCapacity.
+	return validateNBDQoSParams(params)
+}
+
+// validateNBDQoSParams validates traditional NBD QoS parameters.
+// Ensures all numeric values are valid and positive.
+func validateNBDQoSParams(params map[string]string) error {
+	// All NBD QoS parameter keys that accept numeric values
+	numericParams := []string{
+		baseIops, maxIops, baseReadIops, maxReadIops, baseWriteIops, maxWriteIops,
+		baseBps, maxBps, baseReadBps, maxReadBps, baseWriteBps, maxWriteBps,
+		iopsPerGiB, readIopsPerGiB, writeIopsPerGiB,
+		bpsPerGiB, readBpsPerGiB, writeBpsPerGiB,
+		baseVolSizeBytes,
+	}
+
+	for _, key := range numericParams {
+		if val, ok := params[key]; ok && val != "" {
+			parsed, err := strconv.ParseInt(val, 10, 64)
+			if err != nil {
+				return fmt.Errorf("invalid value for %s: %s, must be a valid integer", key, val)
+			}
+			// Allow zero for base limits (can be omitted), but not negative
+			if parsed < 0 {
+				return fmt.Errorf("invalid value for %s: %s, must be non-negative", key, val)
+			}
+		}
+	}
+
 	return nil
 }