Skip to content

Commit

Permalink
UPSTREAM: 44798: Cinder: Automatically Generate Zone if Availability …
Browse files Browse the repository at this point in the history
…in Storage Class is not Configured

Backport of Kubernetes PR kubernetes#44798 (kubernetes#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
  • Loading branch information
pospispa authored and smarterclayton committed May 23, 2017
1 parent c8d8c34 commit b6294c4
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 37 deletions.
2 changes: 1 addition & 1 deletion pkg/cloudprovider/providers/openstack/openstack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
26 changes: 13 additions & 13 deletions pkg/cloudprovider/providers/openstack/openstack_volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/providers/rackspace/rackspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 5 additions & 3 deletions pkg/volume/cinder/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
],
)

Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/cinder/attacher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/volume/cinder/cinder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -482,15 +482,15 @@ 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
}

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",
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/cinder/cinder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
61 changes: 51 additions & 10 deletions pkg/volume/cinder/cinder_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)]
Expand All @@ -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 {
Expand Down

0 comments on commit b6294c4

Please sign in to comment.