Merge pull request #37727 from rkouj/bug-fix-upgrade-test

Automatic merge from submit-queue

SetNodeUpdateStatusNeeded whenever nodeAdd event is received

**What this PR does / why we need it**:
Bug fix and SetNodeStatusUpdateNeeded for a node whenever its api object is added. This is to ensure that we don't lose the attached list of volumes in the node when its api object is deleted and recreated.

fixes https://github.com/kubernetes/kubernetes/issues/37586
         https://github.com/kubernetes/kubernetes/issues/37585


**Special notes for your reviewer**:


<!--  Steps to write your release note:
1. Use the release-note-* labels to set the release note state (if you have access) 
2. Enter your extended release note in the below block; leaving it blank means using the PR title as the release note. If no release note is required, just write `NONE`. 
-->

pkg/controller/volume/attachdetach/attach_detach_controller.go

@@ -245,6 +245,23 @@ func (adc *attachDetachController) podDelete(obj interface{}) {
 
 func (adc *attachDetachController) nodeAdd(obj interface{}) {
 	node, ok := obj.(*v1.Node)
+	// TODO: investigate if nodeName is empty then if we can return
+	// kubernetes/kubernetes/issues/37777
+	if node == nil || !ok {
+		return
+	}
+	nodeName := types.NodeName(node.Name)
+	adc.nodeUpdate(nil, obj)
+	// kubernetes/kubernetes/issues/37586
+	// This is to workaround the case when a node add causes to wipe out
+	// the attached volumes field. This function ensures that we sync with
+	// the actual status.
+	adc.actualStateOfWorld.SetNodeStatusUpdateNeeded(nodeName)
+}
+
+func (adc *attachDetachController) nodeUpdate(oldObj, newObj interface{}) {
+	node, ok := newObj.(*v1.Node)
+	// TODO: investigate if nodeName is empty then if we can return
 	if node == nil || !ok {
 		return
 	}
@@ -255,15 +272,9 @@ func (adc *attachDetachController) nodeAdd(obj interface{}) {
 		// detach controller. Add it to desired state of world.
 		adc.desiredStateOfWorld.AddNode(nodeName)
 	}
-
 	adc.processVolumesInUse(nodeName, node.Status.VolumesInUse)
 }
 
-func (adc *attachDetachController) nodeUpdate(oldObj, newObj interface{}) {
-	// The flow for update is the same as add.
-	adc.nodeAdd(newObj)
-}
-
 func (adc *attachDetachController) nodeDelete(obj interface{}) {
 	node, ok := obj.(*v1.Node)
 	if node == nil || !ok {

pkg/controller/volume/attachdetach/cache/actual_state_of_world.go

@@ -116,6 +116,9 @@ type ActualStateOfWorld interface {
 	// since volumes should be removed from this list as soon a detach operation
 	// is considered, before the detach operation is triggered).
 	GetVolumesToReportAttached() map[types.NodeName][]v1.AttachedVolume
+
+	// GetNodesToUpdateStatusFor returns the map of nodeNames to nodeToUpdateStatusFor
+	GetNodesToUpdateStatusFor() map[types.NodeName]nodeToUpdateStatusFor
 }
 
 // AttachedVolume represents a volume that is attached to a node.
@@ -457,6 +460,7 @@ func (asw *actualStateOfWorld) updateNodeStatusUpdateNeeded(nodeName types.NodeN
 			"Failed to set statusUpdateNeeded to needed %t because nodeName=%q  does not exist",
 			needed,
 			nodeName)
+		return
 	}
 
 	nodeToUpdate.statusUpdateNeeded = needed
@@ -591,6 +595,10 @@ func (asw *actualStateOfWorld) GetVolumesToReportAttached() map[types.NodeName][
 	return volumesToReportAttached
 }
 
+func (asw *actualStateOfWorld) GetNodesToUpdateStatusFor() map[types.NodeName]nodeToUpdateStatusFor {
+	return asw.nodesToUpdateStatusFor
+}
+
 func getAttachedVolume(
 	attachedVolume *attachedVolume,
 	nodeAttachedTo *nodeAttachedTo) AttachedVolume {

pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go

@@ -1094,6 +1094,25 @@ func Test_OneVolumeTwoNodes_TwoDevicePaths(t *testing.T) {
 	verifyAttachedVolume(t, attachedVolumes, generatedVolumeName2, string(volumeName), node2Name, devicePath2, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */)
 }
 
+// Test_SetNodeStatusUpdateNeededError expects the map nodesToUpdateStatusFor
+// to be empty if the SetNodeStatusUpdateNeeded is called on a node that
+// does not exist in the actual state of the world
+func Test_SetNodeStatusUpdateNeededError(t *testing.T) {
+	// Arrange
+	volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t)
+	asw := NewActualStateOfWorld(volumePluginMgr)
+	nodeName := types.NodeName("node-1")
+
+	// Act
+	asw.SetNodeStatusUpdateNeeded(nodeName)
+
+	// Assert
+	nodesToUpdateStatusFor := asw.GetNodesToUpdateStatusFor()
+	if len(nodesToUpdateStatusFor) != 0 {
+		t.Fatalf("nodesToUpdateStatusFor should be empty as nodeName does not exist")
+	}
+}
+
 func verifyAttachedVolume(
 	t *testing.T,
 	attachedVolumes []AttachedVolume,

pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go

@@ -59,6 +59,8 @@ type nodeStatusUpdater struct {
 }
 
 func (nsu *nodeStatusUpdater) UpdateNodeStatuses() error {
+	// TODO: investigate right behavior if nodeName is empty
+	// kubernetes/kubernetes/issues/37777
 	nodesToUpdate := nsu.actualStateOfWorld.GetVolumesToReportAttached()
 	for nodeName, attachedVolumes := range nodesToUpdate {
 		nodeObj, exists, err := nsu.nodeInformer.GetStore().GetByKey(string(nodeName))