From 263a57e20967d4baff65b5e239cfaf1f3d9a193e Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 15 Sep 2017 13:43:43 -0400 Subject: [PATCH] UPSTREAM: 52221: Always populate volume status from node --- .../attachdetach/attach_detach_controller.go | 11 +++---- .../cache/actual_state_of_world.go | 9 ++---- .../cache/actual_state_of_world_test.go | 31 ++++++++++++------- .../reconciler/reconciler_test.go | 8 ++--- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/attach_detach_controller.go b/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/attach_detach_controller.go index 7a97202f78fb..37f6ee05ae12 100644 --- a/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -281,7 +281,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)) } } @@ -450,7 +450,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{}) { @@ -465,7 +465,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" @@ -473,7 +473,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 @@ -483,8 +483,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/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go b/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go index 552530170fbf..3aaf71552520 100644 --- a/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go +++ b/vendor/k8s.io/kubernetes/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 @@ -348,7 +348,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() @@ -360,11 +360,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/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go b/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go index 88ae500d828d..81e5653fd024 100644 --- a/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go +++ b/vendor/k8s.io/kubernetes/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) } + 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 */, false /* force unmount */) + setVolumeMountedErr := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) // Assert if setVolumeMountedErr != nil { t.Fatalf("SetVolumeMountedByNode failed. Expected Actual: <%v>", setVolumeMountedErr) } - attachedVolumes := asw.GetAttachedVolumes() + 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 */) + 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/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go b/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go index ec011918abbd..12da11be39ed 100644 --- a/vendor/k8s.io/kubernetes/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go +++ b/vendor/k8s.io/kubernetes/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)