Merge pull request #44969 from ianchakeres/ianchakeres/mount-err-cleanup

Automatic merge from submit-queue (batch tested with PRs 44590, 44969, 45325, 45208, 44714)

Refactor volume operation log and error messages

What this PR does / why we need it:
Adds wrappers for volume-specific error and log messages. Each message has a simple version that can be displayed to the user and a detailed version that can be used in logs. The messages that are used for events was also cleaned up. @msau42 

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #40905

Special notes for your reviewer:
pkg/kubelet/volumemanager/reconciler/reconciler.go can be refactored. I can do that refactoring after this one.

Release note:

NONE

pkg/volume/util/operationexecutor/operation_executor.go

@@ -21,6 +21,7 @@ limitations under the License.
 package operationexecutor
 
 import (
+	"fmt"
 	"strings"
 	"time"
 
@@ -167,6 +168,47 @@ type ActualStateOfWorldAttacherUpdater interface {
 	AddVolumeToReportAsAttached(volumeName v1.UniqueVolumeName, nodeName types.NodeName)
 }
 
+// VolumeLogger defines a set of operations for generating volume-related logging and error msgs
+type VolumeLogger interface {
+	// Creates a detailed msg that can be used in logs
+	// The msg format follows the pattern "<prefixMsg> <volume details> <suffixMsg>",
+	// where each implementation provides the volume details
+	GenerateMsgDetailed(prefixMsg, suffixMsg string) (detailedMsg string)
+
+	// Creates a detailed error that can be used in logs.
+	// The msg format follows the pattern "<prefixMsg> <volume details>: <err> ",
+	GenerateErrorDetailed(prefixMsg string, err error) (detailedErr error)
+
+	// Creates a simple msg that is user friendly and a detailed msg that can be used in logs
+	// The msg format follows the pattern "<prefixMsg> <volume details> <suffixMsg>",
+	// where each implementation provides the volume details
+	GenerateMsg(prefixMsg, suffixMsg string) (simpleMsg, detailedMsg string)
+
+	// Creates a simple error that is user friendly and a detailed error that can be used in logs.
+	// The msg format follows the pattern "<prefixMsg> <volume details>: <err> ",
+	GenerateError(prefixMsg string, err error) (simpleErr, detailedErr error)
+}
+
+// Generates an error string with the format ": <err>" if err exists
+func errSuffix(err error) string {
+	errStr := ""
+	if err != nil {
+		errStr = fmt.Sprintf(": %v", err)
+	}
+	return errStr
+}
+
+// Generate a detailed error msg for logs
+func generateVolumeMsgDetailed(prefixMsg, suffixMsg, volumeName, details string) (detailedMsg string) {
+	return fmt.Sprintf("%v for volume %q %v %v", prefixMsg, volumeName, details, suffixMsg)
+}
+
+// Generate a simplified error msg for events and a detailed error msg for logs
+func generateVolumeMsg(prefixMsg, suffixMsg, volumeName, details string) (simpleMsg, detailedMsg string) {
+	simpleMsg = fmt.Sprintf("%v for volume %q %v", prefixMsg, volumeName, suffixMsg)
+	return simpleMsg, generateVolumeMsgDetailed(prefixMsg, suffixMsg, volumeName, details)
+}
+
 // VolumeToAttach represents a volume that should be attached to a node.
 type VolumeToAttach struct {
 	// VolumeName is the unique identifier for the volume that should be
@@ -188,6 +230,37 @@ type VolumeToAttach struct {
 	ScheduledPods []*v1.Pod
 }
 
+// GenerateMsgDetailed returns detailed msgs for volumes to attach
+func (volume *VolumeToAttach) GenerateMsgDetailed(prefixMsg, suffixMsg string) (detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) from node %q", volume.VolumeName, volume.NodeName)
+	volumeSpecName := "nil"
+	if volume.VolumeSpec != nil {
+		volumeSpecName = volume.VolumeSpec.Name()
+	}
+	return generateVolumeMsgDetailed(prefixMsg, suffixMsg, volumeSpecName, detailedStr)
+}
+
+// GenerateMsg returns simple and detailed msgs for volumes to attach
+func (volume *VolumeToAttach) GenerateMsg(prefixMsg, suffixMsg string) (simpleMsg, detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) from node %q", volume.VolumeName, volume.NodeName)
+	volumeSpecName := "nil"
+	if volume.VolumeSpec != nil {
+		volumeSpecName = volume.VolumeSpec.Name()
+	}
+	return generateVolumeMsg(prefixMsg, suffixMsg, volumeSpecName, detailedStr)
+}
+
+// GenerateErrorDetailed returns detailed errors for volumes to attach
+func (volume *VolumeToAttach) GenerateErrorDetailed(prefixMsg string, err error) (detailedErr error) {
+	return fmt.Errorf(volume.GenerateMsgDetailed(prefixMsg, errSuffix(err)))
+}
+
+// GenerateError returns simple and detailed errors for volumes to attach
+func (volume *VolumeToAttach) GenerateError(prefixMsg string, err error) (simpleErr, detailedErr error) {
+	simpleMsg, detailedMsg := volume.GenerateMsg(prefixMsg, errSuffix(err))
+	return fmt.Errorf(simpleMsg), fmt.Errorf(detailedMsg)
+}
+
 // VolumeToMount represents a volume that should be attached to this node and
 // mounted to the PodName.
 type VolumeToMount struct {
@@ -228,6 +301,37 @@ type VolumeToMount struct {
 	ReportedInUse bool
 }
 
+// GenerateMsgDetailed returns detailed msgs for volumes to mount
+func (volume *VolumeToMount) GenerateMsgDetailed(prefixMsg, suffixMsg string) (detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) pod %q (UID: %q)", volume.VolumeName, volume.Pod.Name, volume.Pod.UID)
+	volumeSpecName := "nil"
+	if volume.VolumeSpec != nil {
+		volumeSpecName = volume.VolumeSpec.Name()
+	}
+	return generateVolumeMsgDetailed(prefixMsg, suffixMsg, volumeSpecName, detailedStr)
+}
+
+// GenerateMsg returns simple and detailed msgs for volumes to mount
+func (volume *VolumeToMount) GenerateMsg(prefixMsg, suffixMsg string) (simpleMsg, detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) pod %q (UID: %q)", volume.VolumeName, volume.Pod.Name, volume.Pod.UID)
+	volumeSpecName := "nil"
+	if volume.VolumeSpec != nil {
+		volumeSpecName = volume.VolumeSpec.Name()
+	}
+	return generateVolumeMsg(prefixMsg, suffixMsg, volumeSpecName, detailedStr)
+}
+
+// GenerateErrorDetailed returns detailed errors for volumes to mount
+func (volume *VolumeToMount) GenerateErrorDetailed(prefixMsg string, err error) (detailedErr error) {
+	return fmt.Errorf(volume.GenerateMsgDetailed(prefixMsg, errSuffix(err)))
+}
+
+// GenerateError returns simple and detailed errors for volumes to mount
+func (volume *VolumeToMount) GenerateError(prefixMsg string, err error) (simpleErr, detailedErr error) {
+	simpleMsg, detailedMsg := volume.GenerateMsg(prefixMsg, errSuffix(err))
+	return fmt.Errorf(simpleMsg), fmt.Errorf(detailedMsg)
+}
+
 // AttachedVolume represents a volume that is attached to a node.
 type AttachedVolume struct {
 	// VolumeName is the unique identifier for the volume that is attached.
@@ -249,6 +353,37 @@ type AttachedVolume struct {
 	DevicePath string
 }
 
+// GenerateMsgDetailed returns detailed msgs for attached volumes
+func (volume *AttachedVolume) GenerateMsgDetailed(prefixMsg, suffixMsg string) (detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) on node %q", volume.VolumeName, volume.NodeName)
+	volumeSpecName := "nil"
+	if volume.VolumeSpec != nil {
+		volumeSpecName = volume.VolumeSpec.Name()
+	}
+	return generateVolumeMsgDetailed(prefixMsg, suffixMsg, volumeSpecName, detailedStr)
+}
+
+// GenerateMsg returns simple and detailed msgs for attached volumes
+func (volume *AttachedVolume) GenerateMsg(prefixMsg, suffixMsg string) (simpleMsg, detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) on node %q", volume.VolumeName, volume.NodeName)
+	volumeSpecName := "nil"
+	if volume.VolumeSpec != nil {
+		volumeSpecName = volume.VolumeSpec.Name()
+	}
+	return generateVolumeMsg(prefixMsg, suffixMsg, volumeSpecName, detailedStr)
+}
+
+// GenerateErrorDetailed returns detailed errors for attached volumes
+func (volume *AttachedVolume) GenerateErrorDetailed(prefixMsg string, err error) (detailedErr error) {
+	return fmt.Errorf(volume.GenerateMsgDetailed(prefixMsg, errSuffix(err)))
+}
+
+// GenerateError returns simple and detailed errors for attached volumes
+func (volume *AttachedVolume) GenerateError(prefixMsg string, err error) (simpleErr, detailedErr error) {
+	simpleMsg, detailedMsg := volume.GenerateMsg(prefixMsg, errSuffix(err))
+	return fmt.Errorf(simpleMsg), fmt.Errorf(detailedMsg)
+}
+
 // MountedVolume represents a volume that has successfully been mounted to a pod.
 type MountedVolume struct {
 	// PodName is the unique identifier of the pod mounted to.
@@ -353,6 +488,29 @@ type MountedVolume struct {
 	VolumeGidValue string
 }
 
+// GenerateMsgDetailed returns detailed msgs for mounted volumes
+func (volume *MountedVolume) GenerateMsgDetailed(prefixMsg, suffixMsg string) (detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) pod %q (UID: %q)", volume.VolumeName, volume.PodName, volume.PodUID)
+	return generateVolumeMsgDetailed(prefixMsg, suffixMsg, volume.OuterVolumeSpecName, detailedStr)
+}
+
+// GenerateMsg returns simple and detailed msgs for mounted volumes
+func (volume *MountedVolume) GenerateMsg(prefixMsg, suffixMsg string) (simpleMsg, detailedMsg string) {
+	detailedStr := fmt.Sprintf("(UniqueName: %q) pod %q (UID: %q)", volume.VolumeName, volume.PodName, volume.PodUID)
+	return generateVolumeMsg(prefixMsg, suffixMsg, volume.OuterVolumeSpecName, detailedStr)
+}
+
+// GenerateErrorDetailed returns simple and detailed errors for mounted volumes
+func (volume *MountedVolume) GenerateErrorDetailed(prefixMsg string, err error) (detailedErr error) {
+	return fmt.Errorf(volume.GenerateMsgDetailed(prefixMsg, errSuffix(err)))
+}
+
+// GenerateError returns simple and detailed errors for mounted volumes
+func (volume *MountedVolume) GenerateError(prefixMsg string, err error) (simpleErr, detailedErr error) {
+	simpleMsg, detailedMsg := volume.GenerateMsg(prefixMsg, errSuffix(err))
+	return fmt.Errorf(simpleMsg), fmt.Errorf(detailedMsg)
+}
+
 type operationExecutor struct {
 	// pendingOperations keeps track of pending attach and detach operations so
 	// multiple operations are not started on the same volume
@@ -463,7 +621,7 @@ func (oe *operationExecutor) VerifyVolumesAreAttached(
 			volumeSpecMapByPlugin[pluginName],
 			actualStateOfWorld)
 		if err != nil {
-			glog.Errorf("BulkVerifyVolumes.GenerateBulkVolumeVerifyFunc  error bulk verifying volumes for plugin %q with  %v", pluginName, err)
+			glog.Errorf("BulkVerifyVolumes.GenerateBulkVolumeVerifyFunc error bulk verifying volumes for plugin %q with  %v", pluginName, err)
 		}
 		// Ugly hack to ensure - we don't do parallel bulk polling of same volume plugin
 		uniquePluginName := v1.UniqueVolumeName(pluginName)

pkg/volume/util/operationexecutor/operation_generator.go

@@ -115,12 +115,7 @@ func (og *operationGenerator) GenerateVolumesAreAttachedFunc(
 		volumePlugin, err :=
 			og.volumePluginMgr.FindPluginBySpec(volumeAttached.VolumeSpec)
 		if err != nil || volumePlugin == nil {
-			glog.Errorf(
+			glog.Errorf(volumeAttached.GenerateErrorDetailed("VolumesAreAttached.FindPluginBySpec failed", err).Error())
-				"VolumesAreAttached.FindPluginBySpec failed for volume %q (spec.Name: %q) on node %q with error: %v",
-				volumeAttached.VolumeName,
-				volumeAttached.VolumeSpec.Name(),
-				volumeAttached.NodeName,
-				err)
 		}
 		volumeSpecList, pluginExists := volumesPerPlugin[volumePlugin.GetPluginName()]
 		if !pluginExists {
@@ -149,7 +144,7 @@ func (og *operationGenerator) GenerateVolumesAreAttachedFunc(
 			volumeAttacher, newAttacherErr := attachableVolumePlugin.NewAttacher()
 			if newAttacherErr != nil {
 				glog.Errorf(
-					"VolumesAreAttached failed for getting plugin %q with: %v",
+					"VolumesAreAttached.NewAttacher failed for getting plugin %q with: %v",
 					pluginName,
 					newAttacherErr)
 				continue
@@ -197,7 +192,7 @@ func (og *operationGenerator) GenerateBulkVolumeVerifyFunc(
 
 		if newAttacherErr != nil {
 			glog.Errorf(
-				"BulkVerifyVolumes failed for getting plugin %q with: %v",
+				"BulkVerifyVolume.NewAttacher failed for getting plugin %q with: %v",
 				attachableVolumePlugin,
 				newAttacherErr)
 			return nil
@@ -248,22 +243,12 @@ func (og *operationGenerator) GenerateAttachVolumeFunc(
 	attachableVolumePlugin, err :=
 		og.volumePluginMgr.FindAttachablePluginBySpec(volumeToAttach.VolumeSpec)
 	if err != nil || attachableVolumePlugin == nil {
-		return nil, fmt.Errorf(
+		return nil, volumeToAttach.GenerateErrorDetailed("AttachVolume.FindAttachablePluginBySpec failed", err)
-			"AttachVolume.FindAttachablePluginBySpec failed for volume %q (spec.Name: %q) from node %q with: %v",
-			volumeToAttach.VolumeName,
-			volumeToAttach.VolumeSpec.Name(),
-			volumeToAttach.NodeName,
-			err)
 	}
 
 	volumeAttacher, newAttacherErr := attachableVolumePlugin.NewAttacher()
 	if newAttacherErr != nil {
-		return nil, fmt.Errorf(
+		return nil, volumeToAttach.GenerateErrorDetailed("AttachVolume.NewAttacher failed", newAttacherErr)
-			"AttachVolume.NewAttacher failed for volume %q (spec.Name: %q) from node %q with: %v",
-			volumeToAttach.VolumeName,
-			volumeToAttach.VolumeSpec.Name(),
-			volumeToAttach.NodeName,
-			newAttacherErr)
 	}
 
 	return func() error {
@@ -273,34 +258,21 @@ func (og *operationGenerator) GenerateAttachVolumeFunc(
 
 		if attachErr != nil {
 			// On failure, return error. Caller will log and retry.
-			err := fmt.Errorf(
+			eventErr, detailedErr := volumeToAttach.GenerateError("AttachVolume.Attach failed", attachErr)
-				"Failed to attach volume %q on node %q with: %v",
-				volumeToAttach.VolumeSpec.Name(),
-				volumeToAttach.NodeName,
-				attachErr)
 			for _, pod := range volumeToAttach.ScheduledPods {
-				og.recorder.Eventf(pod, v1.EventTypeWarning, kevents.FailedMountVolume, err.Error())
+				og.recorder.Eventf(pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error())
 			}
-			return err
+			return detailedErr
 		}
 
-		glog.Infof(
+		glog.Infof(volumeToAttach.GenerateMsgDetailed("AttachVolume.Attach succeeded", ""))
-			"AttachVolume.Attach succeeded for volume %q (spec.Name: %q) from node %q.",
-			volumeToAttach.VolumeName,
-			volumeToAttach.VolumeSpec.Name(),
-			volumeToAttach.NodeName)
 
 		// Update actual state of world
 		addVolumeNodeErr := actualStateOfWorld.MarkVolumeAsAttached(
 			v1.UniqueVolumeName(""), volumeToAttach.VolumeSpec, volumeToAttach.NodeName, devicePath)
 		if addVolumeNodeErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return volumeToAttach.GenerateErrorDetailed("AttachVolume.MarkVolumeAsAttached failed", addVolumeNodeErr)
-				"AttachVolume.MarkVolumeAsAttached failed for volume %q (spec.Name: %q) from node %q with: %v",
-				volumeToAttach.VolumeName,
-				volumeToAttach.VolumeSpec.Name(),
-				volumeToAttach.NodeName,
-				addVolumeNodeErr)
 		}
 
 		return nil
@@ -324,16 +296,14 @@ func (og *operationGenerator) GenerateDetachVolumeFunc(
 		attachableVolumePlugin, err =
 			og.volumePluginMgr.FindAttachablePluginBySpec(volumeToDetach.VolumeSpec)
 		if err != nil || attachableVolumePlugin == nil {
-			return nil, fmt.Errorf(
+			return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.FindAttachablePluginBySpec failed", err)
-				"DetachVolume.FindAttachablePluginBySpec failed for volume %q (spec.Name: %q) from node %q with: %v",
-				volumeToDetach.VolumeName,
-				volumeToDetach.VolumeSpec.Name(),
-				volumeToDetach.NodeName,
-				err)
 		}
 
 		volumeName, err =
 			attachableVolumePlugin.GetVolumeName(volumeToDetach.VolumeSpec)
+		if err != nil {
+			return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.GetVolumeName failed", err)
+		}
 	} else {
 		var pluginName string
 		// Get attacher plugin and the volumeName by splitting the volume unique name in case
@@ -341,24 +311,16 @@ func (og *operationGenerator) GenerateDetachVolumeFunc(
 		// when a pod has been deleted during the controller downtime
 		pluginName, volumeName, err = volumehelper.SplitUniqueName(volumeToDetach.VolumeName)
 		if err != nil {
-			return nil, err
+			return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.SplitUniqueName failed", err)
 		}
 		attachableVolumePlugin, err = og.volumePluginMgr.FindAttachablePluginByName(pluginName)
 		if err != nil {
-			return nil, fmt.Errorf(
+			return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.FindAttachablePluginBySpec failed", err)
-				"Failed to find plugin for volume %q from node %q with: %v",
-				volumeToDetach.VolumeName,
-				volumeToDetach.NodeName,
-				err)
 		}
 	}
 	volumeDetacher, err := attachableVolumePlugin.NewDetacher()
 	if err != nil {
-		return nil, fmt.Errorf(
+		return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.NewDetacher failed", err)
-			"DetachVolume.NewDetacher failed for volume %q from node %q with: %v",
-			volumeToDetach.VolumeName,
-			volumeToDetach.NodeName,
-			err)
 	}
 
 	return func() error {
@@ -373,17 +335,10 @@ func (og *operationGenerator) GenerateDetachVolumeFunc(
 			// On failure, add volume back to ReportAsAttached list
 			actualStateOfWorld.AddVolumeToReportAsAttached(
 				volumeToDetach.VolumeName, volumeToDetach.NodeName)
-			return fmt.Errorf(
+			return volumeToDetach.GenerateErrorDetailed("DetachVolume.Detach failed", err)
-				"DetachVolume.Detach failed for volume %q from node %q with: %v",
-				volumeToDetach.VolumeName,
-				volumeToDetach.NodeName,
-				err)
 		}
 
-		glog.Infof(
+		glog.Infof(volumeToDetach.GenerateMsgDetailed("DetachVolume.Detach succeeded", ""))
-			"DetachVolume.Detach succeeded for volume %q from node %q.",
-			volumeToDetach.VolumeName,
-			volumeToDetach.NodeName)
 
 		// Update actual state of world
 		actualStateOfWorld.MarkVolumeAsDetached(
@@ -401,13 +356,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 	volumePlugin, err :=
 		og.volumePluginMgr.FindPluginBySpec(volumeToMount.VolumeSpec)
 	if err != nil || volumePlugin == nil {
-		return nil, fmt.Errorf(
+		return nil, volumeToMount.GenerateErrorDetailed("MountVolume.FindPluginBySpec failed", err)
-			"MountVolume.FindPluginBySpec failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-			volumeToMount.VolumeName,
-			volumeToMount.VolumeSpec.Name(),
-			volumeToMount.PodName,
-			volumeToMount.Pod.UID,
-			err)
 	}
 
 	volumeMounter, newMounterErr := volumePlugin.NewMounter(
@@ -415,15 +364,9 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 		volumeToMount.Pod,
 		volume.VolumeOptions{})
 	if newMounterErr != nil {
-		errMsg := fmt.Errorf(
+		eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.NewMounter initialization failed", newMounterErr)
-			"MountVolume.NewMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
+		og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error())
-			volumeToMount.VolumeName,
+		return nil, detailedErr
-			volumeToMount.VolumeSpec.Name(),
-			volumeToMount.PodName,
-			volumeToMount.Pod.UID,
-			newMounterErr)
-		og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, errMsg.Error())
-		return nil, errMsg
 	}
 
 	mountCheckError := checkMountOptionSupport(og, volumeToMount, volumePlugin)
@@ -449,45 +392,22 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 	return func() error {
 		if volumeAttacher != nil {
 			// Wait for attachable volumes to finish attaching
-			glog.Infof(
+			glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach entering", fmt.Sprintf("DevicePath %q", volumeToMount.DevicePath)))
-				"Entering MountVolume.WaitForAttach for volume %q (spec.Name: %q) pod %q (UID: %q) DevicePath: %q",
-				volumeToMount.VolumeName,
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID,
-				volumeToMount.DevicePath)
 
 			devicePath, err := volumeAttacher.WaitForAttach(
 				volumeToMount.VolumeSpec, volumeToMount.DevicePath, waitForAttachTimeout)
 			if err != nil {
 				// On failure, return error. Caller will log and retry.
-				return fmt.Errorf(
+				return volumeToMount.GenerateErrorDetailed("MountVolume.WaitForAttach failed", err)
-					"MountVolume.WaitForAttach failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-					volumeToMount.VolumeName,
-					volumeToMount.VolumeSpec.Name(),
-					volumeToMount.PodName,
-					volumeToMount.Pod.UID,
-					err)
 			}
 
-			glog.Infof(
+			glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach succeeded", ""))
-				"MountVolume.WaitForAttach succeeded for volume %q (spec.Name: %q) pod %q (UID: %q).",
-				volumeToMount.VolumeName,
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID)
 
 			deviceMountPath, err :=
 				volumeAttacher.GetDeviceMountPath(volumeToMount.VolumeSpec)
 			if err != nil {
 				// On failure, return error. Caller will log and retry.
-				return fmt.Errorf(
+				return volumeToMount.GenerateErrorDetailed("MountVolume.GetDeviceMountPath failed", err)
-					"MountVolume.GetDeviceMountPath failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-					volumeToMount.VolumeName,
-					volumeToMount.VolumeSpec.Name(),
-					volumeToMount.PodName,
-					volumeToMount.Pod.UID,
-					err)
 			}
 
 			// Mount device to global mount path
@@ -497,46 +417,30 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 				deviceMountPath)
 			if err != nil {
 				// On failure, return error. Caller will log and retry.
-				err := fmt.Errorf(
+				eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.MountDevice failed", err)
-					"MountVolume.MountDevice failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
+				og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error())
-					volumeToMount.VolumeName,
+				return detailedErr
-					volumeToMount.VolumeSpec.Name(),
-					volumeToMount.PodName,
-					volumeToMount.Pod.UID,
-					err)
-				og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, err.Error())
-				return err
 			}
 
-			glog.Infof(
+			glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.MountDevice succeeded", fmt.Sprintf("device mount path %q", deviceMountPath)))
-				"MountVolume.MountDevice succeeded for volume %q (spec.Name: %q) pod %q (UID: %q) device mount path %q",
-				volumeToMount.VolumeName,
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID,
-				deviceMountPath)
 
 			// Update actual state of world to reflect volume is globally mounted
 			markDeviceMountedErr := actualStateOfWorld.MarkDeviceAsMounted(
 				volumeToMount.VolumeName)
 			if markDeviceMountedErr != nil {
 				// On failure, return error. Caller will log and retry.
-				return fmt.Errorf(
+				return volumeToMount.GenerateErrorDetailed("MountVolume.MarkDeviceAsMounted failed", markDeviceMountedErr)
-					"MountVolume.MarkDeviceAsMounted failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-					volumeToMount.VolumeName,
-					volumeToMount.VolumeSpec.Name(),
-					volumeToMount.PodName,
-					volumeToMount.Pod.UID,
-					markDeviceMountedErr)
 			}
 		}
 
 		if og.checkNodeCapabilitiesBeforeMount {
 			if canMountErr := volumeMounter.CanMount(); canMountErr != nil {
-				errMsg := fmt.Sprintf("Unable to mount volume %v (spec.Name: %v) on pod %v (UID: %v). Verify that your node machine has the required components before attempting to mount this volume type. %s", volumeToMount.VolumeName, volumeToMount.VolumeSpec.Name(), volumeToMount.Pod.Name, volumeToMount.Pod.UID, canMountErr.Error())
+				err = fmt.Errorf(
-				og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, errMsg)
+					"Verify that your node machine has the required components before attempting to mount this volume type. %s",
-				glog.Errorf(errMsg)
+					canMountErr)
-				return fmt.Errorf(errMsg)
+				eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.CanMount failed", err)
+				og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error())
+				return detailedErr
 			}
 		}
 
@@ -544,23 +448,12 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 		mountErr := volumeMounter.SetUp(fsGroup)
 		if mountErr != nil {
 			// On failure, return error. Caller will log and retry.
-			err := fmt.Errorf(
+			eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.SetUp failed", mountErr)
-				"MountVolume.SetUp failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
+			og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error())
-				volumeToMount.VolumeName,
+			return detailedErr
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID,
-				mountErr)
-			og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, err.Error())
-			return err
 		}
 
-		glog.Infof(
+		glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.SetUp succeeded", ""))
-			"MountVolume.SetUp succeeded for volume %q (spec.Name: %q) pod %q (UID: %q).",
-			volumeToMount.VolumeName,
-			volumeToMount.VolumeSpec.Name(),
-			volumeToMount.PodName,
-			volumeToMount.Pod.UID)
 
 		// Update actual state of world
 		markVolMountedErr := actualStateOfWorld.MarkVolumeAsMounted(
@@ -572,13 +465,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
 			volumeToMount.VolumeGidValue)
 		if markVolMountedErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeAsMounted failed", markVolMountedErr)
-				"MountVolume.MarkVolumeAsMounted failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-				volumeToMount.VolumeName,
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID,
-				markVolMountedErr)
 		}
 
 		return nil
@@ -592,25 +479,13 @@ func (og *operationGenerator) GenerateUnmountVolumeFunc(
 	volumePlugin, err :=
 		og.volumePluginMgr.FindPluginByName(volumeToUnmount.PluginName)
 	if err != nil || volumePlugin == nil {
-		return nil, fmt.Errorf(
+		return nil, volumeToUnmount.GenerateErrorDetailed("UnmountVolume.FindPluginByName failed", err)
-			"UnmountVolume.FindPluginByName failed for volume %q (volume.spec.Name: %q) pod %q (UID: %q) err=%v",
-			volumeToUnmount.VolumeName,
-			volumeToUnmount.OuterVolumeSpecName,
-			volumeToUnmount.PodName,
-			volumeToUnmount.PodUID,
-			err)
 	}
 
 	volumeUnmounter, newUnmounterErr := volumePlugin.NewUnmounter(
 		volumeToUnmount.InnerVolumeSpecName, volumeToUnmount.PodUID)
 	if newUnmounterErr != nil {
-		return nil, fmt.Errorf(
+		return nil, volumeToUnmount.GenerateErrorDetailed("UnmountVolume.NewUnmounter failed", newUnmounterErr)
-			"UnmountVolume.NewUnmounter failed for volume %q (volume.spec.Name: %q) pod %q (UID: %q) err=%v",
-			volumeToUnmount.VolumeName,
-			volumeToUnmount.OuterVolumeSpecName,
-			volumeToUnmount.PodName,
-			volumeToUnmount.PodUID,
-			newUnmounterErr)
 	}
 
 	return func() error {
@@ -618,13 +493,7 @@ func (og *operationGenerator) GenerateUnmountVolumeFunc(
 		unmountErr := volumeUnmounter.TearDown()
 		if unmountErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return volumeToUnmount.GenerateErrorDetailed("UnmountVolume.TearDown failed", unmountErr)
-				"UnmountVolume.TearDown failed for volume %q (volume.spec.Name: %q) pod %q (UID: %q) with: %v",
-				volumeToUnmount.VolumeName,
-				volumeToUnmount.OuterVolumeSpecName,
-				volumeToUnmount.PodName,
-				volumeToUnmount.PodUID,
-				unmountErr)
 		}
 
 		glog.Infof(
@@ -642,13 +511,7 @@ func (og *operationGenerator) GenerateUnmountVolumeFunc(
 			volumeToUnmount.PodName, volumeToUnmount.VolumeName)
 		if markVolMountedErr != nil {
 			// On failure, just log and exit
-			glog.Errorf(
+			glog.Errorf(volumeToUnmount.GenerateErrorDetailed("UnmountVolume.MarkVolumeAsUnmounted failed", markVolMountedErr).Error())
-				"UnmountVolume.MarkVolumeAsUnmounted failed for volume %q (volume.spec.Name: %q) pod %q (UID: %q) with: %v",
-				volumeToUnmount.VolumeName,
-				volumeToUnmount.OuterVolumeSpecName,
-				volumeToUnmount.PodName,
-				volumeToUnmount.PodUID,
-				markVolMountedErr)
 		}
 
 		return nil
@@ -663,29 +526,17 @@ func (og *operationGenerator) GenerateUnmountDeviceFunc(
 	attachableVolumePlugin, err :=
 		og.volumePluginMgr.FindAttachablePluginBySpec(deviceToDetach.VolumeSpec)
 	if err != nil || attachableVolumePlugin == nil {
-		return nil, fmt.Errorf(
+		return nil, deviceToDetach.GenerateErrorDetailed("UnmountDevice.FindAttachablePluginBySpec failed", err)
-			"UnmountDevice.FindAttachablePluginBySpec failed for volume %q (spec.Name: %q) with: %v",
-			deviceToDetach.VolumeName,
-			deviceToDetach.VolumeSpec.Name(),
-			err)
 	}
 
 	volumeDetacher, err := attachableVolumePlugin.NewDetacher()
 	if err != nil {
-		return nil, fmt.Errorf(
+		return nil, deviceToDetach.GenerateErrorDetailed("UnmountDevice.NewDetacher failed", err)
-			"UnmountDevice.NewDetacher failed for volume %q (spec.Name: %q) with: %v",
-			deviceToDetach.VolumeName,
-			deviceToDetach.VolumeSpec.Name(),
-			err)
 	}
 
 	volumeAttacher, err := attachableVolumePlugin.NewAttacher()
 	if err != nil {
-		return nil, fmt.Errorf(
+		return nil, deviceToDetach.GenerateErrorDetailed("UnmountDevice.NewAttacher failed", err)
-			"UnmountDevice.NewAttacher failed for volume %q (spec.Name: %q) with: %v",
-			deviceToDetach.VolumeName,
-			deviceToDetach.VolumeSpec.Name(),
-			err)
 	}
 
 	return func() error {
@@ -693,11 +544,7 @@ func (og *operationGenerator) GenerateUnmountDeviceFunc(
 			volumeAttacher.GetDeviceMountPath(deviceToDetach.VolumeSpec)
 		if err != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return deviceToDetach.GenerateErrorDetailed("GetDeviceMountPath failed", err)
-				"GetDeviceMountPath failed for volume %q (spec.Name: %q) with: %v",
-				deviceToDetach.VolumeName,
-				deviceToDetach.VolumeSpec.Name(),
-				err)
 		}
 		refs, err := attachableVolumePlugin.GetDeviceMountRefs(deviceMountPath)
 
@@ -705,21 +552,13 @@ func (og *operationGenerator) GenerateUnmountDeviceFunc(
 			if err == nil {
 				err = fmt.Errorf("The device mount path %q is still mounted by other references %v", deviceMountPath, refs)
 			}
-			return fmt.Errorf(
+			return deviceToDetach.GenerateErrorDetailed("GetDeviceMountRefs check failed", err)
-				"GetDeviceMountRefs check failed for volume %q (spec.Name: %q) with: %v",
-				deviceToDetach.VolumeName,
-				deviceToDetach.VolumeSpec.Name(),
-				err)
 		}
 		// Execute unmount
 		unmountDeviceErr := volumeDetacher.UnmountDevice(deviceMountPath)
 		if unmountDeviceErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return deviceToDetach.GenerateErrorDetailed("UnmountDevice failed", unmountDeviceErr)
-				"UnmountDevice failed for volume %q (spec.Name: %q) with: %v",
-				deviceToDetach.VolumeName,
-				deviceToDetach.VolumeSpec.Name(),
-				unmountDeviceErr)
 		}
 		// Before logging that UnmountDevice succeeded and moving on,
 		// use mounter.PathIsDevice to check if the path is a device,
@@ -736,36 +575,24 @@ func (og *operationGenerator) GenerateUnmountDeviceFunc(
 		} else {
 			deviceOpened, deviceOpenedErr = mounter.DeviceOpened(deviceToDetach.DevicePath)
 			if deviceOpenedErr != nil {
-				return fmt.Errorf(
+				return deviceToDetach.GenerateErrorDetailed("UnmountDevice.DeviceOpened failed", deviceOpenedErr)
-					"UnmountDevice.DeviceOpened failed for volume %q (spec.Name: %q) with: %v",
-					deviceToDetach.VolumeName,
-					deviceToDetach.VolumeSpec.Name(),
-					deviceOpenedErr)
 			}
 		}
 		// The device is still in use elsewhere. Caller will log and retry.
 		if deviceOpened {
-			return fmt.Errorf(
+			return deviceToDetach.GenerateErrorDetailed(
-				"UnmountDevice failed for volume %q (spec.Name: %q) because the device is in use when it was no longer expected to be in use",
+				"UnmountDevice failed",
-				deviceToDetach.VolumeName,
+				fmt.Errorf("the device is in use when it was no longer expected to be in use"))
-				deviceToDetach.VolumeSpec.Name())
 		}
 
-		glog.Infof(
+		glog.Infof(deviceToDetach.GenerateMsgDetailed("UnmountDevice succeeded", ""))
-			"UnmountDevice succeeded for volume %q (spec.Name: %q).",
-			deviceToDetach.VolumeName,
-			deviceToDetach.VolumeSpec.Name())
 
 		// Update actual state of world
 		markDeviceUnmountedErr := actualStateOfWorld.MarkDeviceAsUnmounted(
 			deviceToDetach.VolumeName)
 		if markDeviceUnmountedErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return deviceToDetach.GenerateErrorDetailed("MarkDeviceAsUnmounted failed", markDeviceUnmountedErr)
-				"MarkDeviceAsUnmounted failed for device %q (spec.Name: %q) with: %v",
-				deviceToDetach.VolumeName,
-				deviceToDetach.VolumeSpec.Name(),
-				markDeviceUnmountedErr)
 		}
 
 		return nil
@@ -786,13 +613,7 @@ func (og *operationGenerator) GenerateVerifyControllerAttachedVolumeFunc(
 				volumeToMount.VolumeName, volumeToMount.VolumeSpec, nodeName, "" /* devicePath */)
 			if addVolumeNodeErr != nil {
 				// On failure, return error. Caller will log and retry.
-				return fmt.Errorf(
+				return volumeToMount.GenerateErrorDetailed("VerifyControllerAttachedVolume.MarkVolumeAsAttachedByUniqueVolumeName failed", addVolumeNodeErr)
-					"VerifyControllerAttachedVolume.MarkVolumeAsAttachedByUniqueVolumeName failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-					volumeToMount.VolumeName,
-					volumeToMount.VolumeSpec.Name(),
-					volumeToMount.PodName,
-					volumeToMount.Pod.UID,
-					addVolumeNodeErr)
 			}
 
 			return nil
@@ -805,67 +626,38 @@ func (og *operationGenerator) GenerateVerifyControllerAttachedVolumeFunc(
 			// periodically by kubelet, so it may take as much as 10 seconds
 			// before this clears.
 			// Issue #28141 to enable on demand status updates.
-			return fmt.Errorf("Volume %q (spec.Name: %q) pod %q (UID: %q) has not yet been added to the list of VolumesInUse in the node's volume status",
+			return volumeToMount.GenerateErrorDetailed("Volume has not been added to the list of VolumesInUse in the node's volume status", nil)
-				volumeToMount.VolumeName,
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID)
 		}
 
 		// Fetch current node object
 		node, fetchErr := og.kubeClient.Core().Nodes().Get(string(nodeName), metav1.GetOptions{})
 		if fetchErr != nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return volumeToMount.GenerateErrorDetailed("VerifyControllerAttachedVolume failed fetching node from API server", fetchErr)
-				"VerifyControllerAttachedVolume failed fetching node from API server. Volume %q (spec.Name: %q) pod %q (UID: %q). Error: %v",
-				volumeToMount.VolumeName,
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID,
-				fetchErr)
 		}
 
 		if node == nil {
 			// On failure, return error. Caller will log and retry.
-			return fmt.Errorf(
+			return volumeToMount.GenerateErrorDetailed(
-				"VerifyControllerAttachedVolume failed. Volume %q (spec.Name: %q) pod %q (UID: %q). Error: node object retrieved from API server is nil",
+				"VerifyControllerAttachedVolume failed",
-				volumeToMount.VolumeName,
+				fmt.Errorf("Node object retrieved from API server is nil"))
-				volumeToMount.VolumeSpec.Name(),
-				volumeToMount.PodName,
-				volumeToMount.Pod.UID)
 		}
 
 		for _, attachedVolume := range node.Status.VolumesAttached {
 			if attachedVolume.Name == volumeToMount.VolumeName {
 				addVolumeNodeErr := actualStateOfWorld.MarkVolumeAsAttached(
 					v1.UniqueVolumeName(""), volumeToMount.VolumeSpec, nodeName, attachedVolume.DevicePath)
-				glog.Infof("Controller successfully attached volume %q (spec.Name: %q) pod %q (UID: %q) devicePath: %q",
+				glog.Infof(volumeToMount.GenerateMsgDetailed("Controller attach succeeded", fmt.Sprintf("device path: %q", attachedVolume.DevicePath)))
-					volumeToMount.VolumeName,
-					volumeToMount.VolumeSpec.Name(),
-					volumeToMount.PodName,
-					volumeToMount.Pod.UID,
-					attachedVolume.DevicePath)
-
 				if addVolumeNodeErr != nil {
 					// On failure, return error. Caller will log and retry.
-					return fmt.Errorf(
+					return volumeToMount.GenerateErrorDetailed("VerifyControllerAttachedVolume.MarkVolumeAsAttached failed", addVolumeNodeErr)
-						"VerifyControllerAttachedVolume.MarkVolumeAsAttached failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
-						volumeToMount.VolumeName,
-						volumeToMount.VolumeSpec.Name(),
-						volumeToMount.PodName,
-						volumeToMount.Pod.UID,
-						addVolumeNodeErr)
 				}
 				return nil
 			}
 		}
 
 		// Volume not attached, return error. Caller will log and retry.
-		return fmt.Errorf("Volume %q (spec.Name: %q) pod %q (UID: %q) is not yet attached according to node status",
+		return volumeToMount.GenerateErrorDetailed("Volume not attached according to node status", nil)
-			volumeToMount.VolumeName,
-			volumeToMount.VolumeSpec.Name(),
-			volumeToMount.PodName,
-			volumeToMount.Pod.UID)
 	}, nil
 }
 
@@ -875,40 +667,31 @@ func (og *operationGenerator) verifyVolumeIsSafeToDetach(
 	node, fetchErr := og.kubeClient.Core().Nodes().Get(string(volumeToDetach.NodeName), metav1.GetOptions{})
 	if fetchErr != nil {
 		if errors.IsNotFound(fetchErr) {
-			glog.Warningf("Node %q not found on API server. DetachVolume will skip safe to detach check.",
+			glog.Warningf(volumeToDetach.GenerateMsgDetailed("Node not found on API server. DetachVolume will skip safe to detach check", ""))
-				volumeToDetach.NodeName,
-				volumeToDetach.VolumeName)
 			return nil
 		}
 
 		// On failure, return error. Caller will log and retry.
-		return fmt.Errorf(
+		return volumeToDetach.GenerateErrorDetailed("DetachVolume failed fetching node from API server", fetchErr)
-			"DetachVolume failed fetching node from API server for volume %q from node %q with: %v",
-			volumeToDetach.VolumeName,
-			volumeToDetach.NodeName,
-			fetchErr)
 	}
 
 	if node == nil {
 		// On failure, return error. Caller will log and retry.
-		return fmt.Errorf(
+		return volumeToDetach.GenerateErrorDetailed(
-			"DetachVolume failed fetching node from API server for volume %q from node %q. Error: node object retrieved from API server is nil",
+			"DetachVolume failed fetching node from API server",
-			volumeToDetach.VolumeName,
+			fmt.Errorf("node object retrieved from API server is nil"))
-			volumeToDetach.NodeName)
 	}
 
 	for _, inUseVolume := range node.Status.VolumesInUse {
 		if inUseVolume == volumeToDetach.VolumeName {
-			return fmt.Errorf("DetachVolume failed for volume %q from node %q. Error: volume is still in use by node, according to Node status",
+			return volumeToDetach.GenerateErrorDetailed(
-				volumeToDetach.VolumeName,
+				"DetachVolume failed",
-				volumeToDetach.NodeName)
+				fmt.Errorf("volume is still in use by node, according to Node status"))
 		}
 	}
 
 	// Volume is not marked as in use by node
-	glog.Infof("Verified volume is safe to detach for volume %q from node %q.",
+	glog.Infof(volumeToDetach.GenerateMsgDetailed("Verified volume is safe to detach", ""))
-		volumeToDetach.VolumeName,
-		volumeToDetach.NodeName)
 	return nil
 }
 
@@ -916,15 +699,9 @@ func checkMountOptionSupport(og *operationGenerator, volumeToMount VolumeToMount
 	mountOptions := volume.MountOptionFromSpec(volumeToMount.VolumeSpec)
 
 	if len(mountOptions) > 0 && !plugin.SupportsMountOption() {
-		err := fmt.Errorf(
+		eventErr, detailedErr := volumeToMount.GenerateError("Mount options are not supported for this volume type", nil)
-			"MountVolume.checkMountOptionSupport failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %q",
+		og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.UnsupportedMountOption, eventErr.Error())
-			volumeToMount.VolumeName,
+		return detailedErr
-			volumeToMount.VolumeSpec.Name(),
-			volumeToMount.PodName,
-			volumeToMount.Pod.UID,
-			"Mount options are not supported for this volume type")
-		og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.UnsupportedMountOption, err.Error())
-		return err
 	}
 	return nil
 }