From b6294c4e31b48e3c2cd5f4c1a63ba5e21feaf8f7 Mon Sep 17 00:00:00 2001 From: pospispa Date: Fri, 12 May 2017 11:15:23 +0200 Subject: [PATCH] UPSTREAM: 44798: Cinder: Automatically Generate Zone if Availability in Storage Class is not Configured Backport of Kubernetes PR #44798 (https://github.com/kubernetes/kubernetes/pull/44798). In case the availability parameter is not configured in a cinder Storage Class the cinder volume is always provisioned in the nova availability zone. That is incorrect. Now, the cinder volume is provisioned in a zone that is generated by an algorithm from the set of zone available in the cluster. Positive side-effect: cinder volumes for individual pods in a StatefulSet are provisioned in unique zones. This increases the StatefulSet resilience. :100644 100644 2f91722b65... d4a75b3a40... M pkg/cloudprovider/providers/openstack/openstack_test.go :100644 100644 68e35d520f... cdd1c8d2f4... M pkg/cloudprovider/providers/openstack/openstack_volumes.go :100644 100644 5939ba3a2e... 052c3e1e92... M pkg/cloudprovider/providers/rackspace/rackspace.go :100644 100644 c9a7fc435b... eb5e1d20e2... M pkg/volume/cinder/BUILD :100644 100644 d45b99784d... 956f7a1088... M pkg/volume/cinder/attacher_test.go :100644 100644 047e735568... 06d4528434... M pkg/volume/cinder/cinder.go :100644 100644 7b1735fa69... 66acbcd22e... M pkg/volume/cinder/cinder_test.go :100644 100644 5b0402afd6... c0013e29e3... M pkg/volume/cinder/cinder_util.go --- .../providers/openstack/openstack_test.go | 2 +- .../providers/openstack/openstack_volumes.go | 26 ++++---- .../providers/rackspace/rackspace.go | 4 +- pkg/volume/cinder/BUILD | 8 ++- pkg/volume/cinder/attacher_test.go | 4 +- pkg/volume/cinder/cinder.go | 8 +-- pkg/volume/cinder/cinder_test.go | 4 +- pkg/volume/cinder/cinder_util.go | 61 ++++++++++++++++--- 8 files changed, 80 insertions(+), 37 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_test.go b/pkg/cloudprovider/providers/openstack/openstack_test.go index 2f91722b652f4..d4a75b3a40f71 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_test.go +++ b/pkg/cloudprovider/providers/openstack/openstack_test.go @@ -354,7 +354,7 @@ func TestVolumes(t *testing.T) { tags := map[string]string{ "test": "value", } - vol, err := os.CreateVolume("kubernetes-test-volume-"+rand.String(10), 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/cloudprovider/providers/openstack/openstack_volumes.go b/pkg/cloudprovider/providers/openstack/openstack_volumes.go index 68e35d520f2d9..cdd1c8d2f44ad 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_volumes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_volumes.go @@ -36,7 +36,7 @@ import ( ) type volumeService interface { - createVolume(opts VolumeCreateOpts) (string, error) + createVolume(opts VolumeCreateOpts) (string, string, error) getVolume(diskName string) (Volume, error) deleteVolume(volumeName string) error } @@ -74,7 +74,7 @@ type VolumeCreateOpts struct { Metadata map[string]string } -func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, error) { +func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, string, error) { create_opts := volumes_v1.CreateOpts{ Name: opts.Name, @@ -86,12 +86,12 @@ func (volumes *VolumesV1) createVolume(opts VolumeCreateOpts) (string, error) { vol, err := volumes_v1.Create(volumes.blockstorage, create_opts).Extract() if err != nil { - return "", err + return "", "", err } - return vol.ID, nil + return vol.ID, vol.AvailabilityZone, nil } -func (volumes *VolumesV2) createVolume(opts VolumeCreateOpts) (string, error) { +func (volumes *VolumesV2) createVolume(opts VolumeCreateOpts) (string, string, error) { create_opts := volumes_v2.CreateOpts{ Name: opts.Name, @@ -103,9 +103,9 @@ func (volumes *VolumesV2) createVolume(opts VolumeCreateOpts) (string, error) { vol, err := volumes_v2.Create(volumes.blockstorage, create_opts).Extract() if err != nil { - return "", err + return "", "", err } - return vol.ID, nil + return vol.ID, vol.AvailabilityZone, nil } func (volumes *VolumesV1) getVolume(diskName string) (Volume, error) { @@ -283,12 +283,12 @@ func (os *OpenStack) getVolume(diskName string) (Volume, error) { } // Create a volume of given size (in GiB) -func (os *OpenStack) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeName string, err error) { +func (os *OpenStack) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (string, string, error) { volumes, err := os.volumeService("") if err != nil || volumes == nil { glog.Errorf("Unable to initialize cinder client for region: %s", os.region) - return "", err + return "", "", err } opts := VolumeCreateOpts{ Name: name, @@ -299,15 +299,15 @@ func (os *OpenStack) CreateVolume(name string, size int, vtype, availability str if tags != nil { opts.Metadata = *tags } - volume_id, err := volumes.createVolume(opts) + volumeId, volumeAZ, err := volumes.createVolume(opts) if err != nil { glog.Errorf("Failed to create a %d GB volume: %v", size, err) - return "", err + return "", "", err } - glog.Infof("Created volume %v", volume_id) - return volume_id, nil + glog.Infof("Created volume %v in Availability Zone: %v", volumeId, volumeAZ) + return volumeId, volumeAZ, nil } // GetDevicePath returns the path of an attached block storage volume, specified by its id. diff --git a/pkg/cloudprovider/providers/rackspace/rackspace.go b/pkg/cloudprovider/providers/rackspace/rackspace.go index 5939ba3a2e590..052c3e1e920fd 100644 --- a/pkg/cloudprovider/providers/rackspace/rackspace.go +++ b/pkg/cloudprovider/providers/rackspace/rackspace.go @@ -466,8 +466,8 @@ func (os *Rackspace) GetZone() (cloudprovider.Zone, error) { } // Create a volume of given size (in GiB) -func (rs *Rackspace) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeName string, err error) { - return "", errors.New("unimplemented") +func (rs *Rackspace) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeName string, volumeAZ string, err error) { + return "", "", errors.New("unimplemented") } func (rs *Rackspace) DeleteVolume(volumeName string) error { diff --git a/pkg/volume/cinder/BUILD b/pkg/volume/cinder/BUILD index c9a7fc435b4fa..eb5e1d20e22b6 100644 --- a/pkg/volume/cinder/BUILD +++ b/pkg/volume/cinder/BUILD @@ -19,6 +19,7 @@ go_library( tags = ["automanaged"], deps = [ "//pkg/api/v1:go_default_library", + "//pkg/client/clientset_generated/clientset:go_default_library", "//pkg/cloudprovider:go_default_library", "//pkg/cloudprovider/providers/openstack:go_default_library", "//pkg/cloudprovider/providers/rackspace:go_default_library", @@ -29,9 +30,10 @@ go_library( "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//vendor:github.com/golang/glog", - "//vendor:k8s.io/apimachinery/pkg/api/resource", - "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", - "//vendor:k8s.io/apimachinery/pkg/types", + "//vendor:k8s.io/apimachinery/pkg/api/resource:go_default_library", + "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor:k8s.io/apimachinery/pkg/types:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], ) diff --git a/pkg/volume/cinder/attacher_test.go b/pkg/volume/cinder/attacher_test.go index d45b99784d253..956f7a10885b6 100644 --- a/pkg/volume/cinder/attacher_test.go +++ b/pkg/volume/cinder/attacher_test.go @@ -506,8 +506,8 @@ func (testcase *testcase) ShouldTrustDevicePath() bool { return true } -func (testcase *testcase) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeName string, err error) { - return "", errors.New("Not implemented") +func (testcase *testcase) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeId string, volumeAZ string, err error) { + return "", "", errors.New("Not implemented") } func (testcase *testcase) GetDevicePath(diskId string) string { diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index 047e735568acf..06d45284342c1 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -47,7 +47,7 @@ type CinderProvider interface { AttachDisk(instanceID string, diskName string) (string, error) DetachDisk(instanceID string, partialDiskId string) error DeleteVolume(volumeName string) error - CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (volumeName string, err error) + CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (string, string, error) GetDevicePath(diskId string) string InstanceID() (string, error) GetAttachmentDiskPath(instanceID string, diskName string) (string, error) @@ -239,7 +239,7 @@ type cdManager interface { // Detaches the disk from the kubelet's host machine. DetachDisk(unmounter *cinderVolumeUnmounter) error // Creates a volume - CreateVolume(provisioner *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, err error) + CreateVolume(provisioner *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, err error) // Deletes a volume DeleteVolume(deleter *cinderVolumeDeleter) error } @@ -482,7 +482,7 @@ type cinderVolumeProvisioner struct { var _ volume.Provisioner = &cinderVolumeProvisioner{} func (c *cinderVolumeProvisioner) Provision() (*v1.PersistentVolume, error) { - volumeID, sizeGB, err := c.manager.CreateVolume(c) + volumeID, sizeGB, labels, err := c.manager.CreateVolume(c) if err != nil { return nil, err } @@ -490,7 +490,7 @@ func (c *cinderVolumeProvisioner) Provision() (*v1.PersistentVolume, error) { pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: c.options.PVName, - Labels: map[string]string{}, + Labels: labels, Annotations: map[string]string{ "kubernetes.io/createdby": "cinder-dynamic-provisioner", }, diff --git a/pkg/volume/cinder/cinder_test.go b/pkg/volume/cinder/cinder_test.go index 7b1735fa69518..66acbcd22e133 100644 --- a/pkg/volume/cinder/cinder_test.go +++ b/pkg/volume/cinder/cinder_test.go @@ -116,8 +116,8 @@ func (fake *fakePDManager) DetachDisk(c *cinderVolumeUnmounter) error { return nil } -func (fake *fakePDManager) CreateVolume(c *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, err error) { - return "test-volume-name", 1, nil +func (fake *fakePDManager) CreateVolume(c *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, err error) { + return "test-volume-name", 1, nil, nil } func (fake *fakePDManager) DeleteVolume(cd *cinderVolumeDeleter) error { diff --git a/pkg/volume/cinder/cinder_util.go b/pkg/volume/cinder/cinder_util.go index 5b0402afd66bb..c0013e29e3e7a 100644 --- a/pkg/volume/cinder/cinder_util.go +++ b/pkg/volume/cinder/cinder_util.go @@ -25,7 +25,11 @@ import ( "time" "github.com/golang/glog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/api/v1" + "k8s.io/kubernetes/pkg/client/clientset_generated/clientset" "k8s.io/kubernetes/pkg/util/exec" "k8s.io/kubernetes/pkg/volume" ) @@ -135,10 +139,28 @@ func (util *CinderDiskUtil) DeleteVolume(cd *cinderVolumeDeleter) error { return nil } -func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, err error) { +func getZonesFromNodes(kubeClient clientset.Interface) (sets.String, error) { + // TODO: caching, currently it is overkill because it calls this function + // only when it creates dynamic PV + zones := make(sets.String) + nodes, err := kubeClient.Core().Nodes().List(metav1.ListOptions{}) + if err != nil { + glog.V(2).Infof("Error listing nodes") + return zones, err + } + for _, node := range nodes.Items { + if zone, ok := node.Labels[metav1.LabelZoneFailureDomain]; ok { + zones.Insert(zone) + } + } + glog.V(4).Infof("zones found: %v", zones) + return zones, nil +} + +func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, volumeLabels map[string]string, err error) { cloud, err := c.plugin.getCloudProvider() if err != nil { - return "", 0, err + return "", 0, nil, err } capacity := c.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] @@ -157,21 +179,40 @@ func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID s case "availability": availability = v default: - return "", 0, fmt.Errorf("invalid option %q for volume plugin %s", k, c.plugin.GetPluginName()) + return "", 0, nil, fmt.Errorf("invalid option %q for volume plugin %s", k, c.plugin.GetPluginName()) } } // TODO: implement PVC.Selector parsing if c.options.PVC.Spec.Selector != nil { - return "", 0, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on Cinder") + return "", 0, nil, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on Cinder") } - name, err = cloud.CreateVolume(name, volSizeGB, vtype, availability, c.options.CloudTags) - if err != nil { - glog.V(2).Infof("Error creating cinder volume: %v", err) - return "", 0, err + if availability == "" { + // No zone specified, choose one randomly in the same region + zones, err := getZonesFromNodes(c.plugin.host.GetKubeClient()) + if err != nil { + glog.V(2).Infof("error getting zone information: %v", err) + return "", 0, nil, err + } + // if we did not get any zones, lets leave it blank and gophercloud will + // use zone "nova" as default + if len(zones) > 0 { + availability = volume.ChooseZoneForVolume(zones, c.options.PVC.Name) + } } - glog.V(2).Infof("Successfully created cinder volume %s", name) - return name, volSizeGB, nil + + volumeId, volumeAZ, errr := cloud.CreateVolume(name, volSizeGB, vtype, availability, c.options.CloudTags) + if errr != nil { + glog.V(2).Infof("Error creating cinder volume: %v", errr) + return "", 0, nil, errr + } + glog.V(2).Infof("Successfully created cinder volume %s", volumeId) + + // these are needed that pod is spawning to same AZ + volumeLabels = make(map[string]string) + volumeLabels[metav1.LabelZoneFailureDomain] = volumeAZ + + return volumeId, volSizeGB, volumeLabels, nil } func probeAttachedVolume() error {