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

Shuffle addresslist for random mount server and cleanup error messages. #76983

Merged
merged 1 commit into from
May 3, 2019
Merged

Shuffle addresslist for random mount server and cleanup error messages. #76983

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
25 changes: 13 additions & 12 deletions pkg/volume/glusterfs/glusterfs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package glusterfs
import (
"fmt"
"math"
"math/rand"
"os"
"path"
"runtime"
Expand Down Expand Up @@ -186,13 +187,13 @@ func (plugin *glusterfsPlugin) getEndpointNameAndNamespace(spec *volume.Spec, de
}
return endpoints, endpointsNs, nil
}
return "", "", fmt.Errorf("Spec does not reference a GlusterFS volume type")
return "", "", fmt.Errorf("spec does not reference a GlusterFS volume type")

}
func (plugin *glusterfsPlugin) newMounterInternal(spec *volume.Spec, ep *v1.Endpoints, pod *v1.Pod, mounter mount.Interface) (volume.Mounter, error) {
volPath, readOnly, err := getVolumeInfo(spec)
if err != nil {
klog.Errorf("failed to get volumesource : %v", err)
klog.Errorf("failed to get volumesource: %v", err)
return nil, err
}
return &glusterfsMounter{
Expand Down Expand Up @@ -267,7 +268,7 @@ func (b *glusterfsMounter) CanMount() error {
switch runtime.GOOS {
case "linux":
if _, err := exe.Run("test", "-x", gciLinuxGlusterMountBinaryPath); err != nil {
return fmt.Errorf("Required binary %s is missing", gciLinuxGlusterMountBinaryPath)
return fmt.Errorf("required binary %s is missing", gciLinuxGlusterMountBinaryPath)
}
}
return nil
Expand Down Expand Up @@ -394,7 +395,7 @@ func (b *glusterfsMounter) setUpAtInternal(dir string) error {
// Refer to backup-volfile-servers @ http://docs.gluster.org/en/latest/Administrator%20Guide/Setting%20Up%20Clients/

if (len(addrlist) > 0) && (addrlist[0] != "") {
ip := addrlist[0]
ip := addrlist[rand.Intn(len(addrlist))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it interact with backup-volfile-servers? The comment above suggests that it does not really matter which address is used as mount source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsafrane True.. The effect on backup-volfile-servers is negligible, the reason being backup-volfile-servers used as a list/iteration to try out if one/attempted mount server is not available. Without randomization most of the mount happens on single/same server ( as its available mostly) and this patch should avoid such huge number of mounts on the same server.

errs = b.mounter.Mount(ip+":"+b.path, dir, "glusterfs", mountOptions)
if errs == nil {
klog.Infof("successfully mounted directory %s", dir)
Expand Down Expand Up @@ -427,7 +428,7 @@ func (b *glusterfsMounter) setUpAtInternal(dir string) error {
// it all goes in a log file, we will read the log file
logErr := readGlusterLog(log, b.pod.Name)
if logErr != nil {
return fmt.Errorf("mount failed: %v the following error information was pulled from the glusterfs log to help diagnose this issue: %v", errs, logErr)
return fmt.Errorf("mount failed: %v, the following error information was pulled from the glusterfs log to help diagnose this issue: %v", errs, logErr)
}
return fmt.Errorf("mount failed: %v", errs)

Expand All @@ -442,7 +443,7 @@ func getVolumeInfo(spec *volume.Spec) (string, bool, error) {
spec.PersistentVolume.Spec.Glusterfs != nil {
return spec.PersistentVolume.Spec.Glusterfs.Path, spec.ReadOnly, nil
}
return "", false, fmt.Errorf("Spec does not reference a Glusterfs volume type")
return "", false, fmt.Errorf("spec does not reference a Glusterfs volume type")
}

func (plugin *glusterfsPlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) {
Expand Down Expand Up @@ -743,7 +744,7 @@ func (p *glusterfsVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTop
return nil, fmt.Errorf("%s does not support block volume provisioning", p.plugin.GetPluginName())
}

klog.V(4).Infof("Provision VolumeOptions %v", p.options)
klog.V(4).Infof("provision volume with options %v", p.options)
scName := v1helper.GetPersistentVolumeClaimClass(p.options.PVC)
cfg, err := parseClassParameters(p.options.Parameters, p.plugin.host.GetKubeClient())
if err != nil {
Expand All @@ -762,7 +763,7 @@ func (p *glusterfsVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTop
return nil, fmt.Errorf("failed to reserve GID from table: %v", err)
}

klog.V(2).Infof("Allocated GID %d for PVC %s", gid, p.options.PVC.Name)
klog.V(2).Infof("allocated GID %d for PVC %s", gid, p.options.PVC.Name)

glusterfs, sizeGiB, volID, err := p.CreateVolume(gid)
if err != nil {
Expand Down Expand Up @@ -1162,21 +1163,21 @@ func parseClassParameters(params map[string]string, kubeClient clientset.Interfa
}

if cfg.gidMin > cfg.gidMax {
return nil, fmt.Errorf("StorageClass for provisioner %q must have gidMax value >= gidMin", glusterfsPluginName)
return nil, fmt.Errorf("storageClass for provisioner %q must have gidMax value >= gidMin", glusterfsPluginName)
}

if len(parseVolumeOptions) != 0 {
volOptions := dstrings.Split(parseVolumeOptions, ",")
if len(volOptions) == 0 {
return nil, fmt.Errorf("StorageClass for provisioner %q must have valid (for e.g., 'client.ssl on') volume option", glusterfsPluginName)
return nil, fmt.Errorf("storageClass for provisioner %q must have valid (for e.g., 'client.ssl on') volume option", glusterfsPluginName)
}
cfg.volumeOptions = volOptions

}

if len(parseVolumeNamePrefix) != 0 {
if dstrings.Contains(parseVolumeNamePrefix, "_") {
return nil, fmt.Errorf("Storageclass parameter 'volumenameprefix' should not contain '_' in its value")
return nil, fmt.Errorf("storageclass parameter 'volumenameprefix' should not contain '_' in its value")
}
cfg.volumeNamePrefix = parseVolumeNamePrefix
}
Expand Down Expand Up @@ -1217,7 +1218,7 @@ func getVolumeID(pv *v1.PersistentVolume, volumeName string) (string, error) {
func (plugin *glusterfsPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize resource.Quantity, oldSize resource.Quantity) (resource.Quantity, error) {
pvSpec := spec.PersistentVolume.Spec
volumeName := pvSpec.Glusterfs.Path
klog.V(2).Infof("Received request to expand volume %s", volumeName)
klog.V(2).Infof("received request to expand volume %s", volumeName)
volumeID, err := getVolumeID(spec.PersistentVolume, volumeName)

if err != nil {
Expand Down