Repository: kubernetes-sigs/azurefile-csi-driver
Pull request #174 (2020-03-02 04:56:47)

fix: support corrupted mount
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://git.k8s.io/community/contributors/guide#your-first-contribution and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. Please label this pull request according to what type of issue you are addressing, especially if this is a release targeted pull request. For reference on required PR/issue labels, read here:
https://git.k8s.io/community/contributors/devel/release.md#issue-kind-label
3. Ensure you have added or ran the appropriate tests for your PR: https://git.k8s.io/community/contributors/devel/testing.md
4. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
5. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
6. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
-->

**What type of PR is this?**
/kind feature

**What this PR does / why we need it**:
This PR would 

#### 1. handle broken mount in both `NodeStage` and `NodePublish`
 - detect the broken mount path
 - unmount broken mount path
 - remount mount path

#### 2. add taints to run controller on master node

**Which issue(s) this PR fixes**:
<!-- 
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

**Special notes for your reviewer**:


**Release note**:
```
fix: support corrupted mount
```



Patch:
diff --git a/deploy/csi-azurefile-controller.yaml b/deploy/csi-azurefile-controller.yaml
index 2499ff046..1b77493ca 100644
--- a/deploy/csi-azurefile-controller.yaml
+++ b/deploy/csi-azurefile-controller.yaml
@@ -19,6 +19,11 @@ spec:
       nodeSelector:
         beta.kubernetes.io/os: linux
       priorityClassName: system-cluster-critical
+      tolerations:
+        - key: "node-role.kubernetes.io/master"
+          operator: "Equal"
+          value: "true"
+          effect: "NoSchedule"
       containers:
         - name: csi-provisioner
           image: mcr.microsoft.com/oss/kubernetes-csi/csi-provisioner:v1.4.0
diff --git a/docs/csi-dev.md b/docs/csi-dev.md
index fa103ecd9..8072e15c1 100644
--- a/docs/csi-dev.md
+++ b/docs/csi-dev.md
@@ -13,9 +13,9 @@ $ cd $GOPATH/src/sigs.k8s.io/azurefile-csi-driver
 $ make azurefile
 ```
 
- - Run unit test
+ - Run verification test before submitting code
 ```
-$ make unit-test
+$ make verify
 ```
 
 ## How to test CSI driver in local environment
@@ -99,7 +99,7 @@ $  csc controller delete-snapshot
 ```
 # run `docker login` first
 export REGISTRY=<dockerhub-alias>
-make azurefile-container
+make container
 make push-latest
 ```
 
diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go
index b9b575e37..13e1d792a 100644
--- a/pkg/azurefile/azurefile.go
+++ b/pkg/azurefile/azurefile.go
@@ -307,3 +307,9 @@ func (d *Driver) expandVolume(ctx context.Context, volumeID string, capacityByte
 
 	return volumehelper.GiBToBytes(int64(resp.Quota())), nil
 }
+
+func IsCorruptedDir(dir string) bool {
+	_, pathErr := mount.PathExists(dir)
+	fmt.Printf("IsCorruptedDir(%s) returned with error: %v", dir, pathErr)
+	return pathErr != nil && mount.IsCorruptedMnt(pathErr)
+}
diff --git a/pkg/azurefile/nodeserver.go b/pkg/azurefile/nodeserver.go
index e2300cda3..b6b3b3230 100644
--- a/pkg/azurefile/nodeserver.go
+++ b/pkg/azurefile/nodeserver.go
@@ -115,23 +115,9 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 		return nil, status.Error(codes.InvalidArgument, "Volume capability not provided")
 	}
 
-	notMnt, err := d.mounter.IsLikelyNotMountPoint(targetPath)
-	if err != nil && !os.IsNotExist(err) {
-		return nil, err
-	}
-	if !notMnt {
-		// testing original mount point, make sure the mount link is valid
-		if _, err := ioutil.ReadDir(targetPath); err == nil {
-			klog.V(2).Infof("azureFile - already mounted to target %s", targetPath)
-			return &csi.NodeStageVolumeResponse{}, nil
-		}
-		// todo: mount link is invalid, now unmount and remount later (built-in functionality)
-		klog.Warningf("azureFile - ReadDir %s failed with %v, unmount this directory", targetPath, err)
-		if err := d.mounter.Unmount(targetPath); err != nil {
-			klog.Errorf("azureFile - Unmount directory %s failed with %v", targetPath, err)
-			return nil, err
-		}
-		// notMnt = true
+	err := d.ensureMountPoint(targetPath)
+	if err != nil {
+		return nil, status.Errorf(codes.Internal, "Could not mount target %q: %v", targetPath, err)
 	}
 
 	fsType := req.GetVolumeCapability().GetMount().GetFsType()


Test patch:
diff --git a/charts/latest/azurefile-csi-driver/templates/csi-azurefile-controller.yaml b/charts/latest/azurefile-csi-driver/templates/csi-azurefile-controller.yaml
index edbd03089..fa406c188 100644
--- a/charts/latest/azurefile-csi-driver/templates/csi-azurefile-controller.yaml
+++ b/charts/latest/azurefile-csi-driver/templates/csi-azurefile-controller.yaml
@@ -19,6 +19,11 @@ spec:
       nodeSelector:
         beta.kubernetes.io/os: linux
       priorityClassName: system-cluster-critical
+      tolerations:
+        - key: "node-role.kubernetes.io/master"
+          operator: "Equal"
+          value: "true"
+          effect: "NoSchedule"
       containers:
         - name: csi-provisioner
           image: "{{ .Values.image.csiProvisioner.repository }}:{{ .Values.image.csiProvisioner.tag }}"
diff --git a/pkg/azurefile/azurefile_test.go b/pkg/azurefile/azurefile_test.go
index 4b6dd32c2..1b481e79c 100644
--- a/pkg/azurefile/azurefile_test.go
+++ b/pkg/azurefile/azurefile_test.go
@@ -18,6 +18,9 @@ package azurefile
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -329,3 +332,38 @@ func TestGetSnapshot(t *testing.T) {
 		}
 	}
 }
+
+func TestIsCorruptedDir(t *testing.T) {
+	existingMountPath, err := ioutil.TempDir(os.TempDir(), "csi-mount-test")
+	if err != nil {
+		t.Fatalf("failed to create tmp dir: %v", err)
+	}
+	defer os.RemoveAll(existingMountPath)
+
+	curruptedPath := filepath.Join(existingMountPath, "curruptedPath")
+	if err := os.Symlink(existingMountPath, curruptedPath); err != nil {
+		t.Fatalf("failed to create curruptedPath: %v", err)
+	}
+
+	tests := []struct {
+		desc           string
+		dir            string
+		expectedResult bool
+	}{
+		{
+			desc:           "NotExist dir",
+			dir:            "/tmp/NotExist",
+			expectedResult: false,
+		},
+		{
+			desc:           "Existing dir",
+			dir:            existingMountPath,
+			expectedResult: false,
+		},
+	}
+
+	for i, test := range tests {
+		isCorruptedDir := IsCorruptedDir(test.dir)
+		assert.Equal(t, test.expectedResult, isCorruptedDir, "TestCase[%d]: %s", i, test.desc)
+	}
+}
diff --git a/test/utils/azurefile_log.sh b/test/utils/azurefile_log.sh
index 4e156b446..8ebd2676d 100755
--- a/test/utils/azurefile_log.sh
+++ b/test/utils/azurefile_log.sh
@@ -20,7 +20,7 @@ NS=kube-system
 CONTAINER=azurefile
 
 echo "print out all $NS namespace pods status ..."
-kubectl get pods -n${NS}
+kubectl get pods -n${NS} -o wide
 echo "======================================================================================"
 
 echo "print out csi-azurefile-controller pods ..."