Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #50806 upstream release 1.7 #51177

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 0 additions & 17 deletions pkg/controller/volume/attachdetach/cache/actual_state_of_world.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
Expand Up @@ -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,
Expand Down
Expand Up @@ -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
Expand Down