diff --git a/charts/README.md b/charts/README.md index b076001ec7e..06b11c5a0dd 100644 --- a/charts/README.md +++ b/charts/README.md @@ -103,6 +103,7 @@ The following table lists the configurable parameters of the latest SMB CSI Driv | `linux.resources.smb.requests.memory` | smb-csi-driver memory requests limits | `20Mi` | `windows.enabled` | whether enable windows feature | `false` | | `windows.dsName` | name of driver daemonset on windows | `csi-smb-node-win` | +| `windows.removeSMBMappingDuringUnmount` | remove SMBMapping during unmount on Windows node windows | `true` | | `windows.resources.livenessProbe.limits.memory` | liveness-probe memory limits | `200Mi` | | `windows.resources.livenessProbe.requests.cpu` | liveness-probe cpu requests limits | `10m` | | `windows.resources.livenessProbe.requests.memory` | liveness-probe memory requests limits | `20Mi` | diff --git a/charts/latest/csi-driver-smb-v0.0.0.tgz b/charts/latest/csi-driver-smb-v0.0.0.tgz index 6e2fcd7d2dd..b412574ac01 100644 Binary files a/charts/latest/csi-driver-smb-v0.0.0.tgz and b/charts/latest/csi-driver-smb-v0.0.0.tgz differ diff --git a/charts/latest/csi-driver-smb/templates/csi-smb-node-windows.yaml b/charts/latest/csi-driver-smb/templates/csi-smb-node-windows.yaml index 0d61b251bae..3d55a7ff66e 100755 --- a/charts/latest/csi-driver-smb/templates/csi-smb-node-windows.yaml +++ b/charts/latest/csi-driver-smb/templates/csi-smb-node-windows.yaml @@ -102,6 +102,7 @@ spec: - --nodeid=$(KUBE_NODE_NAME) - "--metrics-address=0.0.0.0:{{ .Values.node.metricsPort }}" - "--enable-get-volume-stats={{ .Values.feature.enableGetVolumeStats }}" + - "--remove-smb-mapping-during-unmount={{ .Values.windows.removeSMBMappingDuringUnmount }}" ports: - containerPort: {{ .Values.node.livenessProbe.healthPort }} name: healthz diff --git a/charts/latest/csi-driver-smb/values.yaml b/charts/latest/csi-driver-smb/values.yaml index 7d94eb2fb0a..313114f6c65 100755 --- a/charts/latest/csi-driver-smb/values.yaml +++ b/charts/latest/csi-driver-smb/values.yaml @@ -115,6 +115,7 @@ windows: enabled: false dsName: csi-smb-node-win # daemonset name kubelet: 'C:\var\lib\kubelet' + removeSMBMappingDuringUnmount: true tolerations: - key: "node.kubernetes.io/os" operator: "Exists" diff --git a/cmd/smbplugin/main.go b/cmd/smbplugin/main.go index 59aa741ab1c..08229b8be1f 100644 --- a/cmd/smbplugin/main.go +++ b/cmd/smbplugin/main.go @@ -36,14 +36,15 @@ 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") - ver = flag.Bool("ver", 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", true, "allow GET_VOLUME_STATS on agent node") - workingMountDir = flag.String("working-mount-dir", "/tmp", "working directory for provisioner to mount smb shares temporarily") + 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") + ver = flag.Bool("ver", 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", true, "allow GET_VOLUME_STATS on agent node") + removeSMBMappingDuringUnmount = flag.Bool("remove-smb-mapping-during-unmount", true, "remove SMBMapping during unmount on Windows node") + workingMountDir = flag.String("working-mount-dir", "/tmp", "working directory for provisioner to mount smb shares temporarily") ) func main() { @@ -67,10 +68,11 @@ func main() { func handle() { driverOptions := smb.DriverOptions{ - NodeID: *nodeID, - DriverName: *driverName, - EnableGetVolumeStats: *enableGetVolumeStats, - WorkingMountDir: *workingMountDir, + NodeID: *nodeID, + DriverName: *driverName, + EnableGetVolumeStats: *enableGetVolumeStats, + RemoveSMBMappingDuringUnmount: *removeSMBMappingDuringUnmount, + WorkingMountDir: *workingMountDir, } driver := smb.NewDriver(&driverOptions) driver.Run(*endpoint, *kubeconfig, false) diff --git a/pkg/csi-common/server_test.go b/pkg/csi-common/server_test.go index c7aa2e04027..6816f5d1c36 100644 --- a/pkg/csi-common/server_test.go +++ b/pkg/csi-common/server_test.go @@ -33,8 +33,9 @@ func TestNewNonBlockingGRPCServer(t *testing.T) { func TestStart(t *testing.T) { s := NewNonBlockingGRPCServer() // sleep a while to avoid race condition in unit test - time.Sleep(time.Millisecond * 2000) + time.Sleep(time.Millisecond * 500) s.Start("tcp://127.0.0.1:0", nil, nil, nil, true) + time.Sleep(time.Millisecond * 500) } func TestServe(t *testing.T) { diff --git a/pkg/mounter/safe_mounter_unix.go b/pkg/mounter/safe_mounter_unix.go index d5ee4d14a84..2cdb245a067 100644 --- a/pkg/mounter/safe_mounter_unix.go +++ b/pkg/mounter/safe_mounter_unix.go @@ -24,7 +24,7 @@ import ( utilexec "k8s.io/utils/exec" ) -func NewSafeMounter() (*mount.SafeFormatAndMount, error) { +func NewSafeMounter(removeSMBMappingDuringUnmount bool) (*mount.SafeFormatAndMount, error) { return &mount.SafeFormatAndMount{ Interface: mount.New(""), Exec: utilexec.New(), diff --git a/pkg/mounter/safe_mounter_unix_test.go b/pkg/mounter/safe_mounter_unix_test.go index cac54b7153c..7991c4736b9 100644 --- a/pkg/mounter/safe_mounter_unix_test.go +++ b/pkg/mounter/safe_mounter_unix_test.go @@ -23,7 +23,7 @@ import ( ) func TestNewSafeMounter(t *testing.T) { - resp, err := NewSafeMounter() + resp, err := NewSafeMounter(true) assert.NotNil(t, resp) assert.Nil(t, err) } diff --git a/pkg/mounter/safe_mounter_windows.go b/pkg/mounter/safe_mounter_windows.go index e67bb0a3dc4..a6808d92454 100644 --- a/pkg/mounter/safe_mounter_windows.go +++ b/pkg/mounter/safe_mounter_windows.go @@ -55,8 +55,9 @@ type CSIProxyMounter interface { var _ CSIProxyMounter = &csiProxyMounter{} type csiProxyMounter struct { - FsClient *fsclient.Client - SMBClient *smbclient.Client + FsClient *fsclient.Client + SMBClient *smbclient.Client + RemoveSMBMappingDuringUnmount bool } func normalizeWindowsPath(path string) string { @@ -101,12 +102,9 @@ func (mounter *csiProxyMounter) SMBMount(source, target, fsType string, mountOpt } source = strings.Replace(source, "/", "\\", -1) - if strings.HasSuffix(source, "\\") { - source = strings.TrimSuffix(source, "\\") - } - + source = strings.TrimSuffix(source, "\\") mappingPath, err := getRootMappingPath(source) - if err != nil { + if mounter.RemoveSMBMappingDuringUnmount && err != nil { return fmt.Errorf("getRootMappingPath(%s) failed with error: %v", source, err) } unlock := lock(mappingPath) @@ -119,16 +117,17 @@ func (mounter *csiProxyMounter) SMBMount(source, target, fsType string, mountOpt Username: mountOptions[0], Password: sensitiveMountOptions[0], } - klog.V(2).Infof("begin to mount %s on %s", source, normalizedTarget) + klog.V(2).Infof("begin to NewSmbGlobalMapping %s on %s", source, normalizedTarget) if _, err := mounter.SMBClient.NewSmbGlobalMapping(context.Background(), smbMountRequest); err != nil { - return fmt.Errorf("smb mapping failed with error: %v", err) + return fmt.Errorf("NewSmbGlobalMapping(%s, %s) failed with error: %v", source, normalizedTarget, err) } - klog.V(2).Infof("mount %s on %s successfully", source, normalizedTarget) + klog.V(2).Infof("NewSmbGlobalMapping %s on %s successfully", source, normalizedTarget) - if err = incementRemotePathReferencesCount(mappingPath, source); err != nil { - klog.Warningf("incementMappingPathCount(%s, %s) failed with error: %v", mappingPath, source, err) + if mounter.RemoveSMBMappingDuringUnmount { + if err := incementRemotePathReferencesCount(mappingPath, source); err != nil { + return fmt.Errorf("incementMappingPathCount(%s, %s) failed with error: %v", mappingPath, source, err) + } } - return nil } @@ -138,35 +137,32 @@ func (mounter *csiProxyMounter) SMBUnmount(target string) error { if remotePath, err := os.Readlink(target); err != nil { klog.Warningf("SMBUnmount: can't get remote path: %v", err) } else { - if strings.HasSuffix(remotePath, "\\") { - remotePath = strings.TrimSuffix(remotePath, "\\") - } + remotePath = strings.TrimSuffix(remotePath, "\\") mappingPath, err := getRootMappingPath(remotePath) - if err != nil { - klog.Warningf("getRootMappingPath(%s) failed with error: %v", remotePath, err) - } else { - klog.V(4).Infof("SMBUnmount: remote path: %s, mapping path: %s", remotePath, mappingPath) + if mounter.RemoveSMBMappingDuringUnmount && err != nil { + return fmt.Errorf("getRootMappingPath(%s) failed with error: %v", remotePath, err) + } + klog.V(4).Infof("SMBUnmount: remote path: %s, mapping path: %s", remotePath, mappingPath) - unlock := lock(mappingPath) - defer unlock() + unlock := lock(mappingPath) + defer unlock() + if mounter.RemoveSMBMappingDuringUnmount { if err := decrementRemotePathReferencesCount(mappingPath, remotePath); err != nil { - klog.Warningf("decrementMappingPathCount(%s, %d) failed with error: %v", mappingPath, remotePath, err) - } else { - count := getRemotePathReferencesCount(mappingPath) - if count == 0 { - smbUnmountRequest := &smb.RemoveSmbGlobalMappingRequest{ - RemotePath: remotePath, - } - klog.V(2).Infof("begin to unmount %s on %s", remotePath, target) - if _, err := mounter.SMBClient.RemoveSmbGlobalMapping(context.Background(), smbUnmountRequest); err != nil { - return fmt.Errorf("smb unmapping failed with error: %v", err) - } else { - klog.V(2).Infof("unmount %s on %s successfully", remotePath, target) - } - } else { - klog.Infof("SMBUnmount: found %f links to %s", count, mappingPath) + return fmt.Errorf("decrementMappingPathCount(%s, %s) failed with error: %v", mappingPath, remotePath, err) + } + count := getRemotePathReferencesCount(mappingPath) + if count == 0 { + smbUnmountRequest := &smb.RemoveSmbGlobalMappingRequest{ + RemotePath: remotePath, + } + klog.V(2).Infof("begin to RemoveSmbGlobalMapping %s on %s", remotePath, target) + if _, err := mounter.SMBClient.RemoveSmbGlobalMapping(context.Background(), smbUnmountRequest); err != nil { + return fmt.Errorf("RemoveSmbGlobalMapping failed with error: %v", err) } + klog.V(2).Infof("RemoveSmbGlobalMapping %s on %s successfully", remotePath, target) + } else { + klog.Infof("SMBUnmount: found %d links to %s", count, mappingPath) } } } @@ -342,7 +338,7 @@ func (mounter *csiProxyMounter) MountSensitiveWithoutSystemdWithMountFlags(sourc // NewCSIProxyMounter - creates a new CSI Proxy mounter struct which encompassed all the // clients to the CSI proxy - filesystem, disk and volume clients. -func NewCSIProxyMounter() (*csiProxyMounter, error) { +func NewCSIProxyMounter(removeSMBMappingDuringUnmount bool) (*csiProxyMounter, error) { fsClient, err := fsclient.NewClient() if err != nil { return nil, err @@ -353,13 +349,14 @@ func NewCSIProxyMounter() (*csiProxyMounter, error) { } return &csiProxyMounter{ - FsClient: fsClient, - SMBClient: smbClient, + FsClient: fsClient, + SMBClient: smbClient, + RemoveSMBMappingDuringUnmount: removeSMBMappingDuringUnmount, }, nil } -func NewSafeMounter() (*mount.SafeFormatAndMount, error) { - csiProxyMounter, err := NewCSIProxyMounter() +func NewSafeMounter(removeSMBMappingDuringUnmount bool) (*mount.SafeFormatAndMount, error) { + csiProxyMounter, err := NewCSIProxyMounter(removeSMBMappingDuringUnmount) if err == nil { klog.V(2).Infof("using CSIProxyMounterV1, %s", csiProxyMounter.GetAPIVersions()) return &mount.SafeFormatAndMount{ diff --git a/pkg/smb/fake_mounter.go b/pkg/smb/fake_mounter.go index 6dc41c53a11..69a7ef686fd 100644 --- a/pkg/smb/fake_mounter.go +++ b/pkg/smb/fake_mounter.go @@ -65,7 +65,7 @@ func (f *fakeMounter) IsLikelyNotMountPoint(file string) (bool, error) { func NewFakeMounter() (*mount.SafeFormatAndMount, error) { if runtime.GOOS == "windows" { - return mounter.NewSafeMounter() + return mounter.NewSafeMounter(true) } return &mount.SafeFormatAndMount{ Interface: &fakeMounter{}, diff --git a/pkg/smb/nodeserver_test.go b/pkg/smb/nodeserver_test.go index 6dd605923c5..3b5643c204a 100644 --- a/pkg/smb/nodeserver_test.go +++ b/pkg/smb/nodeserver_test.go @@ -153,9 +153,9 @@ func TestNodeStageVolume(t *testing.T) { VolumeCapability: &stdVolCap, VolumeContext: volContext, Secrets: secrets}, - flakyWindowsErrorMessage: fmt.Sprintf("volume(vol_1##) mount \"%s\" on %#v failed "+ - "with smb mapping failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.", - strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource), + flakyWindowsErrorMessage: fmt.Sprintf("rpc error: code = Internal desc = volume(vol_1##) mount \"%s\" on %#v failed "+ + "with NewSmbGlobalMapping(%s, %s) failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.", + strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource, testSource, errorMountSensSource), expectedErr: testutil.TestError{ DefaultError: status.Errorf(codes.Internal, fmt.Sprintf("volume(vol_1##) mount \"%s\" on \"%s\" failed with fake "+ @@ -169,9 +169,9 @@ func TestNodeStageVolume(t *testing.T) { VolumeCapability: &stdVolCap, VolumeContext: volContext, Secrets: secrets}, - flakyWindowsErrorMessage: fmt.Sprintf("volume(vol_1##) mount \"%s\" on %#v failed with "+ - "smb mapping failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.", - strings.Replace(testSource, "\\", "\\\\", -1), sourceTest), + flakyWindowsErrorMessage: fmt.Sprintf("rpc error: code = Internal desc = volume(vol_1##) mount \"%s\" on %#v failed with "+ + "NewSmbGlobalMapping(%s, %s) failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.", + strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource, testSource, errorMountSensSource), expectedErr: testutil.TestError{}, }, { @@ -180,9 +180,9 @@ func TestNodeStageVolume(t *testing.T) { VolumeCapability: &stdVolCap, VolumeContext: volContextWithMetadata, Secrets: secrets}, - flakyWindowsErrorMessage: fmt.Sprintf("volume(vol_1##) mount \"%s\" on %#v failed with "+ - "smb mapping failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.", - strings.Replace(testSource, "\\", "\\\\", -1), sourceTest), + flakyWindowsErrorMessage: fmt.Sprintf("rpc error: code = Internal desc = volume(vol_1##) mount \"%s\" on %#v failed with "+ + "NewSmbGlobalMapping(%s, %s) failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.", + strings.Replace(testSource, "\\", "\\\\", -1), errorMountSensSource, testSource, errorMountSensSource), expectedErr: testutil.TestError{}, }, } diff --git a/pkg/smb/smb.go b/pkg/smb/smb.go index 945c0286d03..48c0f72b638 100644 --- a/pkg/smb/smb.go +++ b/pkg/smb/smb.go @@ -50,7 +50,9 @@ type DriverOptions struct { NodeID string DriverName string EnableGetVolumeStats bool - WorkingMountDir string + // this only applies to Windows node + RemoveSMBMappingDuringUnmount bool + WorkingMountDir string } // Driver implements all interfaces of CSI drivers @@ -62,6 +64,8 @@ type Driver struct { volumeLocks *volumeLocks workingMountDir string enableGetVolumeStats bool + // this only applies to Windows node + removeSMBMappingDuringUnmount bool } // NewDriver Creates a NewCSIDriver object. Assumes vendor version is equal to driver version & @@ -72,6 +76,7 @@ func NewDriver(options *DriverOptions) *Driver { driver.Version = driverVersion driver.NodeID = options.NodeID driver.enableGetVolumeStats = options.EnableGetVolumeStats + driver.removeSMBMappingDuringUnmount = options.RemoveSMBMappingDuringUnmount driver.workingMountDir = options.WorkingMountDir driver.volumeLocks = newVolumeLocks() return &driver @@ -85,7 +90,7 @@ func (d *Driver) Run(endpoint, kubeconfig string, testMode bool) { } klog.V(2).Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta) - d.mounter, err = mounter.NewSafeMounter() + d.mounter, err = mounter.NewSafeMounter(d.removeSMBMappingDuringUnmount) if err != nil { klog.Fatalf("Failed to get safe mounter. Error: %v", err) }