From 734da0edb1e06209d48f229c71f5c3b9fdb11490 Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Wed, 31 Oct 2018 17:58:12 +0530 Subject: [PATCH] glusterfs: Allow admin to provide custom endpoint/service name prefix This patch introduces a new SC parameter called `customepnameprefix` in glusterfs plugin. This new storageclass parameter allow admins to create endpoints and services with mentioned prefix. If this parameter has not been set or if its empty, default prefix string ie "glusterfs-dynamic-" will be used for ep/svc name. This patch address below issues# https://github.com/kubernetes/kubernetes/issues/53939 https://github.com/kubernetes/kubernetes/issues/56379 Additional Ref# # https://github.com/kubernetes/kubernetes/issues/56380 Signed-off-by: Humble Chirammal --- pkg/volume/glusterfs/glusterfs.go | 31 ++++++++- pkg/volume/glusterfs/glusterfs_test.go | 88 ++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index 3223f58c2f..f5d2782111 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -67,7 +67,7 @@ var _ volume.Deleter = &glusterfsVolumeDeleter{} const ( glusterfsPluginName = "kubernetes.io/glusterfs" volPrefix = "vol_" - dynamicEpSvcPrefix = "glusterfs-dynamic-" + dynamicEpSvcPrefix = "glusterfs-dynamic" replicaCount = 3 durabilityType = "replicate" secretKeyName = "key" // key name used in secret @@ -75,6 +75,14 @@ const ( defaultGidMin = 2000 defaultGidMax = math.MaxInt32 + // maxCustomEpNamePrefix is the maximum number of chars. + // which can be used as ep/svc name prefix. This number is carved + // out from below formula. + // max length of name of an ep - length of pvc uuid + // where max length of name of an ep is 63 and length of uuid is 37 + + maxCustomEpNamePrefixLen = 26 + // absoluteGidMin/Max are currently the same as the // default values, but they play a different role and // could take a different value. Only thing we need is: @@ -443,6 +451,7 @@ type provisionerConfig struct { volumeOptions []string volumeNamePrefix string thinPoolSnapFactor float32 + customEpNamePrefix string } type glusterfsVolumeProvisioner struct { @@ -772,6 +781,8 @@ func (p *glusterfsVolumeProvisioner) Provision(selectedNode *v1.Node, allowedTop func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsVolumeSource, size int, volID string, err error) { var clusterIDs []string customVolumeName := "" + epServiceName := "" + capacity := p.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] // GlusterFS/heketi creates volumes in units of GiB. @@ -822,7 +833,11 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsVolum return nil, 0, "", fmt.Errorf("failed to get cluster nodes for volume %s: %v", volume, err) } - epServiceName := dynamicEpSvcPrefix + string(p.options.PVC.UID) + if len(p.provisionerConfig.customEpNamePrefix) == 0 { + epServiceName = string(p.options.PVC.UID) + } else { + epServiceName = p.provisionerConfig.customEpNamePrefix + "-" + string(p.options.PVC.UID) + } epNamespace := p.options.PVC.Namespace endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, dynamicHostIps, p.options.PVC.Name) if err != nil { @@ -970,6 +985,7 @@ func parseClassParameters(params map[string]string, kubeClient clientset.Interfa var err error cfg.gidMin = defaultGidMin cfg.gidMax = defaultGidMax + cfg.customEpNamePrefix = dynamicEpSvcPrefix authEnabled := true parseVolumeType := "" @@ -1037,7 +1053,16 @@ func parseClassParameters(params map[string]string, kubeClient clientset.Interfa if len(v) != 0 { parseThinPoolSnapFactor = v } - + case "customepnameprefix": + // If the string has > 'maxCustomEpNamePrefixLen' chars, the final endpoint name will + // exceed the limitation of 63 chars, so fail if prefix is > 'maxCustomEpNamePrefixLen' + // characters. This is only applicable for 'customepnameprefix' string and default ep name + // string will always pass. + if len(v) <= maxCustomEpNamePrefixLen { + cfg.customEpNamePrefix = v + } else { + return nil, fmt.Errorf("'customepnameprefix' value should be < %d characters", maxCustomEpNamePrefixLen) + } default: return nil, fmt.Errorf("invalid option %q for volume plugin %s", k, glusterfsPluginName) } diff --git a/pkg/volume/glusterfs/glusterfs_test.go b/pkg/volume/glusterfs/glusterfs_test.go index 2e8d0af83d..e2f13d3e71 100644 --- a/pkg/volume/glusterfs/glusterfs_test.go +++ b/pkg/volume/glusterfs/glusterfs_test.go @@ -261,6 +261,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 2147483647, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 3}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -283,6 +284,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 2147483647, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 3}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -299,6 +301,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 2147483647, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 3}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -437,6 +440,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 2147483647, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 3}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -454,6 +458,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 5000, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 3}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -472,6 +477,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 5000, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 3}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, @@ -492,6 +498,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 5000, volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 4}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, @@ -512,6 +519,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 5000, volumeType: gapi.VolumeDurabilityInfo{Type: "disperse", Replicate: gapi.ReplicaDurability{Replica: 0}, Disperse: gapi.DisperseDurability{Data: 4, Redundancy: 2}}, thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -532,6 +540,7 @@ func TestParseClassParameters(t *testing.T) { gidMax: 5000, volumeType: gapi.VolumeDurabilityInfo{Type: "disperse", Replicate: gapi.ReplicaDurability{Replica: 0}, Disperse: gapi.DisperseDurability{Data: 4, Redundancy: 2}}, thinPoolSnapFactor: float32(50), + customEpNamePrefix: "glusterfs-dynamic", }, }, @@ -555,6 +564,7 @@ func TestParseClassParameters(t *testing.T) { volumeType: gapi.VolumeDurabilityInfo{Type: "disperse", Replicate: gapi.ReplicaDurability{Replica: 0}, Disperse: gapi.DisperseDurability{Data: 4, Redundancy: 2}}, thinPoolSnapFactor: float32(50), volumeNamePrefix: "dept-dev", + customEpNamePrefix: "glusterfs-dynamic", }, }, { @@ -645,6 +655,84 @@ func TestParseClassParameters(t *testing.T) { true, // expect error nil, }, + + { + "enable custom ep/svc name: customEpNamePrefix: myprefix", + map[string]string{ + "resturl": "https://localhost:8080", + "restauthenabled": "false", + "gidMin": "4000", + "gidMax": "5000", + "volumetype": "replicate:4", + "customEpNamePrefix": "myprefix", + }, + &secret, + false, // expect error + &provisionerConfig{ + url: "https://localhost:8080", + gidMin: 4000, + gidMax: 5000, + volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 4}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, + thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "myprefix", + }, + }, + { + "empty custom ep/svc name: customEpNamePrefix:''", + map[string]string{ + "resturl": "https://localhost:8080", + "restauthenabled": "false", + "gidMin": "4000", + "gidMax": "5000", + "volumetype": "replicate:4", + "customEpNamePrefix": "", + }, + &secret, + false, // expect error + &provisionerConfig{ + url: "https://localhost:8080", + gidMin: 4000, + gidMax: 5000, + volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 4}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, + thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "", + }, + }, + { + "custom ep/svc name with 26 chars: customEpNamePrefix:'charstringhastwentysixchar'", + map[string]string{ + "resturl": "https://localhost:8080", + "restauthenabled": "false", + "gidMin": "4000", + "gidMax": "5000", + "volumetype": "replicate:4", + "customEpNamePrefix": "charstringhastwentysixchar", + }, + &secret, + false, // expect error + &provisionerConfig{ + url: "https://localhost:8080", + gidMin: 4000, + gidMax: 5000, + volumeType: gapi.VolumeDurabilityInfo{Type: "replicate", Replicate: gapi.ReplicaDurability{Replica: 4}, Disperse: gapi.DisperseDurability{Data: 0, Redundancy: 0}}, + thinPoolSnapFactor: float32(1.0), + customEpNamePrefix: "charstringhastwentysixchar", + }, + }, + { + "invalid customepnameprefix ( ie >26 chars) parameter", + map[string]string{ + "resturl": "https://localhost:8080", + "restauthenabled": "false", + "gidMin": "4000", + "gidMax": "5000", + "volumetype": "replicate:4", + "customEpNamePrefix": "myprefixhasmorethan26characters", + }, + &secret, + true, // expect error + nil, + }, } for _, test := range tests {