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 5e96b4c433b..4ee4e26c9b4 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go +++ b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go @@ -125,10 +125,6 @@ type ActualStateOfWorld interface { // GetNodesToUpdateStatusFor returns the map of nodeNames to nodeToUpdateStatusFor GetNodesToUpdateStatusFor() map[types.NodeName]nodeToUpdateStatusFor - - // Removes the given node from the record of attach updates. The node's entire - // volumesToReportAsAttached list is removed. - RemoveNodeFromAttachUpdates(nodeName types.NodeName) error } // AttachedVolume represents a volume that is attached to a node. @@ -264,19 +260,6 @@ func (asw *actualStateOfWorld) AddVolumeToReportAsAttached( asw.addVolumeToReportAsAttached(volumeName, nodeName) } -func (asw *actualStateOfWorld) RemoveNodeFromAttachUpdates(nodeName types.NodeName) error { - asw.Lock() - defer asw.Unlock() - - _, nodeToUpdateExists := asw.nodesToUpdateStatusFor[nodeName] - if nodeToUpdateExists { - delete(asw.nodesToUpdateStatusFor, nodeName) - return nil - } - return fmt.Errorf("node %q does not exist in volumesToReportAsAttached list", - nodeName) -} - func (asw *actualStateOfWorld) AddVolumeNode( uniqueName v1.UniqueVolumeName, volumeSpec *volume.Spec, nodeName types.NodeName, devicePath string) (v1.UniqueVolumeName, error) { asw.Lock() 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 dcc9fd837b9..22a4e3df728 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 @@ -1165,89 +1165,6 @@ func Test_updateNodeStatusUpdateNeededError(t *testing.T) { } } -// Test_RemoveNodeFromAttachUpdates_Positive expects an entire node entry to be removed -// from nodesToUpdateStatusFor -func Test_RemoveNodeFromAttachUpdates_Positive(t *testing.T) { - // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := &actualStateOfWorld{ - attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume), - nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor), - volumePluginMgr: volumePluginMgr, - } - nodeName := types.NodeName("node-1") - nodeToUpdate := nodeToUpdateStatusFor{ - nodeName: nodeName, - statusUpdateNeeded: true, - volumesToReportAsAttached: make(map[v1.UniqueVolumeName]v1.UniqueVolumeName), - } - asw.nodesToUpdateStatusFor[nodeName] = nodeToUpdate - - // Act - err := asw.RemoveNodeFromAttachUpdates(nodeName) - - // Assert - if err != nil { - t.Fatalf("RemoveNodeFromAttachUpdates should not return error, but got: %v", err) - } - if len(asw.nodesToUpdateStatusFor) > 0 { - t.Fatal("nodesToUpdateStatusFor should be empty as its only entry has been deleted.") - } -} - -// Test_RemoveNodeFromAttachUpdates_Negative_NodeDoesNotExist expects an error to be thrown -// when nodeName is not in nodesToUpdateStatusFor. -func Test_RemoveNodeFromAttachUpdates_Negative_NodeDoesNotExist(t *testing.T) { - // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := &actualStateOfWorld{ - attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume), - nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor), - volumePluginMgr: volumePluginMgr, - } - nodeName := types.NodeName("node-1") - nodeToUpdate := nodeToUpdateStatusFor{ - nodeName: nodeName, - statusUpdateNeeded: true, - volumesToReportAsAttached: make(map[v1.UniqueVolumeName]v1.UniqueVolumeName), - } - asw.nodesToUpdateStatusFor[nodeName] = nodeToUpdate - - // Act - err := asw.RemoveNodeFromAttachUpdates("node-2") - - // Assert - if err == nil { - t.Fatal("RemoveNodeFromAttachUpdates should return an error as the nodeName doesn't exist.") - } - if len(asw.nodesToUpdateStatusFor) != 1 { - t.Fatal("The length of nodesToUpdateStatusFor should not change because no operation was performed.") - } -} - -// Test_RemoveNodeFromAttachUpdates_Negative_Empty expects an error to be thrown -// when a nodesToUpdateStatusFor is empty. -func Test_RemoveNodeFromAttachUpdates_Negative_Empty(t *testing.T) { - // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := &actualStateOfWorld{ - attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume), - nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor), - volumePluginMgr: volumePluginMgr, - } - - // Act - err := asw.RemoveNodeFromAttachUpdates("node-1") - - // Assert - if err == nil { - t.Fatal("RemoveNodeFromAttachUpdates should return an error as nodeToUpdateStatusFor is empty.") - } - if len(asw.nodesToUpdateStatusFor) != 0 { - t.Fatal("The length of nodesToUpdateStatusFor should be 0.") - } -} - func verifyAttachedVolume( t *testing.T, attachedVolumes []AttachedVolume, diff --git a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go index acfc2b97bbc..2ebb1706e99 100644 --- a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go +++ b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go @@ -68,13 +68,11 @@ func (nsu *nodeStatusUpdater) UpdateNodeStatuses() error { nodeObj, err := nsu.nodeLister.Get(string(nodeName)) if errors.IsNotFound(err) { // If node does not exist, its status cannot be updated. - // Remove the node entry from the collection of attach updates, preventing the - // status updater from unnecessarily updating the node. + // Do nothing so that there is no retry until node is created. glog.V(2).Infof( "Could not update node status. Failed to find node %q in NodeInformer cache. Error: '%v'", nodeName, err) - nsu.actualStateOfWorld.RemoveNodeFromAttachUpdates(nodeName) continue } else if err != nil { // For all other errors, log error and reset flag statusUpdateNeeded