fix: the volume is not detached after the pod and PVC objects are deleted

This commit is contained in:
weizhichen 2023-02-28 17:14:00 +00:00
parent 6f68a13696
commit b0eeeacfc0
3 changed files with 11 additions and 82 deletions

View File

@ -595,15 +595,14 @@ func (c *csiAttacher) UnmountDevice(deviceMountPath string) error {
driverName = data[volDataKey.driverName] driverName = data[volDataKey.driverName]
volID = data[volDataKey.volHandle] volID = data[volDataKey.volHandle]
} else { } else {
klog.Error(log("UnmountDevice failed to load volume data file [%s]: %v", dataDir, err)) if errors.Is(err, os.ErrNotExist) {
klog.V(4).Info(log("attacher.UnmountDevice skipped because volume data file [%s] does not exist", dataDir))
return nil
}
// 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 {
klog.Errorf(log("attacher.UnmountDevice failed to get driver and volume name from device mount path: %v", err)) klog.Errorf(log("attacher.UnmountDevice failed to get driver and volume name from device mount path: %v", err))
return err return err
} }
}
if c.csiClient == nil { if c.csiClient == nil {
c.csiClient, err = newCsiDriverClient(csiDriverName(driverName)) c.csiClient, err = newCsiDriverClient(csiDriverName(driverName))
@ -682,36 +681,6 @@ func makeDeviceMountPath(plugin *csiPlugin, spec *volume.Spec) (string, error) {
return filepath.Join(plugin.host.GetPluginDir(plugin.GetPluginName()), driver, volSha, globalMountInGlobalPath), nil return filepath.Join(plugin.host.GetPluginDir(plugin.GetPluginName()), driver, volSha, globalMountInGlobalPath), nil
} }
func getDriverAndVolNameFromDeviceMountPath(k8s kubernetes.Interface, deviceMountPath string) (string, string, error) {
// deviceMountPath structure: /var/lib/kubelet/plugins/kubernetes.io/csi/pv/{pvname}/globalmount
dir := filepath.Dir(deviceMountPath)
if file := filepath.Base(deviceMountPath); file != globalMountInGlobalPath {
return "", "", errors.New(log("getDriverAndVolNameFromDeviceMountPath failed, path did not end in %s", globalMountInGlobalPath))
}
// dir is now /var/lib/kubelet/plugins/kubernetes.io/csi/pv/{pvname}
pvName := filepath.Base(dir)
// Get PV and check for errors
pv, err := k8s.CoreV1().PersistentVolumes().Get(context.TODO(), pvName, metav1.GetOptions{})
if err != nil {
return "", "", err
}
if pv == nil || pv.Spec.CSI == nil {
return "", "", errors.New(log("getDriverAndVolNameFromDeviceMountPath could not find CSI Persistent Volume Source for pv: %s", pvName))
}
// Get VolumeHandle and PluginName from pv
csiSource := pv.Spec.CSI
if csiSource.Driver == "" {
return "", "", errors.New(log("getDriverAndVolNameFromDeviceMountPath failed, driver name empty"))
}
if csiSource.VolumeHandle == "" {
return "", "", errors.New(log("getDriverAndVolNameFromDeviceMountPath failed, VolumeHandle empty"))
}
return csiSource.Driver, csiSource.VolumeHandle, nil
}
func verifyAttachmentStatus(attachment *storage.VolumeAttachment, volumeHandle string) (bool, error) { func verifyAttachmentStatus(attachment *storage.VolumeAttachment, volumeHandle string) (bool, error) {
// when we received a deleted event during attachment, fail fast // when we received a deleted event during attachment, fail fast
if attachment == nil { if attachment == nil {

View File

@ -1596,12 +1596,11 @@ func TestAttacherUnmountDevice(t *testing.T) {
}, },
// PV agnostic path negative test cases // PV agnostic path negative test cases
{ {
testName: "fail: missing json, fail to retrieve driver and volumeID from globalpath", testName: "success: json file doesn't exist, unmount device is skipped",
volID: "project/zone/test-vol1",
deviceMountPath: "plugins/csi/" + generateSha("project/zone/test-vol1") + "/globalmount", deviceMountPath: "plugins/csi/" + generateSha("project/zone/test-vol1") + "/globalmount",
jsonFile: "", jsonFile: "",
stageUnstageSet: true, stageUnstageSet: true,
shouldFail: true, createPV: true,
}, },
{ {
testName: "fail: invalid json, fail to retrieve driver and volumeID from globalpath", testName: "fail: invalid json, fail to retrieve driver and volumeID from globalpath",
@ -1611,45 +1610,6 @@ func TestAttacherUnmountDevice(t *testing.T) {
stageUnstageSet: true, stageUnstageSet: true,
shouldFail: true, shouldFail: true,
}, },
// Old style PV based path positive test cases
{
testName: "success: json file doesn't exist, old style pv based global path -> use PV",
volID: "project/zone/test-vol1",
deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
jsonFile: "",
stageUnstageSet: true,
createPV: true,
},
{
testName: "success: invalid json file, old style pv based global path -> use PV",
volID: "project/zone/test-vol1",
deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
jsonFile: `{"driverName"}}`,
stageUnstageSet: true,
createPV: true,
},
{
testName: "stage_unstage not set, PV based path, unmount device is skipped",
deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
jsonFile: `{"driverName":"test-driver","volumeHandle":"test-vol1"}`,
stageUnstageSet: false,
},
// Old style PV based path negative test cases
{
testName: "fail: json file doesn't exist, old style pv based device path, missing PV",
volID: "project/zone/test-vol1",
deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
jsonFile: "",
stageUnstageSet: true,
shouldFail: true,
},
{
testName: "fail: no json, no PV.volID, old style pv based global path",
volID: "",
deviceMountPath: "plugins/csi/pv/test-pv-name/globalmount",
jsonFile: "",
shouldFail: true,
},
} }
for _, tc := range testCases { for _, tc := range testCases {
@ -1713,7 +1673,7 @@ func TestAttacherUnmountDevice(t *testing.T) {
// Verify call goes through all the way // Verify call goes through all the way
expectedSet := 0 expectedSet := 0
if !tc.stageUnstageSet { if !tc.stageUnstageSet || tc.volID == "" {
expectedSet = 1 expectedSet = 1
} }
staged := cdc.nodeClient.GetNodeStagedVolumes() staged := cdc.nodeClient.GetNodeStagedVolumes()
@ -1722,7 +1682,7 @@ func TestAttacherUnmountDevice(t *testing.T) {
} }
_, ok := staged[tc.volID] _, ok := staged[tc.volID]
if ok && tc.stageUnstageSet { if ok && tc.stageUnstageSet && tc.volID != "" {
t.Errorf("found unexpected staged volume: %s", tc.volID) t.Errorf("found unexpected staged volume: %s", tc.volID)
} else if !ok && !tc.stageUnstageSet { } else if !ok && !tc.stageUnstageSet {
t.Errorf("could not find expected staged volume: %s", tc.volID) t.Errorf("could not find expected staged volume: %s", tc.volID)

View File

@ -79,7 +79,7 @@ func loadVolumeData(dir string, fileName string) (map[string]string, error) {
file, err := os.Open(dataFileName) file, err := os.Open(dataFileName)
if err != nil { if err != nil {
return nil, errors.New(log("failed to open volume data file [%s]: %v", dataFileName, err)) return nil, fmt.Errorf("%s: %w", log("failed to open volume data file [%s]", dataFileName), err)
} }
defer file.Close() defer file.Close()
data := map[string]string{} data := map[string]string{}