feat: disable GET_VOLUME_STATS by default

fix test

Author: andyzhangx

charts/README.md

@@ -36,6 +36,8 @@ The following table lists the configurable parameters of the latest SMB CSI Driv
 
 | Parameter                                         | Description                                                | Default                                                           |
 |---------------------------------------------------|------------------------------------------------------------|-------------------------------------------------------------------|
+| `driver.name`                                     | alternative driver name                                    | `smb.csi.k8s.io` |
+| `feature.enableGetVolumeStats`                    | allow GET_VOLUME_STATS on agent node                  | `false`                      |
 | `image.baseRepo`                                  | base repository of driver images                          | `mcr.microsoft.com`                      |
 | `image.smb.repository`                            | csi-driver-smb docker image                                | `/k8s/csi/smb-csi`                                 |
 | `image.smb.tag`                                   | csi-driver-smb docker image tag                            | `latest`                                                            |

charts/latest/csi-driver-smb-v1.4.0.tgz

@@ -93,6 +93,7 @@ spec:
             - --endpoint=$(CSI_ENDPOINT)
             - --nodeid=$(KUBE_NODE_NAME)
             - "--metrics-address=0.0.0.0:{{ .Values.node.metricsPort }}"
+            - "--enable-get-volume-stats={{ .Values.feature.enableGetVolumeStats }}"
           ports:
             - containerPort: {{ .Values.node.livenessProbe.healthPort }}
               name: healthz

charts/latest/csi-driver-smb/templates/csi-smb-node-windows.yaml

@@ -101,6 +101,7 @@ spec:
             - "--endpoint=$(CSI_ENDPOINT)"
             - "--nodeid=$(KUBE_NODE_NAME)"
             - "--metrics-address=0.0.0.0:{{ .Values.node.metricsPort }}"
+            - "--enable-get-volume-stats={{ .Values.feature.enableGetVolumeStats }}"
           ports:
             - containerPort: {{ .Values.node.livenessProbe.healthPort }}
               name: healthz

charts/latest/csi-driver-smb/templates/csi-smb-node.yaml

@@ -28,6 +28,9 @@ rbac:
 driver:
   name: smb.csi.k8s.io
 
+feature:
+  enableGetVolumeStats: false
+
 controller:
   name: csi-smb-controller
   replicas: 2

charts/latest/csi-driver-smb/values.yaml

@@ -660,3 +660,45 @@ func TestNodeGetVolumeStats(t *testing.T) {
 	err := os.RemoveAll(fakePath)
 	assert.NoError(t, err)
 }
+
+func TestCheckGidPresentInMountFlags(t *testing.T) {
+	tests := []struct {
+		desc             string
+		VolumeMountGroup string
+		MountFlags       []string
+		expectedErr      error
+		result           bool
+	}{
+		{
+			desc:             "[Error] VolumeMountGroup is different from gid in mount options",
+			VolumeMountGroup: "2000",
+			MountFlags:       []string{"gid=3000"},
+			expectedErr:      status.Error(codes.InvalidArgument, "gid(3000) in storageClass and pod fsgroup(2000) are not equal"),
+			result:           false,
+		},
+		{
+			desc:             "[Success] Gid present in mount flags",
+			VolumeMountGroup: "",
+			MountFlags:       []string{"gid=3000"},
+			expectedErr:      nil,
+			result:           true,
+		},
+		{
+			desc:             "[Success] Gid not present in mount flags",
+			VolumeMountGroup: "",
+			MountFlags:       []string{},
+			expectedErr:      nil,
+			result:           false,
+		},
+	}
+
+	for _, test := range tests {
+		gIDPresent, err := checkGidPresentInMountFlags(test.VolumeMountGroup, test.MountFlags)
+		if !reflect.DeepEqual(err, test.expectedErr) {
+			t.Errorf("[%s]: Unexpected Error: %v, expected error: %v", test.desc, err, test.expectedErr)
+		}
+		if gIDPresent != test.result {
+			t.Errorf("[%s]: Expected result : %t, Actual result: %t", test.desc, test.result, gIDPresent)
+		}
+	}
+}

pkg/smb/nodeserver_test.go

@@ -39,17 +39,19 @@ type Driver struct {
 	mounter *mount.SafeFormatAndMount
 	// A map storing all volumes with ongoing operations so that additional operations
 	// for that same volume (as defined by VolumeID) return an Aborted error
-	volumeLocks     *volumeLocks
-	workingMountDir string
+	volumeLocks          *volumeLocks
+	workingMountDir      string
+	enableGetVolumeStats bool
 }
 
 // NewDriver 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 NewDriver(nodeID, driverName string) *Driver {
+func NewDriver(nodeID, driverName string, enableGetVolumeStats bool) *Driver {
 	driver := Driver{}
 	driver.Name = driverName
 	driver.Version = driverVersion
 	driver.NodeID = nodeID
+	driver.enableGetVolumeStats = enableGetVolumeStats
 	driver.volumeLocks = newVolumeLocks()
 	return &driver
 }
@@ -84,12 +86,15 @@ func (d *Driver) Run(endpoint, kubeconfig string, testMode bool) {
 		csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER,
 	})
 
-	d.AddNodeServiceCapabilities([]csi.NodeServiceCapability_RPC_Type{
-		csi.NodeServiceCapability_RPC_GET_VOLUME_STATS,
+	nodeCap := []csi.NodeServiceCapability_RPC_Type{
 		csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME,
 		csi.NodeServiceCapability_RPC_SINGLE_NODE_MULTI_WRITER,
 		csi.NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP,
-	})
+	}
+	if d.enableGetVolumeStats {
+		nodeCap = append(nodeCap, csi.NodeServiceCapability_RPC_GET_VOLUME_STATS)
+	}
+	d.AddNodeServiceCapabilities(nodeCap)
 
 	s := csicommon.NewNonBlockingGRPCServer()
 	// Driver d act as IdentityServer, ControllerServer and NodeServer

pkg/smb/smb.go

@@ -30,12 +30,12 @@ const (
 )
 
 func NewFakeDriver() *Driver {
-	return NewDriver(fakeNodeID, DefaultDriverName)
+	return NewDriver(fakeNodeID, DefaultDriverName, true)
 }
 
 func TestNewFakeDriver(t *testing.T) {
 	// Test New fake driver.
-	d := NewDriver(fakeNodeID, DefaultDriverName)
+	d := NewDriver(fakeNodeID, DefaultDriverName, true)
 	assert.NotNil(t, d)
 }
 

pkg/smb/smb_test.go

@@ -36,12 +36,13 @@ func init() {
 }
 
 var (
-	endpoint       = flag.String("endpoint", "unix://tmp/csi.sock", "CSI endpoint")
-	nodeID         = flag.String("nodeid", "", "node id")
-	driverName     = flag.String("drivername", smb.DefaultDriverName, "name of the driver")
-	version        = flag.Bool("version", false, "Print the version and exit.")
-	metricsAddress = flag.String("metrics-address", "0.0.0.0:29644", "export the metrics")
-	kubeconfig     = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
+	endpoint             = flag.String("endpoint", "unix://tmp/csi.sock", "CSI endpoint")
+	nodeID               = flag.String("nodeid", "", "node id")
+	driverName           = flag.String("drivername", smb.DefaultDriverName, "name of the driver")
+	version              = flag.Bool("version", false, "Print the version and exit.")
+	metricsAddress       = flag.String("metrics-address", "0.0.0.0:29644", "export the metrics")
+	kubeconfig           = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
+	enableGetVolumeStats = flag.Bool("enable-get-volume-stats", false, "allow GET_VOLUME_STATS on agent node")
 )
 
 func main() {
@@ -64,7 +65,7 @@ func main() {
 }
 
 func handle() {
-	driver := smb.NewDriver(*nodeID, *driverName)
+	driver := smb.NewDriver(*nodeID, *driverName, *enableGetVolumeStats)
 	if driver == nil {
 		klog.Fatalln("Failed to initialize smb CSI Driver")
 	}

pkg/smbplugin/main.go

@@ -100,7 +100,7 @@ var _ = ginkgo.BeforeSuite(func() {
 
 		nodeid := os.Getenv("nodeid")
 		kubeconfig := os.Getenv(kubeconfigEnvVar)
-		smbDriver = smb.NewDriver(nodeid, smb.DefaultDriverName)
+		smbDriver = smb.NewDriver(nodeid, smb.DefaultDriverName, false)
 		go func() {
 			smbDriver.Run(fmt.Sprintf("unix:///tmp/csi-%s.sock", uuid.NewUUID().String()), kubeconfig, false)
 		}()