rbd: make pool optional in rbd sc if topologyconstraints are present

if rbd storage class is created with topologyconstraintspools replicated pool was still mandatory, making the pool optional if the topologyconstraintspools is requested Closes: #4380 Signed-off-by: parth-gr <[email protected]> Signed-off-by: parth-gr <[email protected]>
ceph · Mar 18, 2024 · 7a2be75 · 7a2be75
1 parent 83ec709
commit 7a2be75
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 9 deletions.
diff --git a/charts/ceph-csi-rbd/templates/storageclass.yaml b/charts/ceph-csi-rbd/templates/storageclass.yaml
@@ -16,8 +16,10 @@ metadata:
 provisioner: {{ .Values.driverName }}
 parameters:
   clusterID: {{ .Values.storageClass.clusterID }}
-  pool: {{ .Values.storageClass.pool }}
   imageFeatures: {{ .Values.storageClass.imageFeatures }}
+{{- if .Values.storageClass.pool }}  
+  pool: {{ .Values.storageClass.pool }}
+{{- end }}  
 {{- if .Values.storageClass.tryOtherMounters }}
   tryOtherMounters: {{ .Values.storageClass.tryOtherMounters | quote}}
 {{- end }}

diff --git a/charts/ceph-csi-rbd/values.yaml b/charts/ceph-csi-rbd/values.yaml
@@ -321,6 +321,7 @@ storageClass:
   dataPool: ""
 
   # (required) Ceph pool into which the RBD image shall be created
+  # (optional) if topologyConstrainedPools is provided
   # eg: pool: replicapool
   pool: replicapool
 

diff --git a/examples/rbd/storageclass.yaml b/examples/rbd/storageclass.yaml
@@ -26,6 +26,7 @@ parameters:
    # dataPool: <ec-data-pool>
 
    # (required) Ceph pool into which the RBD image shall be created
+   # (optional) If the topologyConstrainedPools is provided
    # eg: pool: rbdpool
    pool: <rbd-pool-name>
 

diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go
@@ -76,12 +76,19 @@ func (cs *ControllerServer) validateVolumeReq(ctx context.Context, req *csi.Crea
 	}
 	options := req.GetParameters()
 	if value, ok := options["clusterID"]; !ok || value == "" {
-		return status.Error(codes.InvalidArgument, "missing or empty cluster ID to provision volume from")
+		return status.Error(codes.InvalidArgument, "empty cluster ID to provision volume from")
 	}
-	if value, ok := options["pool"]; !ok || value == "" {
+	poolValue, poolOK := options["pool"]
+	topologyConstrainedPoolsValue, topologyOK := options["topologyConstrainedPools"]
+	if !poolOK {
+		if topologyOK && topologyConstrainedPoolsValue == "" {
+			return status.Error(codes.InvalidArgument, "empty pool name or topologyConstrainedPools to provision volume")
+		} else if !topologyOK {
+			return status.Error(codes.InvalidArgument, "missing or empty pool name to provision volume from")
+		}
+	} else if poolValue == "" {
 		return status.Error(codes.InvalidArgument, "missing or empty pool name to provision volume from")
 	}
-
 	if value, ok := options["dataPool"]; ok && value == "" {
 		return status.Error(codes.InvalidArgument, "empty datapool name to provision volume from")
 	}
@@ -244,14 +251,14 @@ func buildCreateVolumeResponse(req *csi.CreateVolumeRequest, rbdVol *rbdVolume)
 		VolumeContext: volumeContext,
 		ContentSource: req.GetVolumeContentSource(),
 	}
+
 	if rbdVol.Topology != nil {
 		volume.AccessibleTopology = []*csi.Topology{
 			{
 				Segments: rbdVol.Topology,
 			},
 		}
 	}
-
 	return &csi.CreateVolumeResponse{Volume: volume}
 }
 

diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go
@@ -72,10 +72,6 @@ func validateRbdVol(rbdVol *rbdVolume) error {
 		return err
 	}
 
-	if err = validateNonEmptyField(rbdVol.Pool, "Pool", "rbdVolume"); err != nil {
-		return err
-	}
-
 	if err = validateNonEmptyField(rbdVol.ClusterID, "ClusterID", "rbdVolume"); err != nil {
 		return err
 	}
@@ -287,6 +283,10 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
 	if rv.Topology != nil {
 		rv.Pool = imageData.ImagePool
 	}
+	// validate rbdvolume pool only after looking to the topology pools
+	if err = validateNonEmptyField(rv.Pool, "Pool", "rbdVolume"); err != nil {
+		return false, err
+	}
 
 	// NOTE: Return volsize should be on-disk volsize, not request vol size, so
 	// save it for size checks before fetching image data
@@ -433,6 +433,7 @@ func updateTopologyConstraints(rbdVol *rbdVolume, rbdSnap *rbdSnapshot) error {
 		if rbdVol.Topology != nil {
 			rbdVol.Pool = poolName
 			rbdVol.DataPool = dataPoolName
+			rbdVol.JournalPool = poolName
 		}
 
 		return nil
@@ -446,6 +447,7 @@ func updateTopologyConstraints(rbdVol *rbdVolume, rbdSnap *rbdSnapshot) error {
 		rbdVol.Pool = poolName
 		rbdVol.DataPool = dataPoolName
 		rbdVol.Topology = topology
+		rbdVol.JournalPool = poolName
 	}
 
 	return nil