github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-02 00:07:50 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #52221 from gnufied/fix-detach-delay-mount-node

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 52990, 53064, 52686, 52221, 53069). 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>.

Always populate volume status from node

Fixes https://github.com/kubernetes/kubernetes/issues/52036 

As discussed offline with @jingxu97 

/sig storage
This commit is contained in:
Kubernetes Submit Queue 2017-09-26 23:12:30 -07:00 committed by GitHub
commit 1cffa70c0d
4 changed files with 35 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
pkg/controller/volume/attachdetach/attach_detach_controller.go
View File

@ -294,7 +294,7 @@ func (adc *attachDetachController) populateActualStateOfWorld() error {
glog.Errorf("Failed to mark the volume as attached: %v", err) glog.Errorf("Failed to mark the volume as attached: %v", err)
continue continue
} }
adc.processVolumesInUse(nodeName, node.Status.VolumesInUse, true /* forceUnmount */) adc.processVolumesInUse(nodeName, node.Status.VolumesInUse)
adc.addNodeToDswp(node, types.NodeName(node.Name)) adc.addNodeToDswp(node, types.NodeName(node.Name))
} }
} }
@ -463,7 +463,7 @@ func (adc *attachDetachController) nodeUpdate(oldObj, newObj interface{}) {
nodeName := types.NodeName(node.Name) nodeName := types.NodeName(node.Name)
adc.addNodeToDswp(node, nodeName) adc.addNodeToDswp(node, nodeName)
adc.processVolumesInUse(nodeName, node.Status.VolumesInUse, false /* forceUnmount */) adc.processVolumesInUse(nodeName, node.Status.VolumesInUse)
} }
func (adc *attachDetachController) nodeDelete(obj interface{}) { 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) 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" // 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 // corresponding volume in the actual state of the world to indicate that it is
// mounted. // mounted.
func (adc *attachDetachController) processVolumesInUse( 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) glog.V(4).Infof("processVolumesInUse for node %q", nodeName)
for _, attachedVolume := range adc.actualStateOfWorld.GetAttachedVolumesForNode(nodeName) { for _, attachedVolume := range adc.actualStateOfWorld.GetAttachedVolumesForNode(nodeName) {
mounted := false mounted := false
@ -496,8 +496,7 @@ func (adc *attachDetachController) processVolumesInUse(
break break
} }
} }
err := adc.actualStateOfWorld.SetVolumeMountedByNode( err := adc.actualStateOfWorld.SetVolumeMountedByNode(attachedVolume.VolumeName, nodeName, mounted)
attachedVolume.VolumeName, nodeName, mounted, forceUnmount)
if err != nil { if err != nil {
glog.Warningf( glog.Warningf(
"SetVolumeMountedByNode(%q, %q, %q) returned an error: %v", "SetVolumeMountedByNode(%q, %q, %q) returned an error: %v",

9
pkg/controller/volume/attachdetach/cache/actual_state_of_world.go vendored
View File

@ -68,7 +68,7 @@ type ActualStateOfWorld interface {
// returned. // returned.
// If no node with the name nodeName exists in list of attached nodes for // If no node with the name nodeName exists in list of attached nodes for
// the specified volume, an error is returned. // 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 // SetNodeStatusUpdateNeeded sets statusUpdateNeeded for the specified
// node to true indicating the AttachedVolume field in the Node's Status // node to true indicating the AttachedVolume field in the Node's Status
@ -331,7 +331,7 @@ func (asw *actualStateOfWorld) AddVolumeNode(
} }
func (asw *actualStateOfWorld) SetVolumeMountedByNode( 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() asw.Lock()
defer asw.Unlock() defer asw.Unlock()
@ -343,11 +343,6 @@ func (asw *actualStateOfWorld) SetVolumeMountedByNode(
if mounted { if mounted {
// Increment set count // Increment set count
nodeObj.mountedByNodeSetCount = nodeObj.mountedByNodeSetCount + 1 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 nodeObj.mountedByNode = mounted

41
pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go vendored
View File

@ -506,8 +506,8 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) {
} }
// Act // Act
setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */)
setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */)
// Assert // Assert
if setVolumeMountedErr1 != nil { if setVolumeMountedErr1 != nil {
@ -527,7 +527,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) {
// Populates data struct with one volume/node entry. // Populates data struct with one volume/node entry.
// Calls SetVolumeMountedByNode once, setting mounted to false. // 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) { func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) {
// Arrange // Arrange
volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
@ -541,20 +541,27 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) {
t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", addErr) t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", addErr)
} }
// Act
setVolumeMountedErr := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */)
// Assert
if setVolumeMountedErr != nil {
t.Fatalf("SetVolumeMountedByNode failed. Expected <no error> Actual: <%v>", setVolumeMountedErr)
}
attachedVolumes := asw.GetAttachedVolumes() attachedVolumes := asw.GetAttachedVolumes()
if len(attachedVolumes) != 1 { if len(attachedVolumes) != 1 {
t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes))
} }
verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) 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 <no error> 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. // Populates data struct with one volume/node entry.
@ -575,8 +582,8 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotRes
} }
// Act // Act
setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */)
setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */)
generatedVolumeName, addErr = asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) generatedVolumeName, addErr = asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath)
// Assert // Assert
@ -625,8 +632,8 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequest
expectedDetachRequestedTime := asw.GetAttachedVolumes()[0].DetachRequestedTime expectedDetachRequestedTime := asw.GetAttachedVolumes()[0].DetachRequestedTime
// Act // Act
setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */)
setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */)
// Assert // Assert
if setVolumeMountedErr1 != nil { if setVolumeMountedErr1 != nil {
@ -767,8 +774,8 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_UnsetWithInitialSetVolumeMou
if addErr != nil { if addErr != nil {
t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", addErr) t.Fatalf("AddVolumeNode failed. Expected: <no error> Actual: <%v>", addErr)
} }
setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false /* force unmount */) setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */)
setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false /* force unmount */) setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */)
if setVolumeMountedErr1 != nil { if setVolumeMountedErr1 != nil {
t.Fatalf("SetVolumeMountedByNode1 failed. Expected <no error> Actual: <%v>", setVolumeMountedErr1) t.Fatalf("SetVolumeMountedByNode1 failed. Expected <no error> Actual: <%v>", setVolumeMountedErr1)
} }

8
pkg/controller/volume/attachdetach/reconciler/reconciler_test.go
View File

@ -170,8 +170,8 @@ func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithUnmountedVolume(t *te
generatedVolumeName, generatedVolumeName,
nodeName) nodeName)
} }
asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false) asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */)
asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false) asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */)
// Assert // Assert
waitForNewDetacherCallCount(t, 1 /* expectedCallCount */, fakePlugin) waitForNewDetacherCallCount(t, 1 /* expectedCallCount */, fakePlugin)
@ -304,8 +304,8 @@ func Test_Run_Negative_OneDesiredVolumeAttachThenDetachWithUnmountedVolumeUpdate
generatedVolumeName, generatedVolumeName,
nodeName) nodeName)
} }
asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */, false) asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */)
asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */, false) asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */)
// Assert // Assert
verifyNewDetacherCallCount(t, true /* expectZeroNewDetacherCallCount */, fakePlugin) verifyNewDetacherCallCount(t, true /* expectZeroNewDetacherCallCount */, fakePlugin)