Fix spam of multiattach errors in event logs

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

@@ -101,6 +101,10 @@ type DesiredStateOfWorld interface {
 	// GetKeepTerminatedPodVolumesForNode determines if node wants volumes to be
 	// mounted and attached for terminated pods
 	GetKeepTerminatedPodVolumesForNode(k8stypes.NodeName) bool
+
+	// Mark multiattach error as reported to prevent spamming multiple
+	// events for same error
+	SetMultiAttachError(v1.UniqueVolumeName, k8stypes.NodeName)
 }
 
 // VolumeToAttach represents a volume that should be attached to a node.
@@ -329,6 +333,21 @@ func (dsw *desiredStateOfWorld) VolumeExists(
 	return false
 }
 
+func (dsw *desiredStateOfWorld) SetMultiAttachError(
+	volumeName v1.UniqueVolumeName,
+	nodeName k8stypes.NodeName) {
+	dsw.RLock()
+	defer dsw.RUnlock()
+
+	nodeObj, nodeExists := dsw.nodesManaged[nodeName]
+	if nodeExists {
+		if volumeObj, volumeExists := nodeObj.volumesToAttach[volumeName]; volumeExists {
+			volumeObj.multiAttachErrorReported = true
+			dsw.nodesManaged[nodeName].volumesToAttach[volumeName] = volumeObj
+		}
+	}
+}
+
 // GetKeepTerminatedPodVolumesForNode determines if node wants volumes to be
 // mounted and attached for terminated pods
 func (dsw *desiredStateOfWorld) GetKeepTerminatedPodVolumesForNode(nodeName k8stypes.NodeName) bool {

pkg/controller/volume/attachdetach/reconciler/reconciler.go

@@ -241,52 +241,56 @@ func (rc *reconciler) reconcile() {
 		}
 	}
 
+	rc.attachDesiredVolumes()
+
+	// Update Node Status
+	err := rc.nodeStatusUpdater.UpdateNodeStatuses()
+	if err != nil {
+		glog.Warningf("UpdateNodeStatuses failed with: %v", err)
+	}
+}
+
+func (rc *reconciler) attachDesiredVolumes() {
 	// Ensure volumes that should be attached are attached.
 	for _, volumeToAttach := range rc.desiredStateOfWorld.GetVolumesToAttach() {
-		if rc.actualStateOfWorld.VolumeNodeExists(
-			volumeToAttach.VolumeName, volumeToAttach.NodeName) {
+		if rc.actualStateOfWorld.VolumeNodeExists(volumeToAttach.VolumeName, volumeToAttach.NodeName) {
 			// Volume/Node exists, touch it to reset detachRequestedTime
 			glog.V(5).Infof(volumeToAttach.GenerateMsgDetailed("Volume attached--touching", ""))
 			rc.actualStateOfWorld.ResetDetachRequestTime(volumeToAttach.VolumeName, volumeToAttach.NodeName)
-		} else {
-			// Don't even try to start an operation if there is already one running
-			if rc.attacherDetacher.IsOperationPending(volumeToAttach.VolumeName, "") {
-				glog.V(10).Infof("Operation for volume %q is already running. Can't start attach for %q", volumeToAttach.VolumeName, volumeToAttach.NodeName)
-				continue
-			}
+			continue
+		}
+		// Don't even try to start an operation if there is already one running
+		if rc.attacherDetacher.IsOperationPending(volumeToAttach.VolumeName, "") {
+			glog.V(10).Infof("Operation for volume %q is already running. Can't start attach for %q", volumeToAttach.VolumeName, volumeToAttach.NodeName)
+			continue
+		}
 
-			if rc.isMultiAttachForbidden(volumeToAttach.VolumeSpec) {
-				nodes := rc.actualStateOfWorld.GetNodesForVolume(volumeToAttach.VolumeName)
-				if len(nodes) > 0 {
-					if !volumeToAttach.MultiAttachErrorReported {
-						simpleMsg, detailedMsg := volumeToAttach.GenerateMsg("Multi-Attach error", "Volume is already exclusively attached to one node and can't be attached to another")
-						for _, pod := range volumeToAttach.ScheduledPods {
-							rc.recorder.Eventf(pod, v1.EventTypeWarning, kevents.FailedAttachVolume, simpleMsg)
-						}
-						volumeToAttach.MultiAttachErrorReported = true
-						glog.Warningf(detailedMsg)
+		if rc.isMultiAttachForbidden(volumeToAttach.VolumeSpec) {
+			nodes := rc.actualStateOfWorld.GetNodesForVolume(volumeToAttach.VolumeName)
+			if len(nodes) > 0 {
+				if !volumeToAttach.MultiAttachErrorReported {
+					simpleMsg, detailedMsg := volumeToAttach.GenerateMsg("Multi-Attach error", "Volume is already exclusively attached to one node and can't be attached to another")
+					for _, pod := range volumeToAttach.ScheduledPods {
+						rc.recorder.Eventf(pod, v1.EventTypeWarning, kevents.FailedAttachVolume, simpleMsg)
 					}
-					continue
+					rc.desiredStateOfWorld.SetMultiAttachError(volumeToAttach.VolumeName, volumeToAttach.NodeName)
+					glog.Warningf(detailedMsg)
 				}
+				continue
 			}
+		}
 
-			// Volume/Node doesn't exist, spawn a goroutine to attach it
-			glog.V(5).Infof(volumeToAttach.GenerateMsgDetailed("Starting attacherDetacher.AttachVolume", ""))
-			err := rc.attacherDetacher.AttachVolume(volumeToAttach.VolumeToAttach, rc.actualStateOfWorld)
-			if err == nil {
-				glog.Infof(volumeToAttach.GenerateMsgDetailed("attacherDetacher.AttachVolume started", ""))
-			}
-			if err != nil && !exponentialbackoff.IsExponentialBackoff(err) {
-				// Ignore exponentialbackoff.IsExponentialBackoff errors, they are expected.
-				// Log all other errors.
-				glog.Errorf(volumeToAttach.GenerateErrorDetailed("attacherDetacher.AttachVolume failed to start", err).Error())
-			}
+		// Volume/Node doesn't exist, spawn a goroutine to attach it
+		glog.V(5).Infof(volumeToAttach.GenerateMsgDetailed("Starting attacherDetacher.AttachVolume", ""))
+		err := rc.attacherDetacher.AttachVolume(volumeToAttach.VolumeToAttach, rc.actualStateOfWorld)
+		if err == nil {
+			glog.Infof(volumeToAttach.GenerateMsgDetailed("attacherDetacher.AttachVolume started", ""))
+		}
+		if err != nil && !exponentialbackoff.IsExponentialBackoff(err) {
+			// Ignore exponentialbackoff.IsExponentialBackoff errors, they are expected.
+			// Log all other errors.
+			glog.Errorf(volumeToAttach.GenerateErrorDetailed("attacherDetacher.AttachVolume failed to start", err).Error())
 		}
 	}
 
-	// Update Node Status
-	err := rc.nodeStatusUpdater.UpdateNodeStatuses()
-	if err != nil {
-		glog.Warningf("UpdateNodeStatuses failed with: %v", err)
-	}
 }

pkg/controller/volume/attachdetach/reconciler/reconciler_test.go

@@ -455,6 +455,17 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteOnce(t *testing.
 
 	nodesForVolume := asw.GetNodesForVolume(generatedVolumeName)
 
+	// check if multiattach is marked
+	// at least one volume should be marked with multiattach error
+	nodeAttachedTo := nodesForVolume[0]
+	for _, volumeToAttach := range dsw.GetVolumesToAttach() {
+		if volumeToAttach.NodeName != nodeAttachedTo {
+			if !volumeToAttach.MultiAttachErrorReported {
+				t.Fatalf("Expected volume %q on node %q to have multiattach error", volumeToAttach.VolumeName, volumeToAttach.NodeName)
+			}
+		}
+	}
+
 	// Act
 	podToDelete := ""
 	if nodesForVolume[0] == nodeName1 {