From c771bf43c4c650ffb96008d559580d33c7199497 Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Sat, 5 Mar 2016 18:11:17 -0500 Subject: [PATCH] Ubernetes Lite: apply auto-labels to dynamically provisioned volumes Fix #22532 --- pkg/volume/aws_ebs/aws_ebs.go | 14 ++++++++++++-- pkg/volume/aws_ebs/aws_ebs_test.go | 10 ++++++++-- pkg/volume/aws_ebs/aws_util.go | 17 +++++++++++++---- pkg/volume/gce_pd/gce_pd.go | 14 ++++++++++++-- pkg/volume/gce_pd/gce_pd_test.go | 10 ++++++++-- pkg/volume/gce_pd/gce_util.go | 19 ++++++++++++++----- 6 files changed, 67 insertions(+), 17 deletions(-) diff --git a/pkg/volume/aws_ebs/aws_ebs.go b/pkg/volume/aws_ebs/aws_ebs.go index dfec1fcd6979..ced8ed49353e 100644 --- a/pkg/volume/aws_ebs/aws_ebs.go +++ b/pkg/volume/aws_ebs/aws_ebs.go @@ -168,7 +168,7 @@ type ebsManager interface { // Detaches the disk from the kubelet's host machine. DetachDisk(c *awsElasticBlockStoreCleaner) error // Creates a volume - CreateVolume(provisioner *awsElasticBlockStoreProvisioner) (volumeID string, volumeSizeGB int, err error) + CreateVolume(provisioner *awsElasticBlockStoreProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, err error) // Deletes a volume DeleteVolume(deleter *awsElasticBlockStoreDeleter) error } @@ -409,7 +409,7 @@ type awsElasticBlockStoreProvisioner struct { var _ volume.Provisioner = &awsElasticBlockStoreProvisioner{} func (c *awsElasticBlockStoreProvisioner) Provision(pv *api.PersistentVolume) error { - volumeID, sizeGB, err := c.manager.CreateVolume(c) + volumeID, sizeGB, labels, err := c.manager.CreateVolume(c) if err != nil { return err } @@ -417,6 +417,16 @@ func (c *awsElasticBlockStoreProvisioner) Provision(pv *api.PersistentVolume) er pv.Spec.Capacity = api.ResourceList{ api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), } + + if len(labels) != 0 { + if pv.Labels == nil { + pv.Labels = make(map[string]string) + } + for k, v := range labels { + pv.Labels[k] = v + } + } + return nil } diff --git a/pkg/volume/aws_ebs/aws_ebs_test.go b/pkg/volume/aws_ebs/aws_ebs_test.go index caba7a368a43..140e9187b677 100644 --- a/pkg/volume/aws_ebs/aws_ebs_test.go +++ b/pkg/volume/aws_ebs/aws_ebs_test.go @@ -117,8 +117,10 @@ func (fake *fakePDManager) DetachDisk(c *awsElasticBlockStoreCleaner) error { return nil } -func (fake *fakePDManager) CreateVolume(c *awsElasticBlockStoreProvisioner) (volumeID string, volumeSizeGB int, err error) { - return "test-aws-volume-name", 100, nil +func (fake *fakePDManager) CreateVolume(c *awsElasticBlockStoreProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, err error) { + labels = make(map[string]string) + labels["fakepdmanager"] = "yes" + return "test-aws-volume-name", 100, labels, nil } func (fake *fakePDManager) DeleteVolume(cd *awsElasticBlockStoreDeleter) error { @@ -239,6 +241,10 @@ func TestPlugin(t *testing.T) { t.Errorf("Provision() returned unexpected volume size: %v", size) } + if persistentSpec.Labels["fakepdmanager"] != "yes" { + t.Errorf("Provision() returned unexpected labels: %v", persistentSpec.Labels) + } + // Test Deleter volSpec := &volume.Spec{ PersistentVolume: persistentSpec, diff --git a/pkg/volume/aws_ebs/aws_util.go b/pkg/volume/aws_ebs/aws_util.go index cfacc8b8e336..8317e9c6009e 100644 --- a/pkg/volume/aws_ebs/aws_util.go +++ b/pkg/volume/aws_ebs/aws_util.go @@ -126,10 +126,12 @@ func (util *AWSDiskUtil) DeleteVolume(d *awsElasticBlockStoreDeleter) error { return nil } -func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (volumeID string, volumeSizeGB int, err error) { +// CreateVolume creates an AWS EBS volume. +// Returns: volumeID, volumeSizeGB, labels, error +func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (string, int, map[string]string, error) { cloud, err := getCloudProvider() if err != nil { - return "", 0, err + return "", 0, nil, err } // AWS volumes don't have Name field, store the name in Name tag @@ -152,10 +154,17 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (volum name, err := cloud.CreateDisk(volumeOptions) if err != nil { glog.V(2).Infof("Error creating EBS Disk volume: %v", err) - return "", 0, err + return "", 0, nil, err } glog.V(2).Infof("Successfully created EBS Disk volume %s", name) - return name, int(requestGB), nil + + labels, err := cloud.GetVolumeLabels(name) + if err != nil { + // We don't really want to leak the volume here... + glog.Errorf("error building labels for new EBS volume %q: %v", name, err) + } + + return name, int(requestGB), labels, nil } // Attaches the specified persistent disk device to node, verifies that it is attached, and retries if it fails. diff --git a/pkg/volume/gce_pd/gce_pd.go b/pkg/volume/gce_pd/gce_pd.go index 84b0663c7fea..d6027d40067d 100644 --- a/pkg/volume/gce_pd/gce_pd.go +++ b/pkg/volume/gce_pd/gce_pd.go @@ -168,7 +168,7 @@ type pdManager interface { // Detaches the disk from the kubelet's host machine. DetachDisk(c *gcePersistentDiskCleaner) error // Creates a volume - CreateVolume(provisioner *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, err error) + CreateVolume(provisioner *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, err error) // Deletes a volume DeleteVolume(deleter *gcePersistentDiskDeleter) error } @@ -371,7 +371,7 @@ type gcePersistentDiskProvisioner struct { var _ volume.Provisioner = &gcePersistentDiskProvisioner{} func (c *gcePersistentDiskProvisioner) Provision(pv *api.PersistentVolume) error { - volumeID, sizeGB, err := c.manager.CreateVolume(c) + volumeID, sizeGB, labels, err := c.manager.CreateVolume(c) if err != nil { return err } @@ -379,6 +379,16 @@ func (c *gcePersistentDiskProvisioner) Provision(pv *api.PersistentVolume) error pv.Spec.Capacity = api.ResourceList{ api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)), } + + if len(labels) != 0 { + if pv.Labels == nil { + pv.Labels = make(map[string]string) + } + for k, v := range labels { + pv.Labels[k] = v + } + } + return nil } diff --git a/pkg/volume/gce_pd/gce_pd_test.go b/pkg/volume/gce_pd/gce_pd_test.go index 8f6829ed18d0..faa3650ecfab 100644 --- a/pkg/volume/gce_pd/gce_pd_test.go +++ b/pkg/volume/gce_pd/gce_pd_test.go @@ -113,8 +113,10 @@ func (fake *fakePDManager) DetachDisk(c *gcePersistentDiskCleaner) error { return nil } -func (fake *fakePDManager) CreateVolume(c *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, err error) { - return "test-gce-volume-name", 100, nil +func (fake *fakePDManager) CreateVolume(c *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, err error) { + labels = make(map[string]string) + labels["fakepdmanager"] = "yes" + return "test-gce-volume-name", 100, labels, nil } func (fake *fakePDManager) DeleteVolume(cd *gcePersistentDiskDeleter) error { @@ -235,6 +237,10 @@ func TestPlugin(t *testing.T) { t.Errorf("Provision() returned unexpected volume size: %v", size) } + if persistentSpec.Labels["fakepdmanager"] != "yes" { + t.Errorf("Provision() returned unexpected labels: %v", persistentSpec.Labels) + } + // Test Deleter volSpec := &volume.Spec{ PersistentVolume: persistentSpec, diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go index b190aa27b28e..1482610a168e 100644 --- a/pkg/volume/gce_pd/gce_util.go +++ b/pkg/volume/gce_pd/gce_util.go @@ -127,10 +127,12 @@ func (util *GCEDiskUtil) DeleteVolume(d *gcePersistentDiskDeleter) error { return nil } -func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, err error) { +// CreateVolume creates a GCE PD. +// Returns: volumeID, volumeSizeGB, labels, error +func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (string, int, map[string]string, error) { cloud, err := getCloudProvider() if err != nil { - return "", 0, err + return "", 0, nil, err } name := volume.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 63) // GCE PD name can have up to 63 characters @@ -143,16 +145,23 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (volum zone, err := cloud.GetZone() if err != nil { glog.V(2).Infof("error getting zone information from GCE: %v", err) - return "", 0, err + return "", 0, nil, err } err = cloud.CreateDisk(name, zone.FailureDomain, int64(requestGB), *c.options.CloudTags) if err != nil { glog.V(2).Infof("Error creating GCE PD volume: %v", err) - return "", 0, err + return "", 0, nil, err } glog.V(2).Infof("Successfully created GCE PD volume %s", name) - return name, int(requestGB), nil + + labels, err := cloud.GetAutoLabelsForPD(name) + if err != nil { + // We don't really want to leak the volume here... + glog.Errorf("error getting labels for volume %q: %v", name, err) + } + + return name, int(requestGB), labels, nil } // Attaches the specified persistent disk device to node, verifies that it is attached, and retries if it fails.