diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index a608fbf94d7..e7286c7c837 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -294,7 +294,7 @@ func (adc *attachDetachController) populateActualStateOfWorld() error { glog.Errorf("Failed to mark the volume as attached: %v", err) continue } - adc.processVolumesInUse(nodeName, node.Status.VolumesInUse, true /* forceUnmount */) + adc.processVolumesInUse(nodeName, node.Status.VolumesInUse) adc.addNodeToDswp(node, types.NodeName(node.Name)) } } @@ -463,7 +463,7 @@ func (adc *attachDetachController) nodeUpdate(oldObj, newObj interface{}) { nodeName := types.NodeName(node.Name) adc.addNodeToDswp(node, nodeName) - adc.processVolumesInUse(nodeName, node.Status.VolumesInUse, false /* forceUnmount */) + adc.processVolumesInUse(nodeName, node.Status.VolumesInUse) } func (adc *attachDetachController) nodeDelete(obj interface{}) { @@ -478,7 +478,7 @@ func (adc *attachDetachController) nodeDelete(obj interface{}) { glog.Infof("error removing node %q from desired-state-of-world: %v", nodeName, err) } - adc.processVolumesInUse(nodeName, node.Status.VolumesInUse, false /* forceUnmount */) + adc.processVolumesInUse(nodeName, node.Status.VolumesInUse) } // processVolumesInUse processes the list of volumes marked as "in-use" @@ -486,7 +486,7 @@ func (adc *attachDetachController) nodeDelete(obj interface{}) { // corresponding volume in the actual state of the world to indicate that it is // mounted. func (adc *attachDetachController) processVolumesInUse( - nodeName types.NodeName, volumesInUse []v1.UniqueVolumeName, forceUnmount bool) { + nodeName types.NodeName, volumesInUse []v1.UniqueVolumeName) { glog.V(4).Infof("processVolumesInUse for node %q", nodeName) for _, attachedVolume := range adc.actualStateOfWorld.GetAttachedVolumesForNode(nodeName) { mounted := false @@ -496,8 +496,7 @@ func (adc *attachDetachController) processVolumesInUse( break } } - err := adc.actualStateOfWorld.SetVolumeMountedByNode( - attachedVolume.VolumeName, nodeName, mounted, forceUnmount) + err := adc.actualStateOfWorld.SetVolumeMountedByNode(attachedVolume.VolumeName, nodeName, mounted) if err != nil { glog.Warningf( "SetVolumeMountedByNode(%q, %q, %q) returned an error: %v", diff --git a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go index 4ee4e26c9b4..20dfa5a1471 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go +++ b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go @@ -68,7 +68,7 @@ type ActualStateOfWorld interface { // returned. // If no node with the name nodeName exists in list of attached nodes for // the specified volume, an error is returned. - SetVolumeMountedByNode(volumeName v1.UniqueVolumeName, nodeName types.NodeName, mounted bool, forceUnmount bool) error + SetVolumeMountedByNode(volumeName v1.UniqueVolumeName, nodeName types.NodeName, mounted bool) error // SetNodeStatusUpdateNeeded sets statusUpdateNeeded for the specified // node to true indicating the AttachedVolume field in the Node's Status @@ -331,7 +331,7 @@ func (asw *actualStateOfWorld) AddVolumeNode( } func (asw *actualStateOfWorld) SetVolumeMountedByNode( - volumeName v1.UniqueVolumeName, nodeName types.NodeName, mounted bool, forceUnmount bool) error { + volumeName v1.UniqueVolumeName, nodeName types.NodeName, mounted bool) error { asw.Lock() defer asw.Unlock() @@ -343,11 +343,6 @@ func (asw *actualStateOfWorld) SetVolumeMountedByNode( if mounted { // Increment set count nodeObj.mountedByNodeSetCount = nodeObj.mountedByNodeSetCount + 1 - } else { - // Do not allow value to be reset unless it has been set at least once - if nodeObj.mountedByNodeSetCount == 0 && !forceUnmount { - return nil - } } nodeObj.mountedByNode = mounted diff --git a/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go b/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go index 22a4e3df728..14a531ba103 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go +++ b/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go @@ -506,8 +506,8 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) { } // Act - setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) - setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) + setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) + setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) // Assert if setVolumeMountedErr1 != nil { @@ -527,7 +527,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) { // Populates data struct with one volume/node entry. // Calls SetVolumeMountedByNode once, setting mounted to false. -// Verifies mountedByNode is still true (since there was no SetVolumeMountedByNode to true call first) +// Verifies mountedByNode is false because value is overwritten func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) @@ -541,20 +541,27 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } - // Act - setVolumeMountedErr := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) - - // Assert - if setVolumeMountedErr != nil { - t.Fatalf("SetVolumeMountedByNode failed. Expected Actual: <%v>", setVolumeMountedErr) - } - attachedVolumes := asw.GetAttachedVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + + // Act + setVolumeMountedErr := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) + + // Assert + if setVolumeMountedErr != nil { + t.Fatalf("SetVolumeMountedByNode failed. Expected Actual: <%v>", setVolumeMountedErr) + } + + attachedVolumes = asw.GetAttachedVolumes() + if len(attachedVolumes) != 1 { + t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) + } + + verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, false /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) } // Populates data struct with one volume/node entry. @@ -575,8 +582,8 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotRes } // Act - setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) - setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) + setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) + setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) generatedVolumeName, addErr = asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) // Assert @@ -625,8 +632,8 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequest expectedDetachRequestedTime := asw.GetAttachedVolumes()[0].DetachRequestedTime // Act - setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) - setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) + setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) + setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) // Assert if setVolumeMountedErr1 != nil { @@ -767,8 +774,8 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_UnsetWithInitialSetVolumeMou if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } - setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) - setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) + setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) + setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) if setVolumeMountedErr1 != nil { t.Fatalf("SetVolumeMountedByNode1 failed. Expected Actual: <%v>", setVolumeMountedErr1) } diff --git a/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go b/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go index ac3cf60cf66..7792174b2fb 100644 --- a/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go +++ b/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go @@ -170,8 +170,8 @@ func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithUnmountedVolume(t *te generatedVolumeName, nodeName) } - asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false) - asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false) + asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) + asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) // Assert waitForNewDetacherCallCount(t, 1 /* expectedCallCount */, fakePlugin) @@ -304,8 +304,8 @@ func Test_Run_Negative_OneDesiredVolumeAttachThenDetachWithUnmountedVolumeUpdate generatedVolumeName, nodeName) } - asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false) - asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false) + asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) + asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) // Assert verifyNewDetacherCallCount(t, true /* expectZeroNewDetacherCallCount */, fakePlugin)