From 0f3a9cfc5fa87142daa1084d77ed797a2e612028 Mon Sep 17 00:00:00 2001 From: pospispa Date: Thu, 10 Nov 2016 12:32:35 +0100 Subject: [PATCH 1/4] Added func ZonesToSet An admin shall be able to configure a comma separated list of zones for a StorageClass. That's why the func ZonesToSet (string) (set.String, error) is added. The func ZonesToSet converts a string containing a comma separated list of zones to a set. In case the list contains an empty zone an error is returned. --- pkg/volume/util.go | 14 ++++++++++++++ pkg/volume/util_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/pkg/volume/util.go b/pkg/volume/util.go index 64deeeb27ac0..0d0c59c7193a 100644 --- a/pkg/volume/util.go +++ b/pkg/volume/util.go @@ -410,3 +410,17 @@ func JoinMountOptions(userOptions []string, systemOptions []string) []string { } return allMountOptions.UnsortedList() } + +// ZonesToSet converts a string containing a comma separated list of zones to set +func ZonesToSet(zonesString string) (sets.String, error) { + zonesSlice := strings.Split(zonesString, ",") + zonesSet := make(sets.String) + for _, zone := range zonesSlice { + trimmedZone := strings.TrimSpace(zone) + if trimmedZone == "" { + return make(sets.String), fmt.Errorf("comma separated list of zones (%q) must not contain an empty zone", zonesString) + } + zonesSet.Insert(trimmedZone) + } + return zonesSet, nil +} diff --git a/pkg/volume/util_test.go b/pkg/volume/util_test.go index 9af72b133f8d..656fbf04f3e0 100644 --- a/pkg/volume/util_test.go +++ b/pkg/volume/util_test.go @@ -523,3 +523,37 @@ func TestChooseZoneForVolume(t *testing.T) { } } } + +func TestZonesToSet(t *testing.T) { + functionUnderTest := "ZonesToSet" + // First part: want an error + sliceOfZones := []string{"", ",", "us-east-1a, , us-east-1d", ", us-west-1b", "us-west-2b,"} + for _, zones := range sliceOfZones { + if got, err := ZonesToSet(zones); err == nil { + t.Errorf("%v(%v) returned (%v), want (%v)", functionUnderTest, zones, got, "an error") + } + } + + // Second part: want no error + tests := []struct { + zones string + want sets.String + }{ + { + zones: "us-east-1a", + want: sets.String{"us-east-1a": sets.Empty{}}, + }, + { + zones: "us-east-1a, us-west-2a", + want: sets.String{ + "us-east-1a": sets.Empty{}, + "us-west-2a": sets.Empty{}, + }, + }, + } + for _, tt := range tests { + if got, err := ZonesToSet(tt.zones); err != nil || !got.Equal(tt.want) { + t.Errorf("%v(%v) returned (%v), want (%v)", functionUnderTest, tt.zones, got, tt.want) + } + } +} From dd17d620d7e9c18b3faaf39322f946c73b21a807 Mon Sep 17 00:00:00 2001 From: pospispa Date: Mon, 13 Mar 2017 11:54:01 +0100 Subject: [PATCH 2/4] Added func ValidateZone The zone parameter provided in a Storage Class may erroneously be an empty string or contain only spaces and tab characters. Such situation shall be detected and reported as an error. That's why the func ValidateZone was added. --- pkg/volume/util.go | 10 ++++++++++ pkg/volume/util_test.go | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/pkg/volume/util.go b/pkg/volume/util.go index 0d0c59c7193a..aac4c9e543cd 100644 --- a/pkg/volume/util.go +++ b/pkg/volume/util.go @@ -424,3 +424,13 @@ func ZonesToSet(zonesString string) (sets.String, error) { } return zonesSet, nil } + +// ValidateZone returns: +// - an error in case zone is an empty string or contains only any combination of spaces and tab characters +// - nil otherwise +func ValidateZone(zone string) error { + if strings.TrimSpace(zone) == "" { + return fmt.Errorf("the provided %q zone is not valid, it's an empty string or contains only spaces and tab characters", zone) + } + return nil +} diff --git a/pkg/volume/util_test.go b/pkg/volume/util_test.go index 656fbf04f3e0..67d504b451dc 100644 --- a/pkg/volume/util_test.go +++ b/pkg/volume/util_test.go @@ -557,3 +557,23 @@ func TestZonesToSet(t *testing.T) { } } } + +func TestValidateZone(t *testing.T) { + functionUnderTest := "ValidateZone" + + // First part: want an error + errCases := []string{"", " "} + for _, errCase := range errCases { + if got := ValidateZone(errCase); got == nil { + t.Errorf("%v(%v) returned (%v), want (%v)", functionUnderTest, errCase, got, "an error") + } + } + + // Second part: want no error + succCases := []string{" us-east-1a "} + for _, succCase := range succCases { + if got := ValidateZone(succCase); got != nil { + t.Errorf("%v(%v) returned (%v), want (%v)", functionUnderTest, succCase, got, nil) + } + } +} From d73c0d649d469db2de233e6568345f930994998f Mon Sep 17 00:00:00 2001 From: pospispa Date: Thu, 8 Dec 2016 13:01:32 +0100 Subject: [PATCH 3/4] Admin Can Specify in Which GCE Availability Zone(s) a PV Shall Be Created An admin wants to specify in which GCE availability zone(s) users may create persistent volumes using dynamic provisioning. That's why the admin can now configure in StorageClass object a comma separated list of zones. Dynamically created PVs for PVCs that use the StorageClass are created in one of the configured zones. --- .../persistent-volume-provisioning/README.md | 5 +-- pkg/volume/gce_pd/gce_util.go | 36 +++++++++++++++---- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/examples/persistent-volume-provisioning/README.md b/examples/persistent-volume-provisioning/README.md index 56f7dbb5d484..6ba72f0c9bd0 100644 --- a/examples/persistent-volume-provisioning/README.md +++ b/examples/persistent-volume-provisioning/README.md @@ -45,11 +45,12 @@ metadata: provisioner: kubernetes.io/gce-pd parameters: type: pd-standard - zone: us-central1-a + zones: us-central1-a, us-central1-b ``` * `type`: `pd-standard` or `pd-ssd`. Default: `pd-ssd` -* `zone`: GCE zone. If not specified, a random zone in the same region as controller-manager will be chosen. +* `zone`: GCE zone. If neither zone nor zones is specified, volumes are generally round-robin-ed across all active zones where Kubernetes cluster has a node. Note: zone and zones parameters must not be used at the same time. +* `zones`: a comma separated list of GCE zone(s). If neither zone nor zones is specified, volumes are generally round-robin-ed across all active zones where Kubernetes cluster has a node. Note: zone and zones parameters must not be used at the same time. #### vSphere diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go index 8d71fc0eb4fe..0fe15ff0aa50 100644 --- a/pkg/volume/gce_pd/gce_util.go +++ b/pkg/volume/gce_pd/gce_util.go @@ -86,33 +86,55 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin // Apply Parameters (case-insensitive). We leave validation of // the values to the cloud provider. diskType := "" - zone := "" + configuredZone := "" + configuredZones := "" + zonePresent := false + zonesPresent := false for k, v := range c.options.Parameters { switch strings.ToLower(k) { case "type": diskType = v case "zone": - zone = v + zonePresent = true + configuredZone = v + case "zones": + zonesPresent = true + configuredZones = v default: return "", 0, nil, fmt.Errorf("invalid option %q for volume plugin %s", k, c.plugin.GetPluginName()) } } + if zonePresent && zonesPresent { + return "", 0, nil, fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time") + } + // TODO: implement PVC.Selector parsing if c.options.PVC.Spec.Selector != nil { return "", 0, nil, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on GCE") } - if zone == "" { - // No zone specified, choose one randomly in the same region as the - // node is running. - zones, err := cloud.GetAllZones() + var zones sets.String + if !zonePresent && !zonesPresent { + zones, err = cloud.GetAllZones() if err != nil { glog.V(2).Infof("error getting zone information from GCE: %v", err) return "", 0, nil, err } - zone = volume.ChooseZoneForVolume(zones, c.options.PVC.Name) } + if !zonePresent && zonesPresent { + if zones, err = volume.ZonesToSet(configuredZones); err != nil { + return "", 0, nil, err + } + } + if zonePresent && !zonesPresent { + if err := volume.ValidateZone(configuredZone); err != nil { + return "", 0, nil, err + } + zones = make(sets.String) + zones.Insert(configuredZone) + } + zone := volume.ChooseZoneForVolume(zones, c.options.PVC.Name) err = cloud.CreateDisk(name, diskType, zone, int64(requestGB), *c.options.CloudTags) if err != nil { From 9eb912e62fd7a8f9fd7d6347b412e5b80ae98ab3 Mon Sep 17 00:00:00 2001 From: pospispa Date: Thu, 8 Dec 2016 13:35:56 +0100 Subject: [PATCH 4/4] Admin Can Specify in Which AWS Availability Zone(s) a PV Shall Be Created An admin wants to specify in which AWS availability zone(s) users may create persistent volumes using dynamic provisioning. That's why the admin can now configure in StorageClass object a comma separated list of zones. Dynamically created PVs for PVCs that use the StorageClass are created in one of the configured zones. --- .../persistent-volume-provisioning/README.md | 5 ++-- pkg/cloudprovider/providers/aws/aws.go | 30 ++++++++++++++----- pkg/volume/aws_ebs/aws_util.go | 10 +++++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/examples/persistent-volume-provisioning/README.md b/examples/persistent-volume-provisioning/README.md index 6ba72f0c9bd0..124c4f088e12 100644 --- a/examples/persistent-volume-provisioning/README.md +++ b/examples/persistent-volume-provisioning/README.md @@ -25,12 +25,13 @@ metadata: provisioner: kubernetes.io/aws-ebs parameters: type: io1 - zone: us-east-1d + zones: us-east-1d, us-east-1c iopsPerGB: "10" ``` * `type`: `io1`, `gp2`, `sc1`, `st1`. See AWS docs for details. Default: `gp2`. -* `zone`: AWS zone. If not specified, a random zone from those where Kubernetes cluster has a node is chosen. +* `zone`: AWS zone. If neither zone nor zones is specified, volumes are generally round-robin-ed across all active zones where Kubernetes cluster has a node. Note: zone and zones parameters must not be used at the same time. +* `zones`: a comma separated list of AWS zone(s). If neither zone nor zones is specified, volumes are generally round-robin-ed across all active zones where Kubernetes cluster has a node. Note: zone and zones parameters must not be used at the same time. * `iopsPerGB`: only for `io1` volumes. I/O operations per second per GiB. AWS volume plugin multiplies this with size of requested volume to compute IOPS of the volume and caps it at 20 000 IOPS (maximum supported by AWS, see AWS docs). * `encrypted`: denotes whether the EBS volume should be encrypted or not. Valid values are `true` or `false`. * `kmsKeyId`: optional. The full Amazon Resource Name of the key to use when encrypting the volume. If none is supplied but `encrypted` is true, a key is generated by AWS. See AWS docs for valid ARN value. diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 8d42884e146d..e3dfade42f0d 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -287,11 +287,14 @@ const ( // VolumeOptions specifies capacity and tags for a volume. type VolumeOptions struct { - CapacityGB int - Tags map[string]string - PVCName string - VolumeType string - AvailabilityZone string + CapacityGB int + Tags map[string]string + PVCName string + VolumeType string + ZonePresent bool + ZonesPresent bool + AvailabilityZone string + AvailabilityZones string // IOPSPerGB x CapacityGB will give total IOPS of the volume to create. // Calculated total IOPS will be capped at MaxTotalIOPS. IOPSPerGB int @@ -1675,10 +1678,23 @@ func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (KubernetesVolumeID, er return "", fmt.Errorf("error querying for all zones: %v", err) } - createAZ := volumeOptions.AvailabilityZone - if createAZ == "" { + var createAZ string + if !volumeOptions.ZonePresent && !volumeOptions.ZonesPresent { createAZ = volume.ChooseZoneForVolume(allZones, volumeOptions.PVCName) } + if !volumeOptions.ZonePresent && volumeOptions.ZonesPresent { + if adminSetOfZones, err := volume.ZonesToSet(volumeOptions.AvailabilityZones); err != nil { + return "", err + } else { + createAZ = volume.ChooseZoneForVolume(adminSetOfZones, volumeOptions.PVCName) + } + } + if volumeOptions.ZonePresent && !volumeOptions.ZonesPresent { + if err := volume.ValidateZone(volumeOptions.AvailabilityZone); err != nil { + return "", err + } + createAZ = volumeOptions.AvailabilityZone + } var createType string var iops int64 diff --git a/pkg/volume/aws_ebs/aws_util.go b/pkg/volume/aws_ebs/aws_util.go index 72fac0635bf7..bfad040c84a5 100644 --- a/pkg/volume/aws_ebs/aws_util.go +++ b/pkg/volume/aws_ebs/aws_util.go @@ -91,12 +91,18 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (aws.K } // Apply Parameters (case-insensitive). We leave validation of // the values to the cloud provider. + volumeOptions.ZonePresent = false + volumeOptions.ZonesPresent = false for k, v := range c.options.Parameters { switch strings.ToLower(k) { case "type": volumeOptions.VolumeType = v case "zone": + volumeOptions.ZonePresent = true volumeOptions.AvailabilityZone = v + case "zones": + volumeOptions.ZonesPresent = true + volumeOptions.AvailabilityZones = v case "iopspergb": volumeOptions.IOPSPerGB, err = strconv.Atoi(v) if err != nil { @@ -114,6 +120,10 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner) (aws.K } } + if volumeOptions.ZonePresent && volumeOptions.ZonesPresent { + return "", 0, nil, fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time") + } + // TODO: implement PVC.Selector parsing if c.options.PVC.Spec.Selector != nil { return "", 0, nil, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on AWS")