Rename volumesNeedDevicePath
To volumesNeedUpdateFromNodeStatus - because both devicePath and uncertain attach-ability needs to be fixed from node status.
This commit is contained in:
		| @@ -104,24 +104,24 @@ func NewReconciler( | |||||||
| 	volumePluginMgr *volumepkg.VolumePluginMgr, | 	volumePluginMgr *volumepkg.VolumePluginMgr, | ||||||
| 	kubeletPodsDir string) Reconciler { | 	kubeletPodsDir string) Reconciler { | ||||||
| 	return &reconciler{ | 	return &reconciler{ | ||||||
| 		kubeClient:                    kubeClient, | 		kubeClient:                      kubeClient, | ||||||
| 		controllerAttachDetachEnabled: controllerAttachDetachEnabled, | 		controllerAttachDetachEnabled:   controllerAttachDetachEnabled, | ||||||
| 		loopSleepDuration:             loopSleepDuration, | 		loopSleepDuration:               loopSleepDuration, | ||||||
| 		waitForAttachTimeout:          waitForAttachTimeout, | 		waitForAttachTimeout:            waitForAttachTimeout, | ||||||
| 		nodeName:                      nodeName, | 		nodeName:                        nodeName, | ||||||
| 		desiredStateOfWorld:           desiredStateOfWorld, | 		desiredStateOfWorld:             desiredStateOfWorld, | ||||||
| 		actualStateOfWorld:            actualStateOfWorld, | 		actualStateOfWorld:              actualStateOfWorld, | ||||||
| 		populatorHasAddedPods:         populatorHasAddedPods, | 		populatorHasAddedPods:           populatorHasAddedPods, | ||||||
| 		operationExecutor:             operationExecutor, | 		operationExecutor:               operationExecutor, | ||||||
| 		mounter:                       mounter, | 		mounter:                         mounter, | ||||||
| 		hostutil:                      hostutil, | 		hostutil:                        hostutil, | ||||||
| 		skippedDuringReconstruction:   map[v1.UniqueVolumeName]*globalVolumeInfo{}, | 		skippedDuringReconstruction:     map[v1.UniqueVolumeName]*globalVolumeInfo{}, | ||||||
| 		volumePluginMgr:               volumePluginMgr, | 		volumePluginMgr:                 volumePluginMgr, | ||||||
| 		kubeletPodsDir:                kubeletPodsDir, | 		kubeletPodsDir:                  kubeletPodsDir, | ||||||
| 		timeOfLastSync:                time.Time{}, | 		timeOfLastSync:                  time.Time{}, | ||||||
| 		volumesFailedReconstruction:   make([]podVolume, 0), | 		volumesFailedReconstruction:     make([]podVolume, 0), | ||||||
| 		volumesNeedDevicePath:         make([]v1.UniqueVolumeName, 0), | 		volumesNeedUpdateFromNodeStatus: make([]v1.UniqueVolumeName, 0), | ||||||
| 		volumesNeedReportedInUse:      make([]v1.UniqueVolumeName, 0), | 		volumesNeedReportedInUse:        make([]v1.UniqueVolumeName, 0), | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
| @@ -141,11 +141,11 @@ type reconciler struct { | |||||||
| 	skippedDuringReconstruction   map[v1.UniqueVolumeName]*globalVolumeInfo | 	skippedDuringReconstruction   map[v1.UniqueVolumeName]*globalVolumeInfo | ||||||
| 	kubeletPodsDir                string | 	kubeletPodsDir                string | ||||||
| 	// lock protects timeOfLastSync for updating and checking | 	// lock protects timeOfLastSync for updating and checking | ||||||
| 	timeOfLastSyncLock          sync.Mutex | 	timeOfLastSyncLock              sync.Mutex | ||||||
| 	timeOfLastSync              time.Time | 	timeOfLastSync                  time.Time | ||||||
| 	volumesFailedReconstruction []podVolume | 	volumesFailedReconstruction     []podVolume | ||||||
| 	volumesNeedDevicePath       []v1.UniqueVolumeName | 	volumesNeedUpdateFromNodeStatus []v1.UniqueVolumeName | ||||||
| 	volumesNeedReportedInUse    []v1.UniqueVolumeName | 	volumesNeedReportedInUse        []v1.UniqueVolumeName | ||||||
| } | } | ||||||
|  |  | ||||||
| func (rc *reconciler) Run(stopCh <-chan struct{}) { | func (rc *reconciler) Run(stopCh <-chan struct{}) { | ||||||
|   | |||||||
| @@ -56,10 +56,10 @@ func (rc *reconciler) reconcileNew() { | |||||||
| 		rc.cleanOrphanVolumes() | 		rc.cleanOrphanVolumes() | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	if len(rc.volumesNeedDevicePath) != 0 { | 	if len(rc.volumesNeedUpdateFromNodeStatus) != 0 { | ||||||
| 		rc.updateReconstructedFromAPIServer() | 		rc.updateReconstructedFromAPIServer() | ||||||
| 	} | 	} | ||||||
| 	if len(rc.volumesNeedDevicePath) == 0 { | 	if len(rc.volumesNeedUpdateFromNodeStatus) == 0 { | ||||||
| 		// ASW is fully populated only after both devicePaths and uncertain volume attach-ability | 		// ASW is fully populated only after both devicePaths and uncertain volume attach-ability | ||||||
| 		// were reconstructed from the API server. | 		// were reconstructed from the API server. | ||||||
| 		// This will start reconciliation of node.status.volumesInUse. | 		// This will start reconciliation of node.status.volumesInUse. | ||||||
|   | |||||||
| @@ -118,14 +118,14 @@ func TestReconcileWithUpdateReconstructedFromAPIServer(t *testing.T) { | |||||||
|  |  | ||||||
| 	assert.False(t, reconciler.StatesHasBeenSynced()) | 	assert.False(t, reconciler.StatesHasBeenSynced()) | ||||||
|  |  | ||||||
| 	reconciler.volumesNeedDevicePath = append(reconciler.volumesNeedDevicePath, volumeName1, volumeName2) | 	reconciler.volumesNeedUpdateFromNodeStatus = append(reconciler.volumesNeedUpdateFromNodeStatus, volumeName1, volumeName2) | ||||||
| 	// Act - run reconcile loop just once. | 	// Act - run reconcile loop just once. | ||||||
| 	// "volumesNeedDevicePath" is not empty, so no unmount will be triggered. | 	// "volumesNeedUpdateFromNodeStatus" is not empty, so no unmount will be triggered. | ||||||
| 	reconciler.reconcileNew() | 	reconciler.reconcileNew() | ||||||
|  |  | ||||||
| 	// Assert | 	// Assert | ||||||
| 	assert.True(t, reconciler.StatesHasBeenSynced()) | 	assert.True(t, reconciler.StatesHasBeenSynced()) | ||||||
| 	assert.Empty(t, reconciler.volumesNeedDevicePath) | 	assert.Empty(t, reconciler.volumesNeedUpdateFromNodeStatus) | ||||||
|  |  | ||||||
| 	attachedVolumes := asw.GetAttachedVolumes() | 	attachedVolumes := asw.GetAttachedVolumes() | ||||||
| 	assert.Equalf(t, len(attachedVolumes), 2, "two volumes in ASW expected") | 	assert.Equalf(t, len(attachedVolumes), 2, "two volumes in ASW expected") | ||||||
|   | |||||||
| @@ -39,7 +39,7 @@ func (rc *reconciler) readyToUnmount() bool { | |||||||
|  |  | ||||||
| 	// Allow unmount only when ASW device paths were corrected from node.status to prevent | 	// Allow unmount only when ASW device paths were corrected from node.status to prevent | ||||||
| 	// calling unmount with a wrong devicePath. | 	// calling unmount with a wrong devicePath. | ||||||
| 	if len(rc.volumesNeedDevicePath) != 0 { | 	if len(rc.volumesNeedUpdateFromNodeStatus) != 0 { | ||||||
| 		return false | 		return false | ||||||
| 	} | 	} | ||||||
| 	return true | 	return true | ||||||
| @@ -97,7 +97,7 @@ func (rc *reconciler) reconstructVolumes() { | |||||||
| 		// Remember to update DSW with this information. | 		// Remember to update DSW with this information. | ||||||
| 		rc.volumesNeedReportedInUse = reconstructedVolumeNames | 		rc.volumesNeedReportedInUse = reconstructedVolumeNames | ||||||
| 		// Remember to update devicePath from node.status.volumesAttached | 		// Remember to update devicePath from node.status.volumesAttached | ||||||
| 		rc.volumesNeedDevicePath = reconstructedVolumeNames | 		rc.volumesNeedUpdateFromNodeStatus = reconstructedVolumeNames | ||||||
| 	} | 	} | ||||||
| 	klog.V(2).InfoS("Volume reconstruction finished") | 	klog.V(2).InfoS("Volume reconstruction finished") | ||||||
| } | } | ||||||
| @@ -183,18 +183,18 @@ func (rc *reconciler) updateReconstructedFromAPIServer() { | |||||||
| 		// Skip reconstructing devicePath from node objects if kubelet is in standalone mode. | 		// Skip reconstructing devicePath from node objects if kubelet is in standalone mode. | ||||||
| 		// Such kubelet is not expected to mount any attachable volume or Secrets / ConfigMap. | 		// Such kubelet is not expected to mount any attachable volume or Secrets / ConfigMap. | ||||||
| 		klog.V(2).InfoS("Skipped reconstruction of DevicePaths from node.status in standalone mode") | 		klog.V(2).InfoS("Skipped reconstruction of DevicePaths from node.status in standalone mode") | ||||||
| 		rc.volumesNeedDevicePath = nil | 		rc.volumesNeedUpdateFromNodeStatus = nil | ||||||
| 		return | 		return | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	node, fetchErr := rc.kubeClient.CoreV1().Nodes().Get(context.TODO(), string(rc.nodeName), metav1.GetOptions{}) | 	node, fetchErr := rc.kubeClient.CoreV1().Nodes().Get(context.TODO(), string(rc.nodeName), metav1.GetOptions{}) | ||||||
| 	if fetchErr != nil { | 	if fetchErr != nil { | ||||||
| 		// This may repeat few times per second until kubelet is able to read its own status for the first time. | 		// This may repeat few times per second until kubelet is able to read its own status for the first time. | ||||||
| 		klog.V(2).ErrorS(fetchErr, "Failed to get Node status to reconstruct device paths") | 		klog.V(4).ErrorS(fetchErr, "Failed to get Node status to reconstruct device paths") | ||||||
| 		return | 		return | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	for _, volumeID := range rc.volumesNeedDevicePath { | 	for _, volumeID := range rc.volumesNeedUpdateFromNodeStatus { | ||||||
| 		attachable := false | 		attachable := false | ||||||
| 		for _, attachedVolume := range node.Status.VolumesAttached { | 		for _, attachedVolume := range node.Status.VolumesAttached { | ||||||
| 			if volumeID != attachedVolume.Name { | 			if volumeID != attachedVolume.Name { | ||||||
| @@ -208,5 +208,5 @@ func (rc *reconciler) updateReconstructedFromAPIServer() { | |||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	klog.V(2).InfoS("DevicePaths of reconstructed volumes updated") | 	klog.V(2).InfoS("DevicePaths of reconstructed volumes updated") | ||||||
| 	rc.volumesNeedDevicePath = nil | 	rc.volumesNeedUpdateFromNodeStatus = nil | ||||||
| } | } | ||||||
|   | |||||||
| @@ -117,8 +117,8 @@ func TestReconstructVolumes(t *testing.T) { | |||||||
| 			for i := range tc.expectedVolumesNeedDevicePath { | 			for i := range tc.expectedVolumesNeedDevicePath { | ||||||
| 				expectedVolumes[i] = v1.UniqueVolumeName(tc.expectedVolumesNeedDevicePath[i]) | 				expectedVolumes[i] = v1.UniqueVolumeName(tc.expectedVolumesNeedDevicePath[i]) | ||||||
| 			} | 			} | ||||||
| 			if !reflect.DeepEqual(expectedVolumes, rcInstance.volumesNeedDevicePath) { | 			if !reflect.DeepEqual(expectedVolumes, rcInstance.volumesNeedUpdateFromNodeStatus) { | ||||||
| 				t.Errorf("Expected expectedVolumesNeedDevicePath:\n%v\n got:\n%v", expectedVolumes, rcInstance.volumesNeedDevicePath) | 				t.Errorf("Expected expectedVolumesNeedDevicePath:\n%v\n got:\n%v", expectedVolumes, rcInstance.volumesNeedUpdateFromNodeStatus) | ||||||
| 			} | 			} | ||||||
|  |  | ||||||
| 			expectedVolumes = make([]v1.UniqueVolumeName, len(tc.expectedVolumesNeedReportedInUse)) | 			expectedVolumes = make([]v1.UniqueVolumeName, len(tc.expectedVolumesNeedReportedInUse)) | ||||||
| @@ -333,7 +333,7 @@ func TestReconstructVolumesMount(t *testing.T) { | |||||||
| 				return true | 				return true | ||||||
| 			} | 			} | ||||||
| 			// Mark devices paths as reconciled to allow unmounting of volumes. | 			// Mark devices paths as reconciled to allow unmounting of volumes. | ||||||
| 			rcInstance.volumesNeedDevicePath = nil | 			rcInstance.volumesNeedUpdateFromNodeStatus = nil | ||||||
|  |  | ||||||
| 			// Act 2 - reconcile once | 			// Act 2 - reconcile once | ||||||
| 			rcInstance.reconcileNew() | 			rcInstance.reconcileNew() | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Jan Safranek
					Jan Safranek