On AttachDetachController node status update, do not retry when node doesn't exist but keep the node entry in cache

pull/6/head
Cheng Xing 2017-08-16 15:35:33 -07:00
parent 5c558ddb18
commit 1234d2f500
3 changed files with 1 additions and 103 deletions

View File

@ -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()

View File

@ -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,

View File

@ -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