From 7a2be757ae05ea5b6598696e77aff81651363619 Mon Sep 17 00:00:00 2001 From: parth-gr Date: Mon, 26 Feb 2024 19:29:58 +0530 Subject: [PATCH] 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: https://github.com/ceph/ceph-csi/issues/4380 Signed-off-by: parth-gr Signed-off-by: parth-gr --- charts/ceph-csi-rbd/templates/storageclass.yaml | 4 +++- charts/ceph-csi-rbd/values.yaml | 1 + examples/rbd/storageclass.yaml | 1 + internal/rbd/controllerserver.go | 15 +++++++++++---- internal/rbd/rbd_journal.go | 10 ++++++---- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/charts/ceph-csi-rbd/templates/storageclass.yaml b/charts/ceph-csi-rbd/templates/storageclass.yaml index 459a6ea312ac..188362fc0503 100644 --- 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 index 7dc4d019ab17..1f071a20afec 100644 --- 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 index ca7fc9a7247a..601a6696af4c 100644 --- a/examples/rbd/storageclass.yaml +++ b/examples/rbd/storageclass.yaml @@ -26,6 +26,7 @@ parameters: # dataPool: # (required) Ceph pool into which the RBD image shall be created + # (optional) If the topologyConstrainedPools is provided # eg: pool: rbdpool pool: diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index e3796b6611d2..aab30c127ad5 100644 --- 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,6 +251,7 @@ func buildCreateVolumeResponse(req *csi.CreateVolumeRequest, rbdVol *rbdVolume) VolumeContext: volumeContext, ContentSource: req.GetVolumeContentSource(), } + if rbdVol.Topology != nil { volume.AccessibleTopology = []*csi.Topology{ { @@ -251,7 +259,6 @@ func buildCreateVolumeResponse(req *csi.CreateVolumeRequest, rbdVol *rbdVolume) }, } } - return &csi.CreateVolumeResponse{Volume: volume} } diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 55c6a033ebfd..720f2861df74 100644 --- 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