feat: Split controller and node mode, add max volumes per node flag

BREAKING CHANGE: switched flag library, renamed flags and introduced mode argument

Author: hrak

Makefile

@@ -1,5 +1,6 @@
 CMDS=cloudstack-csi-driver cloudstack-csi-sc-syncer
 
+PKG=github.com/leaseweb/cloudstack-csi-driver
 # Revision that gets built into each binary via the main.version
 # string. Uses the `git describe` output based on the most recent
 # version tag with a short revision suffix or, if nothing has been
@@ -8,10 +9,12 @@ CMDS=cloudstack-csi-driver cloudstack-csi-sc-syncer
 # Beware that tags may also be missing in shallow clones as done by
 # some CI systems (like TravisCI, which pulls only 50 commits).
 REV=$(shell git describe --long --tags --match='v*' --dirty 2>/dev/null || git rev-list -n1 HEAD)
+GIT_COMMIT?=$(shell git rev-parse HEAD)
+BUILD_DATE?=$(shell date -u -Iseconds)
 
 DOCKER?=docker
 
-IMPORTPATH_LDFLAGS = -X main.version=$(REV) 
+IMPORTPATH_LDFLAGS = -X ${PKG}/pkg/driver.driverVersion=$(REV) -X ${PKG}/pkg/driver.gitCommit=${GIT_COMMIT} -X ${PKG}/pkg/driver.buildDate=${BUILD_DATE}
 LDFLAGS = -s -w
 FULL_LDFLAGS = $(LDFLAGS) $(IMPORTPATH_LDFLAGS)
 

charts/cloudstack-csi/templates/csi-controller-deploy.yaml

@@ -43,8 +43,9 @@ spec:
           image: "{{ .Values.controller.csiDriverController.image.repository }}:{{ .Values.controller.csiDriverController.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.controller.csiDriverController.image.pullPolicy }}
           args:
+            - "controller"
             - "--endpoint=$(CSI_ENDPOINT)"
-            - "--cloudstackconfig=$(CLOUD_CONFIG)"
+            - "--cloudstack-config=$(CLOUD_CONFIG)"
             - "--logging-format={{ .Values.logFormat }}"
             - "--v={{ .Values.logVerbosityLevel }}"
             {{- if .Values.controller.csiDriverController.extraArgs }}

charts/cloudstack-csi/templates/csi-node-ds.yaml

@@ -31,10 +31,11 @@ spec:
           image: "{{ .Values.node.csiDriver.image.repository }}:{{ .Values.node.csiDriver.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.node.csiDriver.image.pullPolicy }}
           args:
+            - "node"
             - "--endpoint=$(CSI_ENDPOINT)"
-            - "--cloudstackconfig=$(CLOUD_CONFIG)"
+            - "--cloudstack-config=$(CLOUD_CONFIG)"
             - "--logging-format={{ .Values.logFormat }}"
-            - "--nodeName=$(NODE_NAME)"
+            - "--node-name=$(NODE_NAME)"
             - "--v={{ .Values.logVerbosityLevel }}"
             {{- if .Values.node.csiDriver.extraArgs }}
             {{- with .Values.node.csiDriver.extraArgs }}

cmd/cloudstack-csi-driver/main.go

@@ -7,11 +7,11 @@ package main
 
 import (
 	"context"
-	"flag"
 	"fmt"
 	"os"
-	"path"
+	"strings"
 
+	flag "github.com/spf13/pflag"
 	"k8s.io/component-base/featuregate"
 	"k8s.io/component-base/logs"
 	logsapi "k8s.io/component-base/logs/api/v1"
@@ -22,30 +22,51 @@ import (
 	"github.com/leaseweb/cloudstack-csi-driver/pkg/driver"
 )
 
-var (
-	endpoint         = flag.String("endpoint", "unix:///tmp/csi.sock", "CSI endpoint")
-	cloudstackconfig = flag.String("cloudstackconfig", "./cloud-config", "CloudStack configuration file")
-	nodeName         = flag.String("nodeName", "", "Node name")
-	showVersion      = flag.Bool("version", false, "Show version")
-
-	// Version is set by the build process.
-	version = ""
-)
-
 func main() {
+	fs := flag.NewFlagSet("cloudstack-csi-driver", flag.ExitOnError)
 	if err := logsapi.RegisterLogFormat(logsapi.JSONLogFormat, json.Factory{}, logsapi.LoggingBetaOptions); err != nil {
 		klog.ErrorS(err, "failed to register JSON log format")
 	}
 
+	var (
+		showVersion = fs.Bool("version", false, "Show version")
+		args        = os.Args[1:]
+		cmd         = string(driver.AllMode)
+		options     = driver.Options{}
+	)
 	fg := featuregate.NewFeatureGate()
 	err := logsapi.AddFeatureGates(fg)
 	if err != nil {
 		klog.ErrorS(err, "failed to add feature gates")
 	}
 
 	c := logsapi.NewLoggingConfiguration()
-	logsapi.AddGoFlags(c, flag.CommandLine)
-	flag.Parse()
+	logsapi.AddFlags(c, fs)
+
+	if len(os.Args) > 1 && !strings.HasPrefix(os.Args[1], "-") {
+		cmd = os.Args[1]
+		args = os.Args[2:]
+	}
+
+	switch cmd {
+	case string(driver.ControllerMode), string(driver.NodeMode), string(driver.AllMode):
+		options.Mode = driver.Mode(cmd)
+	default:
+		klog.Errorf("Unknown driver mode %s: Expected %s, %s, or %s", cmd, driver.ControllerMode, driver.NodeMode, driver.AllMode)
+		klog.FlushAndExit(klog.ExitFlushTimeout, 0)
+	}
+
+	options.AddFlags(fs)
+
+	if err = fs.Parse(args); err != nil {
+		klog.ErrorS(err, "Failed to parse options")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 0)
+	}
+	if err = options.Validate(); err != nil {
+		klog.ErrorS(err, "Invalid options")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 0)
+	}
+
 	logs.InitLogs()
 	logger := klog.Background()
 	if err = logsapi.ValidateAndApply(c, fg); err != nil {
@@ -54,23 +75,27 @@ func main() {
 	}
 
 	if *showVersion {
-		baseName := path.Base(os.Args[0])
-		fmt.Println(baseName, version) //nolint:forbidigo
+		versionInfo, versionErr := driver.GetVersionJSON()
+		if versionErr != nil {
+			logger.Error(err, "failed to get version")
+			klog.FlushAndExit(klog.ExitFlushTimeout, 1)
+		}
+		fmt.Println(versionInfo) //nolint:forbidigo
 		os.Exit(0)
 	}
 
 	// Setup cloud connector.
-	config, err := cloud.ReadConfig(*cloudstackconfig)
+	config, err := cloud.ReadConfig(options.CloudStackConfig)
 	if err != nil {
 		logger.Error(err, "Cannot read CloudStack configuration")
 		klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 	}
-	logger.Info("Successfully read CloudStack configuration", "cloudstackconfig", *cloudstackconfig)
+	logger.Info("Successfully read CloudStack configuration", "cloudstackconfig", options.CloudStackConfig)
 
 	ctx := klog.NewContext(context.Background(), logger)
 	csConnector := cloud.New(config)
 
-	d, err := driver.New(*endpoint, csConnector, nil, *nodeName, version)
+	d, err := driver.New(ctx, csConnector, &options, nil)
 	if err != nil {
 		logger.Error(err, "Failed to initialize driver")
 		klog.FlushAndExit(klog.ExitFlushTimeout, 1)

deploy/k8s/controller-deployment.yaml

@@ -37,10 +37,11 @@ spec:
           image: cloudstack-csi-driver
           imagePullPolicy: Always
           args:
-            - "-endpoint=$(CSI_ENDPOINT)"
-            - "-cloudstackconfig=/etc/cloudstack-csi-driver/cloud-config"
-            - "-logging-format=text"
-            - "-v=4"
+            - "controller"
+            - "--endpoint=$(CSI_ENDPOINT)"
+            - "--cloudstack-config=/etc/cloudstack-csi-driver/cloud-config"
+            - "--logging-format=text"
+            - "--v=4"
           env:
             - name: CSI_ENDPOINT
               value: unix:///var/lib/csi/sockets/pluginproxy/csi.sock

deploy/k8s/node-daemonset.yaml

@@ -29,11 +29,12 @@ spec:
           image: cloudstack-csi-driver
           imagePullPolicy: Always
           args:
-            - "-endpoint=$(CSI_ENDPOINT)"
-            - "-cloudstackconfig=/etc/cloudstack-csi-driver/cloud-config"
-            - "-logging-format=text"
-            - "-nodeName=$(NODE_NAME)"
-            - "-v=4"
+            - "node"
+            - "--endpoint=$(CSI_ENDPOINT)"
+            - "--cloudstack-config=/etc/cloudstack-csi-driver/cloud-config"
+            - "--logging-format=text"
+            - "--node-name=$(NODE_NAME)"
+            - "--v=4"
           env:
             - name: CSI_ENDPOINT
               value: unix:///csi/csi.sock

pkg/driver/constants.go

@@ -3,6 +3,38 @@ package driver
 // DriverName is the name of the CSI plugin.
 const DriverName = "csi.cloudstack.apache.org"
 
+// Mode is the operating mode of the CSI driver.
+type Mode string
+
+// Driver operating modes.
+const (
+	// ControllerMode is the mode that only starts the controller service.
+	ControllerMode Mode = "controller"
+	// NodeMode is the mode that only starts the node service.
+	NodeMode Mode = "node"
+	// AllMode is the mode that only starts both the controller and the node service.
+	AllMode Mode = "all"
+)
+
+// constants for default command line flag values.
+const (
+	// DefaultCSIEndpoint is the default CSI endpoint for the driver.
+	DefaultCSIEndpoint             = "unix://tmp/csi.sock"
+	DefaultMaxVolAttachLimit int64 = 256
+)
+
+// Filesystem types.
+const (
+	// FSTypeExt2 represents the ext2 filesystem type.
+	FSTypeExt2 = "ext2"
+	// FSTypeExt3 represents the ext3 filesystem type.
+	FSTypeExt3 = "ext3"
+	// FSTypeExt4 represents the ext4 filesystem type.
+	FSTypeExt4 = "ext4"
+	// FSTypeXfs represents the xfs filesystem type.
+	FSTypeXfs = "xfs"
+)
+
 // Topology keys.
 const (
 	ZoneKey = "topology." + DriverName + "/zone"

pkg/driver/driver.go

@@ -5,6 +5,10 @@ package driver
 
 import (
 	"context"
+	"fmt"
+
+	"github.com/container-storage-interface/spec/lib/go/csi"
+	"k8s.io/klog/v2"
 
 	"github.com/leaseweb/cloudstack-csi-driver/pkg/cloud"
 	"github.com/leaseweb/cloudstack-csi-driver/pkg/mount"
@@ -17,29 +21,49 @@ type Interface interface {
 }
 
 type cloudstackDriver struct {
-	endpoint string
-	nodeName string
-	version  string
-
-	connector cloud.Interface
-	mounter   mount.Interface
+	controller csi.ControllerServer
+	identity   csi.IdentityServer
+	node       csi.NodeServer
+	options    *Options
 }
 
 // New instantiates a new CloudStack CSI driver.
-func New(endpoint string, csConnector cloud.Interface, mounter mount.Interface, nodeName string, version string) (Interface, error) {
-	return &cloudstackDriver{
-		endpoint:  endpoint,
-		nodeName:  nodeName,
-		version:   version,
-		connector: csConnector,
-		mounter:   mounter,
-	}, nil
+func New(ctx context.Context, csConnector cloud.Interface, options *Options, mounter mount.Interface) (Interface, error) {
+	logger := klog.FromContext(ctx)
+	logger.Info("Driver starting", "Driver", DriverName, "Version", driverVersion)
+
+	if err := validateMode(options.Mode); err != nil {
+		return nil, fmt.Errorf("invalid driver options: %w", err)
+	}
+
+	driver := &cloudstackDriver{
+		options: options,
+	}
+
+	driver.identity = NewIdentityServer(driverVersion)
+	switch options.Mode {
+	case ControllerMode:
+		driver.controller = NewControllerServer(csConnector)
+	case NodeMode:
+		driver.node = NewNodeServer(csConnector, mounter, options)
+	case AllMode:
+		driver.controller = NewControllerServer(csConnector)
+		driver.node = NewNodeServer(csConnector, mounter, options)
+	default:
+		return nil, fmt.Errorf("unknown mode: %s", options.Mode)
+	}
+
+	return driver, nil
 }
 
 func (cs *cloudstackDriver) Run(ctx context.Context) error {
-	ids := NewIdentityServer(cs.version)
-	ctrls := NewControllerServer(cs.connector)
-	ns := NewNodeServer(cs.connector, cs.mounter, cs.nodeName)
+	return cs.serve(ctx)
+}
+
+func validateMode(mode Mode) error {
+	if mode != AllMode && mode != ControllerMode && mode != NodeMode {
+		return fmt.Errorf("mode is not supported (actual: %s, supported: %v)", mode, []Mode{AllMode, ControllerMode, NodeMode})
+	}
 
-	return cs.serve(ctx, ids, ctrls, ns)
+	return nil
 }

pkg/driver/node.go

@@ -19,28 +19,30 @@ import (
 
 const (
 	// default file system type to be used when it is not provided.
-	defaultFsType = "ext4"
+	defaultFsType = FSTypeExt4
 )
 
 type nodeServer struct {
 	csi.UnimplementedNodeServer
-	connector   cloud.Interface
-	mounter     mount.Interface
-	nodeName    string
-	volumeLocks *util.VolumeLocks
+	connector         cloud.Interface
+	mounter           mount.Interface
+	maxVolumesPerNode int64
+	nodeName          string
+	volumeLocks       *util.VolumeLocks
 }
 
 // NewNodeServer creates a new Node gRPC server.
-func NewNodeServer(connector cloud.Interface, mounter mount.Interface, nodeName string) csi.NodeServer {
+func NewNodeServer(connector cloud.Interface, mounter mount.Interface, options *Options) csi.NodeServer {
 	if mounter == nil {
 		mounter = mount.New()
 	}
 
 	return &nodeServer{
-		connector:   connector,
-		mounter:     mounter,
-		nodeName:    nodeName,
-		volumeLocks: util.NewVolumeLocks(),
+		connector:         connector,
+		mounter:           mounter,
+		maxVolumesPerNode: options.VolumeAttachLimit,
+		nodeName:          options.NodeName,
+		volumeLocks:       util.NewVolumeLocks(),
 	}
 }
 
@@ -392,6 +394,7 @@ func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoReque
 	return &csi.NodeGetInfoResponse{
 		NodeId:             vm.ID,
 		AccessibleTopology: topology.ToCSI(),
+		MaxVolumesPerNode:  ns.maxVolumesPerNode,
 	}, nil
 }