Enable dynamic provisioning of GCE Regional PD

This is the code required to create a GCE Regional PD via the Kubernetes
dynamic provisioning and a GCE PD StorageClass.

pkg/volume/gce_pd/attacher_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/volume"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
 
@@ -361,6 +362,10 @@ func (testcase *testcase) CreateDisk(name string, diskType string, zone string,
 	return errors.New("Not implemented")
 }
 
+func (testcase *testcase) CreateRegionalDisk(name string, diskType string, replicaZones sets.String, sizeGb int64, tags map[string]string) error {
+	return errors.New("Not implemented")
+}
+
 func (testcase *testcase) DeleteDisk(diskToDelete string) error {
 	return errors.New("Not implemented")
 }

pkg/volume/gce_pd/gce_util.go

@@ -40,9 +40,11 @@ const (
 	diskPartitionSuffix  = "-part"
 	diskSDPath           = "/dev/sd"
 	diskSDPattern        = "/dev/sd*"
+	regionalPDZonesAuto  = "auto" // "replica-zones: auto" means Kubernetes will select zones for RePD
 	maxChecks            = 60
 	maxRetries           = 10
 	checkSleepDuration   = time.Second
+	maxRegionalPDZones   = 2
 )
 
 // These variables are modified only in unit tests and should be constant
@@ -70,7 +72,7 @@ func (util *GCEDiskUtil) DeleteVolume(d *gcePersistentDiskDeleter) error {
 }
 
 // CreateVolume creates a GCE PD.
-// Returns: volumeID, volumeSizeGB, labels, error
+// Returns: gcePDName, volumeSizeGB, labels, fsType, error
 func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (string, int, map[string]string, string, error) {
 	cloud, err := getCloudProvider(c.gcePersistentDisk.plugin.host.GetCloudProvider())
 	if err != nil {
@@ -88,8 +90,10 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 	diskType := ""
 	configuredZone := ""
 	configuredZones := ""
+	configuredReplicaZones := ""
 	zonePresent := false
 	zonesPresent := false
+	replicaZonesPresent := false
 	fstype := ""
 	for k, v := range c.options.Parameters {
 		switch strings.ToLower(k) {
@@ -101,6 +105,9 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 		case "zones":
 			zonesPresent = true
 			configuredZones = v
+		case "replica-zones":
+			replicaZonesPresent = true
+			configuredReplicaZones = v
 		case volume.VolumeParameterFSType:
 			fstype = v
 		default:
@@ -108,8 +115,10 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 		}
 	}
 
-	if zonePresent && zonesPresent {
+	if ((zonePresent || zonesPresent) && replicaZonesPresent) ||
-		return "", 0, nil, "", fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time")
+		(zonePresent && zonesPresent) {
+		// 011, 101, 111, 110
+		return "", 0, nil, "", fmt.Errorf("a combination of zone, zones, and replica-zones StorageClass parameters must not be used at the same time")
 	}
 
 	// TODO: implement PVC.Selector parsing
@@ -117,20 +126,47 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 		return "", 0, nil, "", fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on GCE")
 	}
 
+	if !zonePresent && !zonesPresent && replicaZonesPresent {
+		// 001 - "replica-zones" specified
+		replicaZones, err := volume.ZonesToSet(configuredReplicaZones)
+		if err != nil {
+			return "", 0, nil, "", err
+		}
+
+		err = createRegionalPD(
+			name,
+			c.options.PVC.Name,
+			diskType,
+			replicaZones,
+			requestGB,
+			c.options.CloudTags,
+			cloud)
+		if err != nil {
+			glog.V(2).Infof("Error creating regional GCE PD volume: %v", err)
+			return "", 0, nil, "", err
+		}
+
+		glog.V(2).Infof("Successfully created Regional GCE PD volume %s", name)
+	} else {
 		var zones sets.String
 		if !zonePresent && !zonesPresent {
+			// 000 - neither "zone", "zones", or "replica-zones" specified
+			// Pick a zone randomly selected from all active zones where
+			// Kubernetes cluster has a node.
 			zones, err = cloud.GetAllZones()
 			if err != nil {
 				glog.V(2).Infof("error getting zone information from GCE: %v", err)
 				return "", 0, nil, "", err
 			}
-	}
+		} else if !zonePresent && zonesPresent {
-	if !zonePresent && zonesPresent {
+			// 010 - "zones" specified
+			// Pick a zone randomly selected from specified set.
 			if zones, err = volume.ZonesToSet(configuredZones); err != nil {
 				return "", 0, nil, "", err
 			}
-	}
+		} else if zonePresent && !zonesPresent {
-	if zonePresent && !zonesPresent {
+			// 100 - "zone" specified
+			// Use specified zone
 			if err := volume.ValidateZone(configuredZone); err != nil {
 				return "", 0, nil, "", err
 			}
@@ -139,14 +175,20 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 		}
 		zone := volume.ChooseZoneForVolume(zones, c.options.PVC.Name)
 
-	err = cloud.CreateDisk(name, diskType, zone, int64(requestGB), *c.options.CloudTags)
+		if err := cloud.CreateDisk(
-	if err != nil {
+			name,
-		glog.V(2).Infof("Error creating GCE PD volume: %v", err)
+			diskType,
+			zone,
+			int64(requestGB),
+			*c.options.CloudTags); err != nil {
+			glog.V(2).Infof("Error creating single-zone GCE PD volume: %v", err)
 			return "", 0, nil, "", err
 		}
-	glog.V(2).Infof("Successfully created GCE PD volume %s", name)
 
-	labels, err := cloud.GetAutoLabelsForPD(name, zone)
+		glog.V(2).Infof("Successfully created single-zone GCE PD volume %s", 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)
@@ -155,6 +197,48 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 	return name, int(requestGB), labels, fstype, nil
 }
 
+// Creates a Regional PD
+func createRegionalPD(
+	diskName string,
+	pvcName string,
+	diskType string,
+	replicaZones sets.String,
+	requestGB int64,
+	cloudTags *map[string]string,
+	cloud *gcecloud.GCECloud) error {
+
+	autoZoneSelection := false
+	if replicaZones.Len() != maxRegionalPDZones {
+		replicaZonesList := replicaZones.UnsortedList()
+		if replicaZones.Len() == 1 && replicaZonesList[0] == regionalPDZonesAuto {
+			// User requested automatic zone selection.
+			autoZoneSelection = true
+		} else {
+			return fmt.Errorf(
+				"replica-zones specifies %d zones. It must specify %d zones or the keyword \"auto\" to let Kubernetes select zones.",
+				replicaZones.Len(),
+				maxRegionalPDZones)
+		}
+	}
+
+	selectedReplicaZones := replicaZones
+	if autoZoneSelection {
+		selectedReplicaZones = volume.ChooseZonesForVolume(
+			replicaZones, pvcName, maxRegionalPDZones)
+	}
+
+	if err := cloud.CreateRegionalDisk(
+		diskName,
+		diskType,
+		selectedReplicaZones,
+		int64(requestGB),
+		*cloudTags); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // Returns the first path that exists, or empty string if none exist.
 func verifyDevicePath(devicePaths []string, sdBeforeSet sets.String) (string, error) {
 	if err := udevadmChangeToNewDrives(sdBeforeSet); err != nil {

pkg/volume/util.go

@@ -298,9 +298,52 @@ func GetPath(mounter Mounter) (string, error) {
 func ChooseZoneForVolume(zones sets.String, pvcName string) string {
 	// We create the volume in a zone determined by the name
 	// Eventually the scheduler will coordinate placement into an available zone
-	var hash uint32
+	hash, index := getPVCNameHashAndIndexOffset(pvcName)
-	var index uint32
 
+	// Zones.List returns zones in a consistent order (sorted)
+	// We do have a potential failure case where volumes will not be properly spread,
+	// if the set of zones changes during StatefulSet volume creation.  However, this is
+	// probably relatively unlikely because we expect the set of zones to be essentially
+	// static for clusters.
+	// Hopefully we can address this problem if/when we do full scheduler integration of
+	// PVC placement (which could also e.g. avoid putting volumes in overloaded or
+	// unhealthy zones)
+	zoneSlice := zones.List()
+	zone := zoneSlice[(hash+index)%uint32(len(zoneSlice))]
+
+	glog.V(2).Infof("Creating volume for PVC %q; chose zone=%q from zones=%q", pvcName, zone, zoneSlice)
+	return zone
+}
+
+// ChooseZonesForVolume is identical to ChooseZoneForVolume, but selects a multiple zones, for multi-zone disks.
+func ChooseZonesForVolume(zones sets.String, pvcName string, numZones uint32) sets.String {
+	// We create the volume in a zone determined by the name
+	// Eventually the scheduler will coordinate placement into an available zone
+	hash, index := getPVCNameHashAndIndexOffset(pvcName)
+
+	// Zones.List returns zones in a consistent order (sorted)
+	// We do have a potential failure case where volumes will not be properly spread,
+	// if the set of zones changes during StatefulSet volume creation.  However, this is
+	// probably relatively unlikely because we expect the set of zones to be essentially
+	// static for clusters.
+	// Hopefully we can address this problem if/when we do full scheduler integration of
+	// PVC placement (which could also e.g. avoid putting volumes in overloaded or
+	// unhealthy zones)
+	zoneSlice := zones.List()
+	replicaZones := sets.NewString()
+
+	startingIndex := index * numZones
+	for index = startingIndex; index < startingIndex+numZones; index++ {
+		zone := zoneSlice[(hash+index)%uint32(len(zoneSlice))]
+		replicaZones.Insert(zone)
+	}
+
+	glog.V(2).Infof("Creating volume for replicated PVC %q; chosen zones=%q from zones=%q",
+		pvcName, replicaZones.UnsortedList(), zoneSlice)
+	return replicaZones
+}
+
+func getPVCNameHashAndIndexOffset(pvcName string) (hash uint32, index uint32) {
 	if pvcName == "" {
 		// We should always be called with a name; this shouldn't happen
 		glog.Warningf("No name defined during volume create; choosing random zone")
@@ -349,19 +392,7 @@ func ChooseZoneForVolume(zones sets.String, pvcName string) string {
 		hash = h.Sum32()
 	}
 
-	// Zones.List returns zones in a consistent order (sorted)
+	return hash, index
-	// We do have a potential failure case where volumes will not be properly spread,
-	// if the set of zones changes during StatefulSet volume creation.  However, this is
-	// probably relatively unlikely because we expect the set of zones to be essentially
-	// static for clusters.
-	// Hopefully we can address this problem if/when we do full scheduler integration of
-	// PVC placement (which could also e.g. avoid putting volumes in overloaded or
-	// unhealthy zones)
-	zoneSlice := zones.List()
-	zone := zoneSlice[(hash+index)%uint32(len(zoneSlice))]
-
-	glog.V(2).Infof("Creating volume for PVC %q; chose zone=%q from zones=%q", pvcName, zone, zoneSlice)
-	return zone
 }
 
 // UnmountViaEmptyDir delegates the tear down operation for secret, configmap, git_repo and downwardapi

pkg/volume/util_test.go

@@ -396,134 +396,477 @@ func TestChooseZoneForVolume(t *testing.T) {
 	checkFnv32(t, "", 2166136261)
 
 	tests := []struct {
-		Zones      []string
+		Zones      sets.String
 		VolumeName string
 		Expected   string
 	}{
 		// Test for PVC names that don't have a dash
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley",
 			Expected:   "a", // hash("henley") == 0
 		},
 		// Tests for PVC names that end in - number, but don't look like statefulset PVCs
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-0",
 			Expected:   "a", // hash("henley") == 0
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-1",
 			Expected:   "b", // hash("henley") + 1 == 1
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-2",
 			Expected:   "c", // hash("henley") + 2 == 2
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-3",
 			Expected:   "a", // hash("henley") + 3 == 3 === 0 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-4",
 			Expected:   "b", // hash("henley") + 4 == 4 === 1 mod 3
 		},
 		// Tests for PVC names that are edge cases
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-",
 			Expected:   "c", // hash("henley-") = 2652299129 === 2 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "henley-a",
 			Expected:   "c", // hash("henley-a") = 1459735322 === 2 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium--1",
 			Expected:   "c", // hash("") + 1 == 2166136261 + 1 === 2 mod 3
 		},
 		// Tests for PVC names for simple StatefulSet cases
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley-1",
 			Expected:   "b", // hash("henley") + 1 == 1
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "loud-henley-1",
 			Expected:   "b", // hash("henley") + 1 == 1
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "quiet-henley-2",
 			Expected:   "c", // hash("henley") + 2 == 2
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley-2",
 			Expected:   "c", // hash("henley") + 2 == 2
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley-3",
 			Expected:   "a", // hash("henley") + 3 == 3 === 0 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley-4",
 			Expected:   "b", // hash("henley") + 4 == 4 === 1 mod 3
 		},
 		// Tests for statefulsets (or claims) with dashes in the names
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-alpha-henley-2",
 			Expected:   "c", // hash("henley") + 2 == 2
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-beta-henley-3",
 			Expected:   "a", // hash("henley") + 3 == 3 === 0 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-gamma-henley-4",
 			Expected:   "b", // hash("henley") + 4 == 4 === 1 mod 3
 		},
 		// Tests for statefulsets name ending in -
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley--2",
 			Expected:   "a", // hash("") + 2 == 0 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley--3",
 			Expected:   "b", // hash("") + 3 == 1 mod 3
 		},
 		{
-			Zones:      []string{"a", "b", "c"},
+			Zones:      sets.NewString("a", "b", "c"),
 			VolumeName: "medium-henley--4",
 			Expected:   "c", // hash("") + 4 == 2 mod 3
 		},
 	}
 
 	for _, test := range tests {
-		zonesSet := sets.NewString(test.Zones...)
+		actual := ChooseZoneForVolume(test.Zones, test.VolumeName)
 
-		actual := ChooseZoneForVolume(zonesSet, test.VolumeName)
+		if actual != test.Expected {
-
-		for actual != test.Expected {
 			t.Errorf("Test %v failed, expected zone %q, actual %q", test, test.Expected, actual)
 		}
 	}
 }
 
+func TestChooseZonesForVolume(t *testing.T) {
+	checkFnv32(t, "henley", 1180403676)
+	// 1180403676 mod 3 == 0, so the offset from "henley" is 0, which makes it easier to verify this by inspection
+
+	// A few others
+	checkFnv32(t, "henley-", 2652299129)
+	checkFnv32(t, "henley-a", 1459735322)
+	checkFnv32(t, "", 2166136261)
+
+	tests := []struct {
+		Zones      sets.String
+		VolumeName string
+		NumZones   uint32
+		Expected   sets.String
+	}{
+		// Test for PVC names that don't have a dash
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley",
+			NumZones:   1,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 */, "b"),
+		},
+		// Tests for PVC names that end in - number, but don't look like statefulset PVCs
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-0",
+			NumZones:   1,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-0",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 */, "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-1",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("henley") + 1 == 1 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-1",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") + 1 + 1(startingIndex) == 2 */, "a"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-2",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("henley") + 2 == 2 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-2",
+			NumZones:   2,
+			Expected:   sets.NewString("b" /* hash("henley") + 2 + 2(startingIndex) == 4 */, "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-3",
+			NumZones:   1,
+			Expected:   sets.NewString("a" /* hash("henley") + 3 == 3 === 0 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-3",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("henley") + 3 + 3(startingIndex) == 6 */, "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-4",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("henley") + 4 == 4 === 1 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-4",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") + 4 + 4(startingIndex) == 8 */, "a"),
+		},
+		// Tests for PVC names that are edge cases
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("henley-") = 2652299129 === 2 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley-") = 2652299129 === 2 mod 3 = 2 */, "a"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-a",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("henley-a") = 1459735322 === 2 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "henley-a",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley-a") = 1459735322 === 2 mod 3 = 2 */, "a"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium--1",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("") + 1 == 2166136261 + 1 === 2 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium--1",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("") + 1 + 1(startingIndex) == 2166136261 + 1 + 1 === 3 mod 3 = 0 */, "b"),
+		},
+		// Tests for PVC names for simple StatefulSet cases
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-1",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("henley") + 1 == 1 */),
+		},
+		// Tests for PVC names for simple StatefulSet cases
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-1",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") + 1 + 1(startingIndex) == 2 */, "a"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "loud-henley-1",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("henley") + 1 == 1 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "loud-henley-1",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") + 1 + 1(startingIndex) == 2 */, "a"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "quiet-henley-2",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("henley") + 2 == 2 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "quiet-henley-2",
+			NumZones:   2,
+			Expected:   sets.NewString("b" /* hash("henley") + 2 + 2(startingIndex) == 4 */, "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-2",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("henley") + 2 == 2 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-2",
+			NumZones:   2,
+			Expected:   sets.NewString("b" /* hash("henley") + 2 + 2(startingIndex) == 4 */, "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-3",
+			NumZones:   1,
+			Expected:   sets.NewString("a" /* hash("henley") + 3 == 3 === 0 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-3",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("henley") + 3 + 3(startingIndex) == 6 === 6 mod 3 = 0 */, "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-4",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("henley") + 4 == 4 === 1 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley-4",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") + 4 + 4(startingIndex) == 8 === 2 mod 3 */, "a"),
+		},
+		// Tests for statefulsets (or claims) with dashes in the names
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-alpha-henley-2",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("henley") + 2 == 2 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-alpha-henley-2",
+			NumZones:   2,
+			Expected:   sets.NewString("b" /* hash("henley") + 2 + 2(startingIndex) == 4 */, "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-beta-henley-3",
+			NumZones:   1,
+			Expected:   sets.NewString("a" /* hash("henley") + 3 == 3 === 0 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-beta-henley-3",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("henley") + 3 + 3(startingIndex) == 6 === 0 mod 3 */, "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-gamma-henley-4",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("henley") + 4 == 4 === 1 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-gamma-henley-4",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") + 4 + 4(startingIndex) == 8 === 2 mod 3 */, "a"),
+		},
+		// Tests for statefulsets name ending in -
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--2",
+			NumZones:   1,
+			Expected:   sets.NewString("a" /* hash("") + 2 == 0 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--2",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("") + 2 + 2(startingIndex) == 2 mod 3 */, "a"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--3",
+			NumZones:   1,
+			Expected:   sets.NewString("b" /* hash("") + 3 == 1 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--3",
+			NumZones:   2,
+			Expected:   sets.NewString("b" /* hash("") + 3 + 3(startingIndex) == 1 mod 3 */, "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--4",
+			NumZones:   1,
+			Expected:   sets.NewString("c" /* hash("") + 4 == 2 mod 3 */),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--4",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("") + 4 + 4(startingIndex) == 0 mod 3 */, "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--4",
+			NumZones:   3,
+			Expected:   sets.NewString("c" /* hash("") + 4 == 2 mod 3 */, "a", "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c"),
+			VolumeName: "medium-henley--4",
+			NumZones:   4,
+			Expected:   sets.NewString("c" /* hash("") + 4 + 9(startingIndex) == 2 mod 3 */, "a", "b", "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-0",
+			NumZones:   2,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 */, "b"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-1",
+			NumZones:   2,
+			Expected:   sets.NewString("c" /* hash("henley") == 0 + 2 */, "d"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-2",
+			NumZones:   2,
+			Expected:   sets.NewString("e" /* hash("henley") == 0 + 2 + 2(startingIndex) */, "f"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-3",
+			NumZones:   2,
+			Expected:   sets.NewString("g" /* hash("henley") == 0 + 2 + 4(startingIndex) */, "h"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-0",
+			NumZones:   3,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 */, "b", "c"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-1",
+			NumZones:   3,
+			Expected:   sets.NewString("d" /* hash("henley") == 0 + 1 + 2(startingIndex) */, "e", "f"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-2",
+			NumZones:   3,
+			Expected:   sets.NewString("g" /* hash("henley") == 0 + 2 + 4(startingIndex) */, "h", "i"),
+		},
+		{
+			Zones:      sets.NewString("a", "b", "c", "d", "e", "f", "g", "h", "i"),
+			VolumeName: "henley-3",
+			NumZones:   3,
+			Expected:   sets.NewString("a" /* hash("henley") == 0 + 3 + 6(startingIndex) */, "b", "c"),
+		},
+	}
+
+	for _, test := range tests {
+		actual := ChooseZonesForVolume(test.Zones, test.VolumeName, test.NumZones)
+
+		if !actual.Equal(test.Expected) {
+			t.Errorf("Test %v failed, expected zone %#v, actual %#v", test, test.Expected, actual)
+		}
+	}
+}
+
 func TestZonesToSet(t *testing.T) {
 	functionUnderTest := "ZonesToSet"
 	// First part: want an error