GCE Multizone: Allow volumes to be created in non-master zone

We had a long-lasting bug which prevented creation of volumes in non-master zones, because the cloudprovider in the volume label admission controller is not initialized with the multizone setting (issue #27656). This implements a simple workaround: if the volume is created with the failure-domain zone label, we look for the volume in that zone. This is more efficient, avoids introducing a new semantic, and allows users (and the dynamic provisioner) to create volumes in non-master zones. Fixes #27657
kubernetes · Jun 18, 2016 · 9c25665 · 9c25665
1 parent e711cbf
commit 9c25665
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 10 deletions.
diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go
@@ -120,9 +120,11 @@ type Disks interface {
 	// DeleteDisk deletes PD.
 	DeleteDisk(diskToDelete string) error
 
-	// GetAutoLabelsForPD returns labels to apply to PeristentVolume
+	// GetAutoLabelsForPD returns labels to apply to PersistentVolume
 	// representing this PD, namely failure domain and zone.
-	GetAutoLabelsForPD(name string) (map[string]string, error)
+	// zone can be provided to specify the zone for the PD,
+	// if empty all managed zones will be searched.
+	GetAutoLabelsForPD(name string, zone string) (map[string]string, error)
 }
 
 type instRefSlice []*compute.InstanceReference
@@ -2250,13 +2252,27 @@ func (gce *GCECloud) DeleteDisk(diskToDelete string) error {
 // Builds the labels that should be automatically added to a PersistentVolume backed by a GCE PD
 // Specifically, this builds FailureDomain (zone) and Region labels.
 // The PersistentVolumeLabel admission controller calls this and adds the labels when a PV is created.
-func (gce *GCECloud) GetAutoLabelsForPD(name string) (map[string]string, error) {
-	disk, err := gce.getDiskByNameUnknownZone(name)
-	if err != nil {
-		return nil, err
+// If zone is specified, the volume will only be found in the specified zone,
+// otherwise all managed zones will be searched.
+func (gce *GCECloud) GetAutoLabelsForPD(name string, zone string) (map[string]string, error) {
+	var disk *gceDisk
+	var err error
+	if zone == "" {
+		disk, err = gce.getDiskByNameUnknownZone(name)
+		if err != nil {
+			return nil, err
+		}
+		zone = disk.Zone
+	} else {
+		// We could assume the disks exists; we have all the information we need
+		// However it is more consistent to ensure the disk exists,
+		// and in future we may gather addition information (e.g. disk type, IOPS etc)
+		disk, err = gce.getDiskByName(name, zone)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	zone := disk.Zone
 	region, err := GetGCERegion(zone)
 	if err != nil {
 		return nil, err

diff --git a/pkg/volume/gce_pd/attacher_test.go b/pkg/volume/gce_pd/attacher_test.go
@@ -342,6 +342,6 @@ func (testcase *testcase) DeleteDisk(diskToDelete string) error {
 	return errors.New("Not implemented")
 }
 
-func (testcase *testcase) GetAutoLabelsForPD(name string) (map[string]string, error) {
+func (testcase *testcase) GetAutoLabelsForPD(name string, zone string) (map[string]string, error) {
 	return map[string]string{}, errors.New("Not implemented")
 }
diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go
@@ -97,7 +97,7 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 	}
 	glog.V(2).Infof("Successfully created GCE PD volume %s", name)
 
-	labels, err := cloud.GetAutoLabelsForPD(name)
+	labels, err := cloud.GetAutoLabelsForPD(name, zone)
 	if err != nil {
 		// We don't really want to leak the volume here...
 		glog.Errorf("error getting labels for volume %q: %v", name, err)

diff --git a/plugin/pkg/admission/persistentvolume/label/admission.go b/plugin/pkg/admission/persistentvolume/label/admission.go
@@ -25,6 +25,7 @@ import (
 
 	"k8s.io/kubernetes/pkg/admission"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/cloudprovider"
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/aws"
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
@@ -159,7 +160,10 @@ func (l *persistentVolumeLabel) findGCEPDLabels(volume *api.PersistentVolume) (m
 		return nil, fmt.Errorf("unable to build GCE cloud provider for PD")
 	}
 
-	labels, err := provider.GetAutoLabelsForPD(volume.Spec.GCEPersistentDisk.PDName)
+	// If the zone is already labeled, honor the hint
+	zone := volume.Labels[unversioned.LabelZoneFailureDomain]
+
+	labels, err := provider.GetAutoLabelsForPD(volume.Spec.GCEPersistentDisk.PDName, zone)
 	if err != nil {
 		return nil, err
 	}