From 1d0b1c227b85a836988bedf15fd3f12c1e372a3f Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Fri, 12 Feb 2016 09:46:59 +0100 Subject: [PATCH] Add PV.Name into names of generated GCE/AWS/OSP volumes. Volume names have now format -dynamic-. pv-name is guaranteed to be unique in Kubernetes cluster, adding ensures we don't conflict with any running cluster in the cloud project (kube-controller-manager --cluster-name=XXX). 'kubernetes' is the default cluster name. --- .../app/controllermanager.go | 2 +- .../controllermanager/controllermanager.go | 2 +- .../providers/openstack/openstack.go | 7 ++++-- .../providers/openstack/openstack_test.go | 4 ++- ...persistentvolume_provisioner_controller.go | 15 ++++++++--- ...stentvolume_provisioner_controller_test.go | 2 +- pkg/volume/aws_ebs/aws_util.go | 20 +++++++++++---- pkg/volume/cinder/cinder_util.go | 3 ++- pkg/volume/gce_pd/gce_util.go | 3 +-- pkg/volume/plugins.go | 4 +++ pkg/volume/util.go | 17 +++++++++++++ pkg/volume/util_test.go | 25 +++++++++++++++++++ 12 files changed, 86 insertions(+), 18 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index d427186a2c9..7c0963672d1 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -312,7 +312,7 @@ func StartControllers(s *options.CMServer, kubeClient *client.Client, kubeconfig pvRecycler.Run() if provisioner != nil { - pvController, err := persistentvolumecontroller.NewPersistentVolumeProvisionerController(persistentvolumecontroller.NewControllerClient(clientset.NewForConfigOrDie(client.AddUserAgent(kubeconfig, "persistent-volume-provisioner"))), s.PVClaimBinderSyncPeriod.Duration, volumePlugins, provisioner, cloud) + pvController, err := persistentvolumecontroller.NewPersistentVolumeProvisionerController(persistentvolumecontroller.NewControllerClient(clientset.NewForConfigOrDie(client.AddUserAgent(kubeconfig, "persistent-volume-provisioner"))), s.PVClaimBinderSyncPeriod.Duration, s.ClusterName, volumePlugins, provisioner, cloud) if err != nil { glog.Fatalf("Failed to start persistent volume provisioner controller: %+v", err) } diff --git a/contrib/mesos/pkg/controllermanager/controllermanager.go b/contrib/mesos/pkg/controllermanager/controllermanager.go index 30d766904b7..2120d9dd71a 100644 --- a/contrib/mesos/pkg/controllermanager/controllermanager.go +++ b/contrib/mesos/pkg/controllermanager/controllermanager.go @@ -268,7 +268,7 @@ func (s *CMServer) Run(_ []string) error { pvRecycler.Run() if provisioner != nil { - pvController, err := persistentvolumecontroller.NewPersistentVolumeProvisionerController(persistentvolumecontroller.NewControllerClient(clientset.NewForConfigOrDie(client.AddUserAgent(kubeconfig, "persistent-volume-controller"))), s.PVClaimBinderSyncPeriod.Duration, volumePlugins, provisioner, cloud) + pvController, err := persistentvolumecontroller.NewPersistentVolumeProvisionerController(persistentvolumecontroller.NewControllerClient(clientset.NewForConfigOrDie(client.AddUserAgent(kubeconfig, "persistent-volume-controller"))), s.PVClaimBinderSyncPeriod.Duration, s.ClusterName, volumePlugins, provisioner, cloud) if err != nil { glog.Fatalf("Failed to start persistent volume provisioner controller: %+v", err) } diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index e0e13edf2ac..86ff39addab 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -1038,7 +1038,7 @@ func (os *OpenStack) getVolume(diskName string) (volumes.Volume, error) { } // Create a volume of given size (in GiB) -func (os *OpenStack) CreateVolume(size int, tags *map[string]string) (volumeName string, err error) { +func (os *OpenStack) CreateVolume(name string, size int, tags *map[string]string) (volumeName string, err error) { sClient, err := openstack.NewBlockStorageV1(os.provider, gophercloud.EndpointOpts{ Region: os.region, @@ -1049,7 +1049,10 @@ func (os *OpenStack) CreateVolume(size int, tags *map[string]string) (volumeName return "", err } - opts := volumes.CreateOpts{Size: size} + opts := volumes.CreateOpts{ + Name: name, + Size: size, + } if tags != nil { opts.Metadata = *tags } diff --git a/pkg/cloudprovider/providers/openstack/openstack_test.go b/pkg/cloudprovider/providers/openstack/openstack_test.go index 40b485cc0ae..a4db2fafb3c 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_test.go +++ b/pkg/cloudprovider/providers/openstack/openstack_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + "k8s.io/kubernetes/pkg/util/rand" + "github.com/rackspace/gophercloud" ) @@ -213,7 +215,7 @@ func TestVolumes(t *testing.T) { tags := map[string]string{ "test": "value", } - vol, err := os.CreateVolume(1, &tags) + vol, err := os.CreateVolume("kubernetes-test-volume-"+rand.String(10), 1, &tags) if err != nil { t.Fatalf("Cannot create a new Cinder volume: %v", err) } diff --git a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go b/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go index ed50edf512e..2c916ff5221 100644 --- a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go +++ b/pkg/controller/persistentvolume/persistentvolume_provisioner_controller.go @@ -49,6 +49,7 @@ type PersistentVolumeProvisionerController struct { pluginMgr volume.VolumePluginMgr stopChannels map[string]chan struct{} mutex sync.RWMutex + clusterName string } // constant name values for the controllers stopChannels map. @@ -57,11 +58,12 @@ const volumesStopChannel = "volumes" const claimsStopChannel = "claims" // NewPersistentVolumeProvisionerController creates a new PersistentVolumeProvisionerController -func NewPersistentVolumeProvisionerController(client controllerClient, syncPeriod time.Duration, plugins []volume.VolumePlugin, provisioner volume.ProvisionableVolumePlugin, cloud cloudprovider.Interface) (*PersistentVolumeProvisionerController, error) { +func NewPersistentVolumeProvisionerController(client controllerClient, syncPeriod time.Duration, clusterName string, plugins []volume.VolumePlugin, provisioner volume.ProvisionableVolumePlugin, cloud cloudprovider.Interface) (*PersistentVolumeProvisionerController, error) { controller := &PersistentVolumeProvisionerController{ client: client, cloud: cloud, provisioner: provisioner, + clusterName: clusterName, } if err := controller.pluginMgr.InitPlugins(plugins, controller); err != nil { @@ -172,7 +174,7 @@ func (controller *PersistentVolumeProvisionerController) reconcileClaim(claim *a } glog.V(5).Infof("PersistentVolumeClaim[%s] provisioning", claim.Name) - provisioner, err := newProvisioner(controller.provisioner, claim, nil) + provisioner, err := controller.newProvisioner(controller.provisioner, claim, nil) if err != nil { return fmt.Errorf("Unexpected error getting new provisioner for claim %s: %v\n", claim.Name, err) } @@ -274,7 +276,7 @@ func provisionVolume(pv *api.PersistentVolume, controller *PersistentVolumeProvi } claim := obj.(*api.PersistentVolumeClaim) - provisioner, _ := newProvisioner(controller.provisioner, claim, pv) + provisioner, _ := controller.newProvisioner(controller.provisioner, claim, pv) err := provisioner.Provision(pv) if err != nil { glog.Errorf("Could not provision %s", pv.Name) @@ -330,7 +332,7 @@ func (controller *PersistentVolumeProvisionerController) Stop() { } } -func newProvisioner(plugin volume.ProvisionableVolumePlugin, claim *api.PersistentVolumeClaim, pv *api.PersistentVolume) (volume.Provisioner, error) { +func (controller *PersistentVolumeProvisionerController) newProvisioner(plugin volume.ProvisionableVolumePlugin, claim *api.PersistentVolumeClaim, pv *api.PersistentVolume) (volume.Provisioner, error) { tags := make(map[string]string) tags[cloudVolumeCreatedForClaimNamespaceTag] = claim.Namespace tags[cloudVolumeCreatedForClaimNameTag] = claim.Name @@ -345,6 +347,11 @@ func newProvisioner(plugin volume.ProvisionableVolumePlugin, claim *api.Persiste AccessModes: claim.Spec.AccessModes, PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete, CloudTags: &tags, + ClusterName: controller.clusterName, + } + + if pv != nil { + volumeOptions.PVName = pv.Name } provisioner, err := plugin.NewProvisioner(volumeOptions) diff --git a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go b/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go index 2b9661ee34a..6229b23ed17 100644 --- a/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go +++ b/pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go @@ -95,7 +95,7 @@ func makeTestClaim() *api.PersistentVolumeClaim { func makeTestController() (*PersistentVolumeProvisionerController, *mockControllerClient, *volume.FakeVolumePlugin) { mockClient := &mockControllerClient{} mockVolumePlugin := &volume.FakeVolumePlugin{} - controller, _ := NewPersistentVolumeProvisionerController(mockClient, 1*time.Second, nil, mockVolumePlugin, &fake_cloud.FakeCloud{}) + controller, _ := NewPersistentVolumeProvisionerController(mockClient, 1*time.Second, "fake-kubernetes", nil, mockVolumePlugin, &fake_cloud.FakeCloud{}) return controller, mockClient, mockVolumePlugin } diff --git a/pkg/volume/aws_ebs/aws_util.go b/pkg/volume/aws_ebs/aws_util.go index 083842db63f..998aa25987f 100644 --- a/pkg/volume/aws_ebs/aws_util.go +++ b/pkg/volume/aws_ebs/aws_util.go @@ -132,12 +132,22 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (volum return "", 0, err } - requestBytes := c.options.Capacity.Value() - // The cloud provider works with gigabytes, convert to GiB with rounding up - requestGB := volume.RoundUpSize(requestBytes, 1024*1024*1024) + // AWS volumes don't have Name field, store the name in Name tag + var tags map[string]string + if c.options.CloudTags == nil { + tags = make(map[string]string) + } else { + tags = *c.options.CloudTags + } + tags["Name"] = volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 255) // AWS tags can have 255 characters - volumeOptions := &aws.VolumeOptions{} - volumeOptions.CapacityGB = int(requestGB) + requestBytes := c.options.Capacity.Value() + // AWS works with gigabytes, convert to GiB with rounding up + requestGB := int(volume.RoundUpSize(requestBytes, 1024*1024*1024)) + volumeOptions := &aws.VolumeOptions{ + CapacityGB: requestGB, + Tags: &tags, + } name, err := cloud.CreateDisk(volumeOptions) if err != nil { diff --git a/pkg/volume/cinder/cinder_util.go b/pkg/volume/cinder/cinder_util.go index dd5d3d743b3..339515ae40d 100644 --- a/pkg/volume/cinder/cinder_util.go +++ b/pkg/volume/cinder/cinder_util.go @@ -150,7 +150,8 @@ func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID s volSizeBytes := c.options.Capacity.Value() // Cinder works with gigabytes, convert to GiB with rounding up volSizeGB := int(volume.RoundUpSize(volSizeBytes, 1024*1024*1024)) - name, err := cloud.CreateVolume(volSizeGB, c.options.CloudTags) + name := volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 255) // Cinder volume name can have up to 255 characters + name, err = cloud.CreateVolume(name, volSizeGB, c.options.CloudTags) if err != nil { glog.V(2).Infof("Error creating cinder volume: %v", err) return "", 0, err diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go index 36719ab92ee..b190aa27b28 100644 --- a/pkg/volume/gce_pd/gce_util.go +++ b/pkg/volume/gce_pd/gce_util.go @@ -27,7 +27,6 @@ import ( "github.com/golang/glog" "k8s.io/kubernetes/pkg/cloudprovider" gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" - "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/exec" "k8s.io/kubernetes/pkg/util/keymutex" "k8s.io/kubernetes/pkg/util/runtime" @@ -134,7 +133,7 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (volum return "", 0, err } - name := fmt.Sprintf("kube-dynamic-%s", util.NewUUID()) + name := volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 63) // GCE PD name can have up to 63 characters requestBytes := c.options.Capacity.Value() // GCE works with gigabytes, convert to GiB with rounding up requestGB := volume.RoundUpSize(requestBytes, 1024*1024*1024) diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 21ef5a9cbb1..3e9074b3e1b 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -49,6 +49,10 @@ type VolumeOptions struct { AccessModes []api.PersistentVolumeAccessMode // Reclamation policy for a persistent volume PersistentVolumeReclaimPolicy api.PersistentVolumeReclaimPolicy + // PV.Name of the appropriate PersistentVolume. Used to generate cloud volume name. + PVName string + // Unique name of Kubernetes cluster. + ClusterName string // Tags to attach to the real volume in the cloud provider - e.g. AWS EBS CloudTags *map[string]string } diff --git a/pkg/volume/util.go b/pkg/volume/util.go index 544dfbf9da4..8df127cd531 100644 --- a/pkg/volume/util.go +++ b/pkg/volume/util.go @@ -148,3 +148,20 @@ func CalculateTimeoutForVolume(minimumTimeout, timeoutIncrement int, pv *api.Per func RoundUpSize(volumeSizeBytes int64, allocationUnitBytes int64) int64 { return (volumeSizeBytes + allocationUnitBytes - 1) / allocationUnitBytes } + +// GenerateVolumeName returns a PV name with clusterName prefix. +// The function should be used to generate a name of GCE PD or Cinder volume. +// It basically adds "-dynamic-" before the PV name, +// making sure the resulting string fits given length and cuts "dynamic" +// if not. +func GenerateVolumeName(clusterName, pvName string, maxLength int) string { + prefix := clusterName + "-dynamic" + pvLen := len(pvName) + + // cut the "-dynamic" to fit full pvName into maxLength + // +1 for the '-' dash + if pvLen+1+len(prefix) > maxLength { + prefix = prefix[:maxLength-pvLen-1] + } + return prefix + "-" + pvName +} diff --git a/pkg/volume/util_test.go b/pkg/volume/util_test.go index 5eefb8cc851..faccd021ad2 100644 --- a/pkg/volume/util_test.go +++ b/pkg/volume/util_test.go @@ -128,3 +128,28 @@ func TestCalculateTimeoutForVolume(t *testing.T) { t.Errorf("Expected 4500 for timeout but got %v", timeout) } } + +func TestGenerateVolumeName(t *testing.T) { + + // Normal operation, no truncate + v1 := GenerateVolumeName("kubernetes", "pv-cinder-abcde", 255) + if v1 != "kubernetes-dynamic-pv-cinder-abcde" { + t.Errorf("Expected kubernetes-dynamic-pv-cinder-abcde, got %s", v1) + } + + // Truncate trailing "6789-dynamic" + prefix := strings.Repeat("0123456789", 9) // 90 characters prefix + 8 chars. of "-dynamic" + v2 := GenerateVolumeName(prefix, "pv-cinder-abcde", 100) + expect := prefix[:84] + "-pv-cinder-abcde" + if v2 != expect { + t.Errorf("Expected %s, got %s", expect, v2) + } + + // Truncate really long cluster name + prefix = strings.Repeat("0123456789", 1000) // 10000 characters prefix + v3 := GenerateVolumeName(prefix, "pv-cinder-abcde", 100) + if v3 != expect { + t.Errorf("Expected %s, got %s", expect, v3) + } + +}