Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor volume operation log and error messages #44969

Merged
merged 1 commit into from
May 5, 2017
Merged

Refactor volume operation log and error messages #44969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
160 changes: 159 additions & 1 deletion pkg/volume/util/operationexecutor/operation_executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ limitations under the License.
package operationexecutor

import (
"fmt"
"strings"
"time"

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down