Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: remove global CSIInstanceID and pass configured value instead #4747

Merged
merged 2 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/cephcsi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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.
Expand Down
17 changes: 4 additions & 13 deletions internal/cephfs/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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
Expand All @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cephfs/groupcontrollerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type Config struct {
DriverName string
Namespace string
ClusterName string
InstanceID string
SetMetadata bool
}

Expand Down
1 change: 1 addition & 0 deletions internal/controller/persistentvolume/persistentvolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
25 changes: 21 additions & 4 deletions internal/csi-common/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")

Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions internal/nfs/controller/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion internal/nfs/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/rbd/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 2 additions & 11 deletions internal/rbd/globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
5 changes: 3 additions & 2 deletions internal/rbd/rbd_journal.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,8 @@ func RegenerateJournal(
volumeID,
requestName,
owner,
clusterName string,
clusterName,
instanceID string,
setMetadata bool,
cr *util.Credentials,
) (string, error) {
Expand Down Expand Up @@ -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
Expand Down