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]>
ceph · Mar 8, 2024 · 31ae182 · 31ae182
1 parent 83ec709
commit 31ae182
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 22 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
@@ -79,9 +79,10 @@ func (cs *ControllerServer) validateVolumeReq(ctx context.Context, req *csi.Crea
 		return status.Error(codes.InvalidArgument, "missing or empty cluster ID to provision volume from")
 	}
 	if value, ok := options["pool"]; !ok || value == "" {
-		return status.Error(codes.InvalidArgument, "missing or empty pool name to provision volume from")
+		if value, ok := options["topologyConstrainedPools"]; !ok || value == "" {
+			return status.Error(codes.InvalidArgument, "missing or empty pool name and topologyConstrainedPools to provision volume from, provide either one")
+		}
 	}
-
 	if value, ok := options["dataPool"]; ok && value == "" {
 		return status.Error(codes.InvalidArgument, "empty datapool name to provision volume from")
 	}
@@ -225,33 +226,36 @@ func (cs *ControllerServer) parseVolCreateRequest(
 }
 
 func buildCreateVolumeResponse(req *csi.CreateVolumeRequest, rbdVol *rbdVolume) *csi.CreateVolumeResponse {
-	// remove kubernetes csi prefixed parameters.
-	volumeContext := k8s.RemoveCSIPrefixedParameters(req.GetParameters())
-	volumeContext["pool"] = rbdVol.Pool
-	volumeContext["journalPool"] = rbdVol.JournalPool
-	volumeContext["imageName"] = rbdVol.RbdImageName
-	if rbdVol.RadosNamespace != "" {
-		volumeContext["radosNamespace"] = rbdVol.RadosNamespace
-	}
-
-	if rbdVol.DataPool != "" {
-		volumeContext["dataPool"] = rbdVol.DataPool
-	}
-
 	volume := &csi.Volume{
 		VolumeId:      rbdVol.VolID,
 		CapacityBytes: rbdVol.VolSize,
-		VolumeContext: volumeContext,
 		ContentSource: req.GetVolumeContentSource(),
 	}
-	if rbdVol.Topology != nil {
-		volume.AccessibleTopology = []*csi.Topology{
+
+	// remove kubernetes csi prefixed parameters.
+	volumeContext := k8s.RemoveCSIPrefixedParameters(req.GetParameters())
+	volumeContext["journalPool"] = rbdVol.JournalPool
+	volumeContext["imageName"] = rbdVol.RbdImageName
+
+	if rbdVol.Topology == nil {
+		volumeContext["pool"] = rbdVol.Pool
+		if rbdVol.RadosNamespace != "" {
+			volumeContext["radosNamespace"] = rbdVol.RadosNamespace
+		}
+		if rbdVol.DataPool != "" {
+			volumeContext["dataPool"] = rbdVol.DataPool
+		}
+	} else {
+		accessibleTopology := []*csi.Topology{
 			{
 				Segments: rbdVol.Topology,
 			},
 		}
+		volume.AccessibleTopology = accessibleTopology
 	}
 
+	volume.VolumeContext = volumeContext
+
 	return &csi.CreateVolumeResponse{Volume: volume}
 }
 
@@ -1289,7 +1293,9 @@ func (cs *ControllerServer) validateSnapshotReq(ctx context.Context, req *csi.Cr
 		return status.Error(codes.InvalidArgument, "empty snapshot name prefix to provision snapshot from")
 	}
 	if value, ok := options["pool"]; ok && value == "" {
-		return status.Error(codes.InvalidArgument, "empty pool name in which rbd image will be created")
+		if value, ok := options["topologyConstrainedPools"]; !ok || value == "" {
+			return status.Error(codes.InvalidArgument, "missing or empty pool name and topologyConstrainedPools in which rbd image will be created")
+		}
 	}
 
 	return nil

diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go
@@ -584,7 +584,9 @@ func RegenerateJournal(
 	}
 
 	if rbdVol.Pool, ok = volumeAttributes["pool"]; !ok {
-		return "", errors.New("required 'pool' parameter missing in volume attributes")
+		if volumeAttributes["topologyConstrainedPools"] == "" {
+			return "", errors.New("required 'pool' and 'topologyConstrainedPools' parameter missing in volume attributes provide wither one")
+		}
 	}
 	err = rbdVol.Connect(cr)
 	if err != nil {

diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go
@@ -1272,7 +1272,9 @@ func genVolFromVolumeOptions(
 	rbdVol := &rbdVolume{}
 	rbdVol.Pool, ok = volOptions["pool"]
 	if !ok {
-		return nil, errors.New("missing required parameter pool")
+		if volOptions["topologyConstrainedPools"] == "" {
+			return nil, errors.New("missing required parameter pool and topologyConstrainedPools provide either one")
+		}
 	}
 
 	rbdVol.DataPool = volOptions["dataPool"]