From c3a8cea87eaf7b9b0d6d31230caa5ac6f7debd9f Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Fri, 2 Aug 2024 15:19:02 +0200 Subject: [PATCH 1/2] util: add CSIDriver.GetInstanceID() There has been some confusion about using different variables for the InstanceID of the RBD-driver. By removing the global variable CSIInstanceID, there should be no confusion anymore what variable to use. Signed-off-by: Niels de Vos --- cmd/cephcsi.go | 5 ++-- internal/controller/controller.go | 1 + .../persistentvolume/persistentvolume.go | 1 + internal/csi-common/driver.go | 25 ++++++++++++++++--- internal/nfs/controller/controllerserver.go | 4 +-- internal/nfs/driver/driver.go | 2 +- internal/rbd/driver/driver.go | 4 +-- internal/rbd/globals.go | 13 ++-------- internal/rbd/rbd_journal.go | 5 ++-- 9 files changed, 36 insertions(+), 24 deletions(-) diff --git a/cmd/cephcsi.go b/cmd/cephcsi.go index 9ef607be463..a43ac276362 100644 --- a/cmd/cephcsi.go +++ b/cmd/cephcsi.go @@ -70,8 +70,8 @@ func init() { flag.StringVar(&conf.StagingPath, "stagingpath", defaultStagingPath, "staging path") flag.StringVar(&conf.ClusterName, "clustername", "", "name of the cluster") flag.BoolVar(&conf.SetMetadata, "setmetadata", false, "set metadata on the volume") - flag.StringVar(&conf.InstanceID, "instanceid", "", "Unique ID distinguishing this instance of Ceph CSI among other"+ - " instances, when sharing Ceph clusters across CSI instances for provisioning") + flag.StringVar(&conf.InstanceID, "instanceid", "default", "Unique ID distinguishing this instance of Ceph-CSI"+ + " among other instances, when sharing Ceph clusters across CSI instances for provisioning") flag.IntVar(&conf.PidLimit, "pidlimit", 0, "the PID limit to configure through cgroups") flag.BoolVar(&conf.IsControllerServer, "controllerserver", false, "start cephcsi controller server") flag.BoolVar(&conf.IsNodeServer, "nodeserver", false, "start cephcsi node server") @@ -249,6 +249,7 @@ func main() { DriverName: dname, Namespace: conf.DriverNamespace, ClusterName: conf.ClusterName, + InstanceID: conf.InstanceID, SetMetadata: conf.SetMetadata, } // initialize all controllers before starting. diff --git a/internal/controller/controller.go b/internal/controller/controller.go index c1707f14c85..37066045e91 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -39,6 +39,7 @@ type Config struct { DriverName string Namespace string ClusterName string + InstanceID string SetMetadata bool } diff --git a/internal/controller/persistentvolume/persistentvolume.go b/internal/controller/persistentvolume/persistentvolume.go index 433bf616b2f..07ff02d8f11 100644 --- a/internal/controller/persistentvolume/persistentvolume.go +++ b/internal/controller/persistentvolume/persistentvolume.go @@ -189,6 +189,7 @@ func (r *ReconcilePersistentVolume) reconcilePV(ctx context.Context, obj runtime requestName, pvcNamespace, r.config.ClusterName, + r.config.InstanceID, r.config.SetMetadata, cr) if err != nil { diff --git a/internal/csi-common/driver.go b/internal/csi-common/driver.go index 1100f05a3af..bfefba1015d 100644 --- a/internal/csi-common/driver.go +++ b/internal/csi-common/driver.go @@ -30,6 +30,11 @@ type CSIDriver struct { name string nodeID string version string + + // instance is the instance ID that is unique to an instance of CSI, used when sharing + // ceph clusters across CSI instances, to differentiate omap names per CSI instance. + instance string + // topology constraints that this nodeserver will advertise topology map[string]string capabilities []*csi.ControllerServiceCapability @@ -40,7 +45,7 @@ type CSIDriver struct { // NewCSIDriver Creates a NewCSIDriver object. Assumes vendor // version is equal to driver version & does not support optional // driver plugin info manifest field. Refer to CSI spec for more details. -func NewCSIDriver(name, v, nodeID string) *CSIDriver { +func NewCSIDriver(name, v, nodeID, instance string) *CSIDriver { if name == "" { klog.Errorf("Driver name missing") @@ -59,15 +64,27 @@ func NewCSIDriver(name, v, nodeID string) *CSIDriver { return nil } + if instance == "" { + klog.Errorf("Instance argument missing") + + return nil + } + driver := CSIDriver{ - name: name, - version: v, - nodeID: nodeID, + name: name, + version: v, + nodeID: nodeID, + instance: instance, } return &driver } +// GetInstance returns the instance identification of the CSI driver. +func (d *CSIDriver) GetInstanceID() string { + return d.instance +} + // ValidateControllerServiceRequest validates the controller // plugin capabilities. func (d *CSIDriver) ValidateControllerServiceRequest(c csi.ControllerServiceCapability_RPC_Type) error { diff --git a/internal/nfs/controller/controllerserver.go b/internal/nfs/controller/controllerserver.go index 15c610131a7..125330284d2 100644 --- a/internal/nfs/controller/controllerserver.go +++ b/internal/nfs/controller/controllerserver.go @@ -45,8 +45,8 @@ type Server struct { // NewControllerServer initialize a controller server for ceph CSI driver. func NewControllerServer(d *csicommon.CSIDriver) *Server { // global instance of the volume journal, yuck - store.VolJournal = journal.NewCSIVolumeJournalWithNamespace(cephfs.CSIInstanceID, fsutil.RadosNamespace) - store.SnapJournal = journal.NewCSISnapshotJournalWithNamespace(cephfs.CSIInstanceID, fsutil.RadosNamespace) + store.VolJournal = journal.NewCSIVolumeJournalWithNamespace(d.GetInstanceID(), fsutil.RadosNamespace) + store.SnapJournal = journal.NewCSISnapshotJournalWithNamespace(d.GetInstanceID(), fsutil.RadosNamespace) return &Server{ backendServer: cephfs.NewControllerServer(d), diff --git a/internal/nfs/driver/driver.go b/internal/nfs/driver/driver.go index ecefa55a946..51eefc568e4 100644 --- a/internal/nfs/driver/driver.go +++ b/internal/nfs/driver/driver.go @@ -39,7 +39,7 @@ func NewDriver() *Driver { // ceph CSI driver which can serve multiple parallel requests. func (fs *Driver) Run(conf *util.Config) { // Initialize default library driver - cd := csicommon.NewCSIDriver(conf.DriverName, util.DriverVersion, conf.NodeID) + cd := csicommon.NewCSIDriver(conf.DriverName, util.DriverVersion, conf.NodeID, conf.InstanceID) if cd == nil { log.FatalLogMsg("failed to initialize CSI driver") } diff --git a/internal/rbd/driver/driver.go b/internal/rbd/driver/driver.go index c1db88fa908..cebfc53cf6b 100644 --- a/internal/rbd/driver/driver.go +++ b/internal/rbd/driver/driver.go @@ -100,7 +100,7 @@ func (r *Driver) Run(conf *util.Config) { rbd.InitJournals(conf.InstanceID) // Initialize default library driver - r.cd = csicommon.NewCSIDriver(conf.DriverName, util.DriverVersion, conf.NodeID) + r.cd = csicommon.NewCSIDriver(conf.DriverName, util.DriverVersion, conf.NodeID, conf.InstanceID) if r.cd == nil { log.FatalLogMsg("Failed to initialize CSI Driver.") } @@ -217,7 +217,7 @@ func (r *Driver) setupCSIAddonsServer(conf *util.Config) error { fcs := casrbd.NewFenceControllerServer() r.cas.RegisterService(fcs) - rcs := casrbd.NewReplicationServer(rbd.CSIInstanceID, NewControllerServer(r.cd)) + rcs := casrbd.NewReplicationServer(conf.InstanceID, NewControllerServer(r.cd)) r.cas.RegisterService(rcs) vgcs := casrbd.NewVolumeGroupServer(conf.InstanceID) diff --git a/internal/rbd/globals.go b/internal/rbd/globals.go index b1a6ef3dced..c16a11d3e9c 100644 --- a/internal/rbd/globals.go +++ b/internal/rbd/globals.go @@ -23,10 +23,6 @@ import ( ) var ( - // CSIInstanceID is the instance ID that is unique to an instance of CSI, used when sharing - // ceph clusters across CSI instances, to differentiate omap names per CSI instance. - CSIInstanceID = "default" - // volJournal and snapJournal are used to maintain RADOS based journals for CO generated // VolumeName to backing RBD images. volJournal *journal.Config @@ -91,11 +87,6 @@ func SetGlobalBool(name string, value bool) { // NodeService where appropriate. Using global journals limits the ability to // configure these options based on the Ceph cluster or StorageClass. func InitJournals(instance string) { - // Use passed in instance ID, if provided for omap suffix naming - if instance != "" { - CSIInstanceID = instance - } - - volJournal = journal.NewCSIVolumeJournal(CSIInstanceID) - snapJournal = journal.NewCSISnapshotJournal(CSIInstanceID) + volJournal = journal.NewCSIVolumeJournal(instance) + snapJournal = journal.NewCSISnapshotJournal(instance) } diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 78f5d5fdde6..44c951c5338 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -547,7 +547,8 @@ func RegenerateJournal( volumeID, requestName, owner, - clusterName string, + clusterName, + instanceID string, setMetadata bool, cr *util.Credentials, ) (string, error) { @@ -595,7 +596,7 @@ func RegenerateJournal( if rbdVol.JournalPool == "" { rbdVol.JournalPool = rbdVol.Pool } - volJournal = journal.NewCSIVolumeJournal(CSIInstanceID) + volJournal = journal.NewCSIVolumeJournal(instanceID) j, err := volJournal.Connect(rbdVol.Monitors, rbdVol.RadosNamespace, cr) if err != nil { return "", err From 616c563b27caf7bfd0b81bfa189f1509b2ff25fe Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Mon, 5 Aug 2024 16:05:24 +0200 Subject: [PATCH 2/2] cephfs: use conf.InstanceID instead of global variable RBD does not have a global CSIInstanceID variable anymore, there is no need for CephFS to use one either. Signed-off-by: Niels de Vos --- internal/cephfs/driver.go | 17 ++++------------- internal/cephfs/groupcontrollerserver_test.go | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/internal/cephfs/driver.go b/internal/cephfs/driver.go index 162b027ee2f..97509956d77 100644 --- a/internal/cephfs/driver.go +++ b/internal/cephfs/driver.go @@ -45,10 +45,6 @@ type Driver struct { cas *csiaddons.CSIAddonsServer } -// CSIInstanceID is the instance ID that is unique to an instance of CSI, used when sharing -// ceph clusters across CSI instances, to differentiate omap names per CSI instance. -var CSIInstanceID = "default" - // NewDriver returns new ceph driver. func NewDriver() *Driver { return &Driver{} @@ -105,11 +101,6 @@ func (fs *Driver) Run(conf *util.Config) { log.FatalLogMsg("cephfs: failed to load ceph mounters: %v", err) } - // Use passed in instance ID, if provided for omap suffix naming - if conf.InstanceID != "" { - CSIInstanceID = conf.InstanceID - } - // Use passed in radosNamespace, if provided for storing CSI specific objects and keys. if conf.RadosNamespaceCephFS != "" { fsutil.RadosNamespace = conf.RadosNamespaceCephFS @@ -127,16 +118,16 @@ func (fs *Driver) Run(conf *util.Config) { } // Create an instance of the volume journal - store.VolJournal = journal.NewCSIVolumeJournalWithNamespace(CSIInstanceID, fsutil.RadosNamespace) + store.VolJournal = journal.NewCSIVolumeJournalWithNamespace(conf.InstanceID, fsutil.RadosNamespace) - store.SnapJournal = journal.NewCSISnapshotJournalWithNamespace(CSIInstanceID, fsutil.RadosNamespace) + store.SnapJournal = journal.NewCSISnapshotJournalWithNamespace(conf.InstanceID, fsutil.RadosNamespace) store.VolumeGroupJournal = journal.NewCSIVolumeGroupJournalWithNamespace( - CSIInstanceID, + conf.InstanceID, fsutil.RadosNamespace) // Initialize default library driver - fs.cd = csicommon.NewCSIDriver(conf.DriverName, util.DriverVersion, conf.NodeID) + fs.cd = csicommon.NewCSIDriver(conf.DriverName, util.DriverVersion, conf.NodeID, conf.InstanceID) if fs.cd == nil { log.FatalLogMsg("failed to initialize CSI driver") } diff --git a/internal/cephfs/groupcontrollerserver_test.go b/internal/cephfs/groupcontrollerserver_test.go index 6aacc223bb7..a505064c7f1 100644 --- a/internal/cephfs/groupcontrollerserver_test.go +++ b/internal/cephfs/groupcontrollerserver_test.go @@ -31,7 +31,7 @@ func TestControllerServer_validateCreateVolumeGroupSnapshotRequest(t *testing.T) t.Parallel() cs := ControllerServer{ DefaultControllerServer: csicommon.NewDefaultControllerServer( - csicommon.NewCSIDriver("cephfs.csi.ceph.com", "1.0.0", "test")), + csicommon.NewCSIDriver("cephfs.csi.ceph.com", "1.0.0", "test", "default")), } type args struct {