Merge pull request #64882 from jsafrane/csi-mount-json

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix UnmountDevice with deleted pod.

When a pod is deleted, kubelet can't read VolumeAttachment objects. It should cache all information in a json file.

Fixes #63827

~~Work in progress: missing (unit?) tests~~

**Release note**:

```release-note
NONE
```

@saad-ali @vladimirvivien @sbezverk 
/sig storage

pkg/volume/csi/csi_attacher.go

@@ -269,6 +269,25 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo
 		return err
 	}
 
+	dataDir := filepath.Dir(deviceMountPath)
+	if err := os.MkdirAll(dataDir, 0750); err != nil {
+		glog.Error(log("attacher.MountDevice failed to create dir %#v:  %v", dataDir, err))
+		return err
+	}
+
+	data := map[string]string{
+		volDataKey.volHandle:  csiSource.VolumeHandle,
+		volDataKey.driverName: csiSource.Driver,
+	}
+	if err := saveVolumeData(dataDir, volDataFileName, data); err != nil {
+		glog.Error(log("failed to save volume info data: %v", err))
+		if err := os.RemoveAll(dataDir); err != nil {
+			glog.Error(log("failed to remove dir after error [%s]: %v", dataDir, err))
+			return err
+		}
+		return err
+	}
+
 	if c.csiClient == nil {
 		c.csiClient = newCsiDriverClient(csiSource.Driver)
 	}
@@ -461,10 +480,21 @@ func (c *csiAttacher) UnmountDevice(deviceMountPath string) error {
 	glog.V(4).Info(log("attacher.UnmountDevice(%s)", deviceMountPath))
 
 	// Setup
-	driverName, volID, err := getDriverAndVolNameFromDeviceMountPath(c.k8s, deviceMountPath)
+	var driverName, volID string
-	if err != nil {
+	dataDir := filepath.Dir(deviceMountPath)
-		glog.Errorf(log("attacher.UnmountDevice failed to get driver and volume name from device mount path: %v", err))
+	data, err := loadVolumeData(dataDir, volDataFileName)
-		return err
+	if err == nil {
+		driverName = data[volDataKey.driverName]
+		volID = data[volDataKey.volHandle]
+	} else {
+		glog.Error(log("UnmountDevice failed to load volume data file [%s]: %v", dataDir, err))
+
+		// The volume might have been mounted by old CSI volume plugin. Fall back to the old behavior: read PV from API server
+		driverName, volID, err = getDriverAndVolNameFromDeviceMountPath(c.k8s, deviceMountPath)
+		if err != nil {
+			glog.Errorf(log("attacher.UnmountDevice failed to get driver and volume name from device mount path: %v", err))
+			return err
+		}
 	}
 
 	if c.csiClient == nil {
@@ -482,6 +512,11 @@ func (c *csiAttacher) UnmountDevice(deviceMountPath string) error {
 	}
 	if !stageUnstageSet {
 		glog.Infof(log("attacher.UnmountDevice STAGE_UNSTAGE_VOLUME capability not set. Skipping UnmountDevice..."))
+		// Just	delete the global directory + json file
+		if err := removeMountDir(c.plugin, deviceMountPath); err != nil {
+			return fmt.Errorf("failed to clean up gloubal mount %s: %s", dataDir, err)
+		}
+
 		return nil
 	}
 
@@ -495,6 +530,11 @@ func (c *csiAttacher) UnmountDevice(deviceMountPath string) error {
 		return err
 	}
 
+	// Delete the global directory + json file
+	if err := removeMountDir(c.plugin, deviceMountPath); err != nil {
+		return fmt.Errorf("failed to clean up gloubal mount %s: %s", dataDir, err)
+	}
+
 	glog.V(4).Infof(log("attacher.UnmountDevice successfully requested NodeStageVolume [%s]", deviceMountPath))
 	return nil
 }

pkg/volume/csi/csi_attacher_test.go

@@ -18,6 +18,7 @@ package csi
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -604,49 +605,52 @@ func TestAttacherUnmountDevice(t *testing.T) {
 		testName        string
 		volID           string
 		deviceMountPath string
+		jsonFile        string
+		createPV        bool
 		stageUnstageSet bool
 		shouldFail      bool
 	}{
 		{
-			testName:        "normal",
+			testName:        "normal, json file exists",
 			volID:           "project/zone/test-vol1",
-			deviceMountPath: "/tmp/csi-test049507108/plugins/csi/pv/test-pv-name/globalmount",
+			deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
+			jsonFile:        `{"driverName": "csi", "volumeHandle":"project/zone/test-vol1"}`,
+			createPV:        false,
 			stageUnstageSet: true,
 		},
 		{
-			testName:        "no volID",
+			testName:        "normal, json file doesn't exist -> use PV",
+			volID:           "project/zone/test-vol1",
+			deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
+			jsonFile:        "",
+			createPV:        true,
+			stageUnstageSet: true,
+		},
+		{
+			testName:        "invalid json ->  use PV",
+			volID:           "project/zone/test-vol1",
+			deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
+			jsonFile:        `{"driverName"}}`,
+			createPV:        true,
+			stageUnstageSet: true,
+		},
+		{
+			testName:        "no json, no PV.volID",
 			volID:           "",
-			deviceMountPath: "/tmp/csi-test049507108/plugins/csi/pv/test-pv-name/globalmount",
+			deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
-			stageUnstageSet: true,
+			jsonFile:        "",
+			createPV:        true,
 			shouldFail:      true,
 		},
 		{
-			testName:        "no device mount path",
+			testName:        "no json, no PV",
 			volID:           "project/zone/test-vol1",
-			deviceMountPath: "",
+			deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
+			jsonFile:        "",
+			createPV:        false,
 			stageUnstageSet: true,
 			shouldFail:      true,
 		},
-		{
-			testName:        "missing part of device mount path",
-			volID:           "project/zone/test-vol1",
-			deviceMountPath: "/tmp/csi-test049507108/plugins/csi/pv/test-pv-name/globalmount",
-			stageUnstageSet: true,
-			shouldFail:      true,
-		},
-		{
-			testName:        "test volume name mismatch",
-			volID:           "project/zone/test-vol1",
-			deviceMountPath: "/tmp/csi-test049507108/plugins/csi/pv/test-pv-name/globalmount",
-			stageUnstageSet: true,
-			shouldFail:      true,
-		},
-		{
-			testName:        "stage_unstage not set",
-			volID:           "project/zone/test-vol1",
-			deviceMountPath: "/tmp/csi-test049507108/plugins/csi/pv/test-pv-name/globalmount",
-			stageUnstageSet: false,
-		},
 		{
 			testName:        "stage_unstage not set no vars should not fail",
 			stageUnstageSet: false,
@@ -666,29 +670,45 @@ func TestAttacherUnmountDevice(t *testing.T) {
 		csiAttacher := attacher.(*csiAttacher)
 		csiAttacher.csiClient = setupClient(t, tc.stageUnstageSet)
 
+		if tc.deviceMountPath != "" {
+			tc.deviceMountPath = filepath.Join(tmpDir, tc.deviceMountPath)
+		}
+
 		// Add the volume to NodeStagedVolumes
 		cdc := csiAttacher.csiClient.(*fakeCsiDriverClient)
 		cdc.nodeClient.AddNodeStagedVolume(tc.volID, tc.deviceMountPath)
 
-		// Make the PV for this object
+		// Make JSON for this object
+		if tc.deviceMountPath != "" {
+			if err := os.MkdirAll(tc.deviceMountPath, 0755); err != nil {
+				t.Fatalf("error creating directory %s: %s", tc.deviceMountPath, err)
+			}
+		}
 		dir := filepath.Dir(tc.deviceMountPath)
-		// dir is now /var/lib/kubelet/plugins/kubernetes.io/csi/pv/{pvname}
+		if tc.jsonFile != "" {
-		pvName := filepath.Base(dir)
+			dataPath := filepath.Join(dir, volDataFileName)
-		pv := makeTestPV(pvName, 5, "csi", tc.volID)
+			if err := ioutil.WriteFile(dataPath, []byte(tc.jsonFile), 0644); err != nil {
-		_, err := csiAttacher.k8s.CoreV1().PersistentVolumes().Create(pv)
+				t.Fatalf("error creating %s: %s", dataPath, err)
-		if err != nil && !tc.shouldFail {
+			}
-			t.Fatalf("Failed to create PV: %v", err)
+		}
+		if tc.createPV {
+			// Make the PV for this object
+			pvName := filepath.Base(dir)
+			pv := makeTestPV(pvName, 5, "csi", tc.volID)
+			_, err := csiAttacher.k8s.CoreV1().PersistentVolumes().Create(pv)
+			if err != nil && !tc.shouldFail {
+				t.Fatalf("Failed to create PV: %v", err)
+			}
 		}
 
 		// Run
-		err = csiAttacher.UnmountDevice(tc.deviceMountPath)
+		err := csiAttacher.UnmountDevice(tc.deviceMountPath)
-
 		// Verify
 		if err != nil {
 			if !tc.shouldFail {
 				t.Errorf("test should not fail, but error occurred: %v", err)
 			}
-			return
+			continue
 		}
 		if err == nil && tc.shouldFail {
 			t.Errorf("test should fail, but no error occurred")
@@ -711,6 +731,18 @@ func TestAttacherUnmountDevice(t *testing.T) {
 			t.Errorf("could not find expected staged volume: %s", tc.volID)
 		}
 
+		if tc.jsonFile != "" && !tc.shouldFail {
+			dataPath := filepath.Join(dir, volDataFileName)
+			if _, err := os.Stat(dataPath); !os.IsNotExist(err) {
+				if err != nil {
+					t.Errorf("error checking file %s: %s", dataPath, err)
+				} else {
+					t.Errorf("json file %s should not exists, but it does", dataPath)
+				}
+			} else {
+				t.Logf("json file %s was correctly removed", dataPath)
+			}
+		}
 	}
 }