diff --git a/pkg/volume/util/operationexecutor/operation_executor.go b/pkg/volume/util/operationexecutor/operation_executor.go index f8ae260244a..3225cacd44d 100644 --- a/pkg/volume/util/operationexecutor/operation_executor.go +++ b/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 " ", + // 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 " : ", + 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 " ", + // 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 " : ", + GenerateError(prefixMsg string, err error) (simpleErr, detailedErr error) +} + +// Generates an error string with the format ": " 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) diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 8b2b0f17f98..7fa2d71cc85 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/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( - "VolumesAreAttached.FindPluginBySpec failed for volume %q (spec.Name: %q) on node %q with error: %v", - volumeAttached.VolumeName, - volumeAttached.VolumeSpec.Name(), - volumeAttached.NodeName, - err) + glog.Errorf(volumeAttached.GenerateErrorDetailed("VolumesAreAttached.FindPluginBySpec failed", err).Error()) } 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( - "AttachVolume.FindAttachablePluginBySpec failed for volume %q (spec.Name: %q) from node %q with: %v", - volumeToAttach.VolumeName, - volumeToAttach.VolumeSpec.Name(), - volumeToAttach.NodeName, - err) + return nil, volumeToAttach.GenerateErrorDetailed("AttachVolume.FindAttachablePluginBySpec failed", err) } volumeAttacher, newAttacherErr := attachableVolumePlugin.NewAttacher() if newAttacherErr != nil { - return nil, fmt.Errorf( - "AttachVolume.NewAttacher failed for volume %q (spec.Name: %q) from node %q with: %v", - volumeToAttach.VolumeName, - volumeToAttach.VolumeSpec.Name(), - volumeToAttach.NodeName, - newAttacherErr) + return nil, volumeToAttach.GenerateErrorDetailed("AttachVolume.NewAttacher failed", 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( - "Failed to attach volume %q on node %q with: %v", - volumeToAttach.VolumeSpec.Name(), - volumeToAttach.NodeName, - attachErr) + eventErr, detailedErr := volumeToAttach.GenerateError("AttachVolume.Attach failed", 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( - "AttachVolume.Attach succeeded for volume %q (spec.Name: %q) from node %q.", - volumeToAttach.VolumeName, - volumeToAttach.VolumeSpec.Name(), - volumeToAttach.NodeName) + glog.Infof(volumeToAttach.GenerateMsgDetailed("AttachVolume.Attach succeeded", "")) // 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( - "AttachVolume.MarkVolumeAsAttached failed for volume %q (spec.Name: %q) from node %q with: %v", - volumeToAttach.VolumeName, - volumeToAttach.VolumeSpec.Name(), - volumeToAttach.NodeName, - addVolumeNodeErr) + return volumeToAttach.GenerateErrorDetailed("AttachVolume.MarkVolumeAsAttached failed", 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( - "DetachVolume.FindAttachablePluginBySpec failed for volume %q (spec.Name: %q) from node %q with: %v", - volumeToDetach.VolumeName, - volumeToDetach.VolumeSpec.Name(), - volumeToDetach.NodeName, - err) + return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.FindAttachablePluginBySpec failed", 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( - "Failed to find plugin for volume %q from node %q with: %v", - volumeToDetach.VolumeName, - volumeToDetach.NodeName, - err) + return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.FindAttachablePluginBySpec failed", err) } } volumeDetacher, err := attachableVolumePlugin.NewDetacher() if err != nil { - return nil, fmt.Errorf( - "DetachVolume.NewDetacher failed for volume %q from node %q with: %v", - volumeToDetach.VolumeName, - volumeToDetach.NodeName, - err) + return nil, volumeToDetach.GenerateErrorDetailed("DetachVolume.NewDetacher failed", 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( - "DetachVolume.Detach failed for volume %q from node %q with: %v", - volumeToDetach.VolumeName, - volumeToDetach.NodeName, - err) + return volumeToDetach.GenerateErrorDetailed("DetachVolume.Detach failed", err) } - glog.Infof( - "DetachVolume.Detach succeeded for volume %q from node %q.", - volumeToDetach.VolumeName, - volumeToDetach.NodeName) + glog.Infof(volumeToDetach.GenerateMsgDetailed("DetachVolume.Detach succeeded", "")) // 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( - "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) + return nil, volumeToMount.GenerateErrorDetailed("MountVolume.FindPluginBySpec failed", err) } volumeMounter, newMounterErr := volumePlugin.NewMounter( @@ -415,15 +364,9 @@ func (og *operationGenerator) GenerateMountVolumeFunc( volumeToMount.Pod, volume.VolumeOptions{}) if newMounterErr != nil { - errMsg := fmt.Errorf( - "MountVolume.NewMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID, - newMounterErr) - og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, errMsg.Error()) - return nil, errMsg + eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.NewMounter initialization failed", newMounterErr) + og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error()) + return nil, detailedErr } 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( - "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) + glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach entering", fmt.Sprintf("DevicePath %q", 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( - "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) + return volumeToMount.GenerateErrorDetailed("MountVolume.WaitForAttach failed", err) } - glog.Infof( - "MountVolume.WaitForAttach succeeded for volume %q (spec.Name: %q) pod %q (UID: %q).", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID) + glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach succeeded", "")) deviceMountPath, err := volumeAttacher.GetDeviceMountPath(volumeToMount.VolumeSpec) if err != nil { // On failure, return error. Caller will log and retry. - return fmt.Errorf( - "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) + return volumeToMount.GenerateErrorDetailed("MountVolume.GetDeviceMountPath failed", 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( - "MountVolume.MountDevice failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID, - err) - og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, err.Error()) - return err + eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.MountDevice failed", err) + og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error()) + return detailedErr } - glog.Infof( - "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) + glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.MountDevice succeeded", fmt.Sprintf("device mount path %q", 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( - "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) + return volumeToMount.GenerateErrorDetailed("MountVolume.MarkDeviceAsMounted failed", 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()) - og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, errMsg) - glog.Errorf(errMsg) - return fmt.Errorf(errMsg) + err = fmt.Errorf( + "Verify that your node machine has the required components before attempting to mount this volume type. %s", + canMountErr) + 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( - "MountVolume.SetUp failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID, - mountErr) - og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, err.Error()) - return err + eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.SetUp failed", mountErr) + og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.FailedMountVolume, eventErr.Error()) + return detailedErr } - glog.Infof( - "MountVolume.SetUp succeeded for volume %q (spec.Name: %q) pod %q (UID: %q).", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID) + glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.SetUp succeeded", "")) // 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( - "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 volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeAsMounted failed", 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( - "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) + return nil, volumeToUnmount.GenerateErrorDetailed("UnmountVolume.FindPluginByName failed", err) } volumeUnmounter, newUnmounterErr := volumePlugin.NewUnmounter( volumeToUnmount.InnerVolumeSpecName, volumeToUnmount.PodUID) if newUnmounterErr != nil { - return nil, fmt.Errorf( - "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 nil, volumeToUnmount.GenerateErrorDetailed("UnmountVolume.NewUnmounter failed", 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( - "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) + return volumeToUnmount.GenerateErrorDetailed("UnmountVolume.TearDown failed", 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( - "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) + glog.Errorf(volumeToUnmount.GenerateErrorDetailed("UnmountVolume.MarkVolumeAsUnmounted failed", markVolMountedErr).Error()) } 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( - "UnmountDevice.FindAttachablePluginBySpec failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - err) + return nil, deviceToDetach.GenerateErrorDetailed("UnmountDevice.FindAttachablePluginBySpec failed", err) } volumeDetacher, err := attachableVolumePlugin.NewDetacher() if err != nil { - return nil, fmt.Errorf( - "UnmountDevice.NewDetacher failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - err) + return nil, deviceToDetach.GenerateErrorDetailed("UnmountDevice.NewDetacher failed", err) } volumeAttacher, err := attachableVolumePlugin.NewAttacher() if err != nil { - return nil, fmt.Errorf( - "UnmountDevice.NewAttacher failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - err) + return nil, deviceToDetach.GenerateErrorDetailed("UnmountDevice.NewAttacher failed", 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( - "GetDeviceMountPath failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - err) + return deviceToDetach.GenerateErrorDetailed("GetDeviceMountPath failed", 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( - "GetDeviceMountRefs check failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - err) + return deviceToDetach.GenerateErrorDetailed("GetDeviceMountRefs check failed", err) } // Execute unmount unmountDeviceErr := volumeDetacher.UnmountDevice(deviceMountPath) if unmountDeviceErr != nil { // On failure, return error. Caller will log and retry. - return fmt.Errorf( - "UnmountDevice failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - unmountDeviceErr) + return deviceToDetach.GenerateErrorDetailed("UnmountDevice failed", 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( - "UnmountDevice.DeviceOpened failed for volume %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - deviceOpenedErr) + return deviceToDetach.GenerateErrorDetailed("UnmountDevice.DeviceOpened failed", deviceOpenedErr) } } // The device is still in use elsewhere. Caller will log and retry. if deviceOpened { - return fmt.Errorf( - "UnmountDevice failed for volume %q (spec.Name: %q) because the device is in use when it was no longer expected to be in use", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name()) + return deviceToDetach.GenerateErrorDetailed( + "UnmountDevice failed", + fmt.Errorf("the device is in use when it was no longer expected to be in use")) } - glog.Infof( - "UnmountDevice succeeded for volume %q (spec.Name: %q).", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name()) + glog.Infof(deviceToDetach.GenerateMsgDetailed("UnmountDevice succeeded", "")) // 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( - "MarkDeviceAsUnmounted failed for device %q (spec.Name: %q) with: %v", - deviceToDetach.VolumeName, - deviceToDetach.VolumeSpec.Name(), - markDeviceUnmountedErr) + return deviceToDetach.GenerateErrorDetailed("MarkDeviceAsUnmounted failed", 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( - "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 volumeToMount.GenerateErrorDetailed("VerifyControllerAttachedVolume.MarkVolumeAsAttachedByUniqueVolumeName failed", 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", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID) + return volumeToMount.GenerateErrorDetailed("Volume has not been added to the list of VolumesInUse in the node's volume status", nil) } // 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( - "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) + return volumeToMount.GenerateErrorDetailed("VerifyControllerAttachedVolume failed fetching node from API server", fetchErr) } if node == nil { // On failure, return error. Caller will log and retry. - return fmt.Errorf( - "VerifyControllerAttachedVolume failed. Volume %q (spec.Name: %q) pod %q (UID: %q). Error: node object retrieved from API server is nil", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID) + return volumeToMount.GenerateErrorDetailed( + "VerifyControllerAttachedVolume failed", + fmt.Errorf("Node object retrieved from API server is nil")) } 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", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID, - attachedVolume.DevicePath) - + glog.Infof(volumeToMount.GenerateMsgDetailed("Controller attach succeeded", fmt.Sprintf("device path: %q", attachedVolume.DevicePath))) if addVolumeNodeErr != nil { // On failure, return error. Caller will log and retry. - return fmt.Errorf( - "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 volumeToMount.GenerateErrorDetailed("VerifyControllerAttachedVolume.MarkVolumeAsAttached failed", 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", - volumeToMount.VolumeName, - volumeToMount.VolumeSpec.Name(), - volumeToMount.PodName, - volumeToMount.Pod.UID) + return volumeToMount.GenerateErrorDetailed("Volume not attached according to node status", nil) }, 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.", - volumeToDetach.NodeName, - volumeToDetach.VolumeName) + glog.Warningf(volumeToDetach.GenerateMsgDetailed("Node not found on API server. DetachVolume will skip safe to detach check", "")) return nil } // On failure, return error. Caller will log and retry. - return fmt.Errorf( - "DetachVolume failed fetching node from API server for volume %q from node %q with: %v", - volumeToDetach.VolumeName, - volumeToDetach.NodeName, - fetchErr) + return volumeToDetach.GenerateErrorDetailed("DetachVolume failed fetching node from API server", fetchErr) } if node == nil { // On failure, return error. Caller will log and retry. - return fmt.Errorf( - "DetachVolume failed fetching node from API server for volume %q from node %q. Error: node object retrieved from API server is nil", - volumeToDetach.VolumeName, - volumeToDetach.NodeName) + return volumeToDetach.GenerateErrorDetailed( + "DetachVolume failed fetching node from API server", + fmt.Errorf("node object retrieved from API server is nil")) } 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", - volumeToDetach.VolumeName, - volumeToDetach.NodeName) + return volumeToDetach.GenerateErrorDetailed( + "DetachVolume failed", + 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.", - volumeToDetach.VolumeName, - volumeToDetach.NodeName) + glog.Infof(volumeToDetach.GenerateMsgDetailed("Verified volume is safe to detach", "")) 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( - "MountVolume.checkMountOptionSupport failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %q", - volumeToMount.VolumeName, - 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 + eventErr, detailedErr := volumeToMount.GenerateError("Mount options are not supported for this volume type", nil) + og.recorder.Eventf(volumeToMount.Pod, v1.EventTypeWarning, kevents.UnsupportedMountOption, eventErr.Error()) + return detailedErr } return nil }