From c1248302786025cbc7ff5d301f4291087a9887e8 Mon Sep 17 00:00:00 2001
From: Jing Xu <jinxu@google.com>
Date: Thu, 10 Nov 2016 16:23:55 -0800
Subject: [PATCH] fix issue in reconstruct volume data when kubelet restarts

During state reconstruction when kubelet restarts, outerVolueSpecName
cannot be recovered by scanning the disk directories. But this
information is used by volume manager to check whether pod's volume is
mounted or not. There are two possible cases:
1. pod is not deleted during kubelet restarts so that desired state
should have the information. reconciler.updateState() will use this
inforamtion to update.
2. pod is deleted during this period, reconciler has to use
InnerVolumeSpecName, but it should be ok since this information will not
be used for volume cleanup (umount)
---
 .../cache/actual_state_of_world.go            |  1 -
 .../volumemanager/reconciler/reconciler.go    | 35 ++++++++++++++-----
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go
index 6bd382a1c42..b29cfec8071 100644
--- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go
+++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go
@@ -371,7 +371,6 @@ func (asw *actualStateOfWorld) addVolume(
 	} else {
 		// If volume object already exists, update the fields such as device path
 		volumeObj.devicePath = devicePath
-		volumeObj.spec = volumeSpec
 		glog.V(2).Infof("Volume %q is already added to attachedVolume list, update device path %q",
 			volumeName,
 			devicePath)
diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler.go b/pkg/kubelet/volumemanager/reconciler/reconciler.go
index 949e685309b..3db72963756 100644
--- a/pkg/kubelet/volumemanager/reconciler/reconciler.go
+++ b/pkg/kubelet/volumemanager/reconciler/reconciler.go
@@ -474,6 +474,7 @@ func (rc *reconciler) syncStates(podsDir string) {
 		volumesNeedUpdate[reconstructedVolume.volumeName] = reconstructedVolume
 
 	}
+
 	if len(volumesNeedUpdate) > 0 {
 		if err = rc.updateStates(volumesNeedUpdate); err != nil {
 			glog.Errorf("Error occurred during reconstruct volume from disk: %v", err)
@@ -492,10 +493,6 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
 	if err != nil {
 		return nil, err
 	}
-	volumeName, err := plugin.GetVolumeName(volumeSpec)
-	if err != nil {
-		return nil, err
-	}
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{
 			UID: types.UID(volume.podName),
@@ -505,6 +502,11 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
 	if err != nil {
 		return nil, err
 	}
+
+	volumeName, err := plugin.GetVolumeName(volumeSpec)
+	if err != nil {
+		return nil, err
+	}
 	var uniqueVolumeName api.UniqueVolumeName
 	if attachablePlugin != nil {
 		uniqueVolumeName = volumehelper.GetUniqueVolumeName(volume.pluginName, volumeName)
@@ -527,10 +529,14 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
 	}
 
 	reconstructedVolume := &reconstructedVolume{
-		volumeName:          uniqueVolumeName,
-		podName:             volume.podName,
-		volumeSpec:          volumeSpec,
-		outerVolumeSpecName: volumeName, /* volumeName is InnerVolumeSpecName. But this information will not be used for cleanup */
+		volumeName: uniqueVolumeName,
+		podName:    volume.podName,
+		volumeSpec: volumeSpec,
+		// volume.volumeSpecName is actually InnerVolumeSpecName. But this information will likely to be updated in updateStates()
+		// by checking the desired state volumeToMount list and getting the real OuterVolumeSpecName.
+		// In case the pod is deleted during this period and desired state does not have this information, it will not be used
+		// for volume cleanup.
+		outerVolumeSpecName: volume.volumeSpecName,
 		pod:                 pod,
 		pluginIsAttachable:  attachablePlugin != nil,
 		volumeGidValue:      "",
@@ -550,11 +556,22 @@ func (rc *reconciler) updateStates(volumesNeedUpdate map[api.UniqueVolumeName]*r
 			if volume, exists := volumesNeedUpdate[attachedVolume.Name]; exists {
 				volume.devicePath = attachedVolume.DevicePath
 				volumesNeedUpdate[attachedVolume.Name] = volume
-				glog.V(4).Infof("Get devicePath from node status for volume (%q): %q", attachedVolume.Name, volume.devicePath)
+				glog.V(4).Infof("Update devicePath from node status for volume (%q): %q", attachedVolume.Name, volume.devicePath)
 			}
 		}
 	}
 
+	// Get the list of volumes from desired state and update OuterVolumeSpecName if the information is avaiable
+	volumesToMount := rc.desiredStateOfWorld.GetVolumesToMount()
+	for _, volumeToMount := range volumesToMount {
+		if volume, exists := volumesNeedUpdate[volumeToMount.VolumeName]; exists {
+			volume.outerVolumeSpecName = volumeToMount.OuterVolumeSpecName
+			volumesNeedUpdate[volumeToMount.VolumeName] = volume
+			glog.V(4).Infof("Update OuterVolumeSpecName from desired state for volume (%q): %q",
+				volumeToMount.VolumeName, volume.outerVolumeSpecName)
+		}
+	}
+
 	for _, volume := range volumesNeedUpdate {
 		err := rc.actualStateOfWorld.MarkVolumeAsAttached(
 			volume.volumeName, volume.volumeSpec, "" /* nodeName */, volume.devicePath)