diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index 49fc16a7a2e..b7a64f846cd 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/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 { 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 3bf7ff1756a..dbb5cb698a7 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go +++ b/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 { 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 9c0e05e66e0..22bee95ff1f 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 @@ -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, diff --git a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go b/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go index 90f85f99aad..5c27763e4c6 100644 --- a/pkg/controller/volume/attachdetach/statusupdater/node_status_updater.go +++ b/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))