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
This commit is contained in:
Justin Santa Barbara 2016-06-17 23:16:07 -04:00
parent e711cbf912
commit 9c2566572d
4 changed files with 30 additions and 10 deletions

View File

@ -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

View File

@ -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")
}

View File

@ -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)

View File

@ -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
}