Add DynamicProvisioningScheduling support for GCE PD and RePD

Signed-off-by: Deep Debroy <ddebroy@docker.com>

pkg/volume/gce_pd/gce_pd.go

@@ -32,6 +32,7 @@ import (
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
 	"k8s.io/kubernetes/pkg/features"
+	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	"k8s.io/kubernetes/pkg/util/mount"
 	kstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
@@ -303,7 +304,7 @@ func (plugin *gcePersistentDiskPlugin) ConstructVolumeSpec(volumeName, mountPath
 // Abstract interface to PD operations.
 type pdManager interface {
 	// Creates a volume
-	CreateVolume(provisioner *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, fstype string, err error)
+	CreateVolume(provisioner *gcePersistentDiskProvisioner, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm) (volumeID string, volumeSizeGB int, labels map[string]string, fstype string, err error)
 	// Deletes a volume
 	DeleteVolume(deleter *gcePersistentDiskDeleter) error
 }
@@ -471,7 +472,7 @@ func (c *gcePersistentDiskProvisioner) Provision(selectedNode *v1.Node, allowedT
 		return nil, fmt.Errorf("invalid AccessModes %v: only AccessModes %v are supported", c.options.PVC.Spec.AccessModes, c.plugin.GetAccessModes())
 	}
 
-	volumeID, sizeGB, labels, fstype, err := c.manager.CreateVolume(c)
+	volumeID, sizeGB, labels, fstype, err := c.manager.CreateVolume(c, selectedNode, allowedTopologies)
 	if err != nil {
 		return nil, err
 	}
@@ -519,14 +520,32 @@ func (c *gcePersistentDiskProvisioner) Provision(selectedNode *v1.Node, allowedT
 		pv.Spec.AccessModes = c.plugin.GetAccessModes()
 	}
 
+	requirements := make([]v1.NodeSelectorRequirement, 0)
 	if len(labels) != 0 {
 		if pv.Labels == nil {
 			pv.Labels = make(map[string]string)
 		}
 		for k, v := range labels {
 			pv.Labels[k] = v
+			var values []string
+			if k == kubeletapis.LabelZoneFailureDomain {
+				values, err = util.LabelZonesToList(v)
+				if err != nil {
+					return nil, fmt.Errorf("failed to convert label string for Zone: %s to a List: %v", v, err)
+				}
+			} else {
+				values = []string{v}
+			}
+			requirements = append(requirements, v1.NodeSelectorRequirement{Key: k, Operator: v1.NodeSelectorOpIn, Values: values})
 		}
 	}
 
+	if utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) && len(requirements) > 0 {
+		pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity)
+		pv.Spec.NodeAffinity.Required = new(v1.NodeSelector)
+		pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1)
+		pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = requirements
+	}
+
 	return pv, nil
 }

pkg/volume/gce_pd/gce_pd_test.go

@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"reflect"
+	"sort"
 	"testing"
 
 	"k8s.io/api/core/v1"
@@ -27,10 +29,12 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 	utiltesting "k8s.io/client-go/util/testing"
+	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
 	"k8s.io/kubernetes/pkg/volume/util"
+	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 )
 
 func TestCanSupport(t *testing.T) {
@@ -78,9 +82,10 @@ func TestGetAccessModes(t *testing.T) {
 type fakePDManager struct {
 }
 
-func (fake *fakePDManager) CreateVolume(c *gcePersistentDiskProvisioner) (volumeID string, volumeSizeGB int, labels map[string]string, fstype string, err error) {
+func (fake *fakePDManager) CreateVolume(c *gcePersistentDiskProvisioner, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm) (volumeID string, volumeSizeGB int, labels map[string]string, fstype string, err error) {
 	labels = make(map[string]string)
 	labels["fakepdmanager"] = "yes"
+	labels[kubeletapis.LabelZoneFailureDomain] = "zone1__zone2"
 	return "test-gce-volume-name", 100, labels, "", nil
 }
 
@@ -91,6 +96,15 @@ func (fake *fakePDManager) DeleteVolume(cd *gcePersistentDiskDeleter) error {
 	return nil
 }
 
+func getNodeSelectorRequirementWithKey(key string, term v1.NodeSelectorTerm) (*v1.NodeSelectorRequirement, error) {
+	for _, r := range term.MatchExpressions {
+		if r.Key == key {
+			return &r, nil
+		}
+	}
+	return nil, fmt.Errorf("key %s not found", key)
+}
+
 func TestPlugin(t *testing.T) {
 	tmpDir, err := utiltesting.MkTmpdir("gcepdTest")
 	if err != nil {
@@ -182,7 +196,35 @@ func TestPlugin(t *testing.T) {
 	}
 
 	if persistentSpec.Labels["fakepdmanager"] != "yes" {
-		t.Errorf("Provision() returned unexpected labels: %v", persistentSpec.Labels)
+		t.Errorf("Provision() returned unexpected value for fakepdmanager: %v", persistentSpec.Labels["fakepdmanager"])
+	}
+
+	if persistentSpec.Labels[kubeletapis.LabelZoneFailureDomain] != "zone1__zone2" {
+		t.Errorf("Provision() returned unexpected value for %s: %v", kubeletapis.LabelZoneFailureDomain, persistentSpec.Labels[kubeletapis.LabelZoneFailureDomain])
+	}
+
+	if persistentSpec.Spec.NodeAffinity == nil {
+		t.Errorf("Unexpected nil NodeAffinity found")
+	}
+	if len(persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms) != 1 {
+		t.Errorf("Unexpected number of NodeSelectorTerms")
+	}
+	term := persistentSpec.Spec.NodeAffinity.Required.NodeSelectorTerms[0]
+	if len(term.MatchExpressions) != 2 {
+		t.Errorf("Unexpected number of NodeSelectorRequirements in volume NodeAffinity: %d", len(term.MatchExpressions))
+	}
+	r, _ := getNodeSelectorRequirementWithKey("fakepdmanager", term)
+	if r == nil || r.Values[0] != "yes" || r.Operator != v1.NodeSelectorOpIn {
+		t.Errorf("NodeSelectorRequirement fakepdmanager-in-yes not found in volume NodeAffinity")
+	}
+	zones, _ := volumeutil.ZonesToSet("zone1,zone2")
+	r, _ = getNodeSelectorRequirementWithKey(kubeletapis.LabelZoneFailureDomain, term)
+	if r == nil {
+		t.Errorf("NodeSelectorRequirement %s-in-%v not found in volume NodeAffinity", kubeletapis.LabelZoneFailureDomain, zones)
+	}
+	sort.Strings(r.Values)
+	if !reflect.DeepEqual(r.Values, zones.List()) {
+		t.Errorf("ZoneFailureDomain elements %v does not match zone labels %v", r.Values, zones)
 	}
 
 	// Test Deleter

pkg/volume/gce_pd/gce_util.go

@@ -87,7 +87,7 @@ func (util *GCEDiskUtil) DeleteVolume(d *gcePersistentDiskDeleter) error {
 
 // CreateVolume creates a GCE PD.
 // Returns: gcePDName, volumeSizeGB, labels, fsType, error
-func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (string, int, map[string]string, string, error) {
+func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm) (string, int, map[string]string, string, error) {
 	cloud, err := getCloudProvider(c.gcePersistentDisk.plugin.host.GetCloudProvider())
 	if err != nil {
 		return "", 0, nil, "", err
@@ -104,7 +104,7 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 	// to the cloud provider.
 	diskType := ""
 	configuredZone := ""
-	configuredZones := ""
+	var configuredZones sets.String
 	zonePresent := false
 	zonesPresent := false
 	replicationType := replicationTypeNone
@@ -118,7 +118,10 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 			configuredZone = v
 		case "zones":
 			zonesPresent = true
-			configuredZones = v
+			configuredZones, err = volumeutil.ZonesToSet(v)
+			if err != nil {
+				return "", 0, nil, "", err
+			}
 		case "replication-type":
 			if !utilfeature.DefaultFeatureGate.Enabled(features.GCERegionalPersistentDisk) {
 				return "", 0, nil, "",
@@ -133,76 +136,49 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 		}
 	}
 
-	if zonePresent && zonesPresent {
-		return "", 0, nil, "", fmt.Errorf("the 'zone' and 'zones' StorageClass parameters must not be used at the same time")
-	}
-
-	if replicationType == replicationTypeRegionalPD && zonePresent {
-		// If a user accidentally types 'zone' instead of 'zones', we want to throw an error
-		// instead of assuming that 'zones' is empty and proceed by randomly selecting zones.
-		return "", 0, nil, "", fmt.Errorf("the '%s' replication type does not support the 'zone' parameter; use 'zones' instead", replicationTypeRegionalPD)
-	}
-
 	// 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")
 	}
 
+	var activezones sets.String
+	activezones, err = cloud.GetAllCurrentZones()
+	if err != nil {
+		return "", 0, nil, "", err
+	}
+
 	switch replicationType {
 	case replicationTypeRegionalPD:
-		err = createRegionalPD(
+		selectedZones, err := volumeutil.SelectZonesForVolume(zonePresent, zonesPresent, configuredZone, configuredZones, activezones, node, allowedTopologies, c.options.PVC.Name, maxRegionalPDZones)
-			name,
-			c.options.PVC.Name,
-			diskType,
-			configuredZones,
-			requestGB,
-			c.options.CloudTags,
-			cloud)
 		if err != nil {
+			glog.V(2).Infof("Error selecting zones for regional GCE PD volume: %v", err)
+			return "", 0, nil, "", err
+		}
+		if err = cloud.CreateRegionalDisk(
+			name,
+			diskType,
+			selectedZones,
+			int64(requestGB),
+			*c.options.CloudTags); 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)
 
 	case replicationTypeNone:
-		var zones sets.String
+		selectedZone, err := volumeutil.SelectZoneForVolume(zonePresent, zonesPresent, configuredZone, configuredZones, activezones, node, allowedTopologies, c.options.PVC.Name)
-		if !zonePresent && !zonesPresent {
+		if err != nil {
-			// 00 - neither "zone" or "zones" specified
+			return "", 0, nil, "", err
-			// Pick a zone randomly selected from all active zones where
-			// Kubernetes cluster has a node.
-			zones, err = cloud.GetAllCurrentZones()
-			if err != nil {
-				glog.V(2).Infof("error getting zone information from GCE: %v", err)
-				return "", 0, nil, "", err
-			}
-		} else if !zonePresent && zonesPresent {
-			// 01 - "zones" specified
-			// Pick a zone randomly selected from specified set.
-			if zones, err = volumeutil.ZonesToSet(configuredZones); err != nil {
-				return "", 0, nil, "", err
-			}
-		} else if zonePresent && !zonesPresent {
-			// 10 - "zone" specified
-			// Use specified zone
-			if err := volumeutil.ValidateZone(configuredZone); err != nil {
-				return "", 0, nil, "", err
-			}
-			zones = make(sets.String)
-			zones.Insert(configuredZone)
 		}
-		zone := volumeutil.ChooseZoneForVolume(zones, c.options.PVC.Name)
-
 		if err := cloud.CreateDisk(
 			name,
 			diskType,
-			zone,
+			selectedZone,
 			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 single-zone GCE PD volume %s", name)
 
 	default:
@@ -218,57 +194,6 @@ 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,
-	zonesString string,
-	requestGB int64,
-	cloudTags *map[string]string,
-	cloud *gcecloud.GCECloud) error {
-
-	var replicaZones sets.String
-	var err error
-
-	if zonesString == "" {
-		// Consider all zones
-		replicaZones, err = cloud.GetAllCurrentZones()
-		if err != nil {
-			glog.V(2).Infof("error getting zone information from GCE: %v", err)
-			return err
-		}
-	} else {
-		replicaZones, err = volumeutil.ZonesToSet(zonesString)
-		if err != nil {
-			return err
-		}
-	}
-
-	zoneCount := replicaZones.Len()
-	var selectedReplicaZones sets.String
-	if zoneCount < maxRegionalPDZones {
-		return fmt.Errorf("cannot specify only %d zone(s) for Regional PDs.", zoneCount)
-	} else if zoneCount == maxRegionalPDZones {
-		selectedReplicaZones = replicaZones
-	} else {
-		// Must randomly select zones
-		selectedReplicaZones = volumeutil.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, diskName string) (string, error) {
 	if err := udevadmChangeToNewDrives(sdBeforeSet); err != nil {

pkg/volume/util/util.go

@@ -320,62 +320,102 @@ func LoadPodFromFile(filePath string) (*v1.Pod, error) {
 	return pod, nil
 }
 
-// SelectZone selects a zone for a volume based on several factors:
+// SelectZoneForVolume is a wrapper around SelectZonesForVolume
-// node.zone, allowedTopologies, zone/zones parameters from storageclass
+// to select a single zone for a volume based on parameters
-// and zones with active nodes from the cluster
 func SelectZoneForVolume(zoneParameterPresent, zonesParameterPresent bool, zoneParameter string, zonesParameter, zonesWithNodes sets.String, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm, pvcName string) (string, error) {
+	zones, err := SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent, zoneParameter, zonesParameter, zonesWithNodes, node, allowedTopologies, pvcName, 1)
+	if err != nil {
+		return "", err
+	}
+	zone, ok := zones.PopAny()
+	if !ok {
+		return "", fmt.Errorf("could not determine a zone to provision volume in")
+	}
+	return zone, nil
+}
+
+// SelectZonesForVolume selects zones for a volume based on several factors:
+// node.zone, allowedTopologies, zone/zones parameters from storageclass,
+// zones with active nodes from the cluster. The number of zones = replicas.
+func SelectZonesForVolume(zoneParameterPresent, zonesParameterPresent bool, zoneParameter string, zonesParameter, zonesWithNodes sets.String, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm, pvcName string, numReplicas uint32) (sets.String, error) {
 	if zoneParameterPresent && zonesParameterPresent {
-		return "", fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time")
+		return nil, fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time")
 	}
 
-	// pick zone from node if present
+	var zoneFromNode string
+	// pick one zone from node if present
 	if node != nil {
 		// DynamicProvisioningScheduling implicit since node is not nil
 		if zoneParameterPresent || zonesParameterPresent {
-			return "", fmt.Errorf("zone[s] cannot be specified in StorageClass if VolumeBindingMode is set to WaitForFirstConsumer. Please specify allowedTopologies in StorageClass for constraining zones.")
+			return nil, fmt.Errorf("zone[s] cannot be specified in StorageClass if VolumeBindingMode is set to WaitForFirstConsumer. Please specify allowedTopologies in StorageClass for constraining zones")
 		}
 
-		// pick node's zone and ignore any allowedTopologies (since scheduler is assumed to have accounted for it already)
+		// pick node's zone for one of the replicas
-		z, ok := node.ObjectMeta.Labels[kubeletapis.LabelZoneFailureDomain]
+		var ok bool
+		zoneFromNode, ok = node.ObjectMeta.Labels[kubeletapis.LabelZoneFailureDomain]
 		if !ok {
-			return "", fmt.Errorf("%s Label for node missing", kubeletapis.LabelZoneFailureDomain)
+			return nil, fmt.Errorf("%s Label for node missing", kubeletapis.LabelZoneFailureDomain)
+		}
+		// if single replica volume and node with zone found, return immediately
+		if numReplicas == 1 {
+			return sets.NewString(zoneFromNode), nil
 		}
-		return z, nil
 	}
 
 	// pick zone from allowedZones if specified
 	allowedZones, err := ZonesFromAllowedTopologies(allowedTopologies)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	if (len(allowedTopologies) > 0) && (allowedZones.Len() == 0) {
-		return "", fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", kubeletapis.LabelZoneFailureDomain, kubeletapis.LabelZoneFailureDomain)
+		return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", kubeletapis.LabelZoneFailureDomain, kubeletapis.LabelZoneFailureDomain)
 	}
 
 	if allowedZones.Len() > 0 {
 		// DynamicProvisioningScheduling implicit since allowedZones present
 		if zoneParameterPresent || zonesParameterPresent {
-			return "", fmt.Errorf("zone[s] cannot be specified in StorageClass if allowedTopologies specified")
+			return nil, fmt.Errorf("zone[s] cannot be specified in StorageClass if allowedTopologies specified")
+		}
+		// scheduler will guarantee if node != null above, zoneFromNode is member of allowedZones.
+		// so if zoneFromNode != "", we can safely assume it is part of allowedZones.
+		if zones, err := chooseZonesForVolumeIncludingZone(allowedZones, pvcName, zoneFromNode, numReplicas); err != nil {
+			return nil, fmt.Errorf("cannot process zones in allowedTopologies: %v", err)
+		} else {
+			return zones, nil
 		}
-		return ChooseZoneForVolume(*allowedZones, pvcName), nil
 	}
 
 	// pick zone from parameters if present
 	if zoneParameterPresent {
-		return zoneParameter, nil
+		if numReplicas > 1 {
+			return nil, fmt.Errorf("zone cannot be specified if desired number of replicas for pv is greather than 1. Please specify zones or allowedTopologies to specify desired zones")
+		}
+		return sets.NewString(zoneParameter), nil
 	}
 
 	if zonesParameterPresent {
-		return ChooseZoneForVolume(zonesParameter, pvcName), nil
+		if uint32(zonesParameter.Len()) < numReplicas {
+			return nil, fmt.Errorf("not enough zones found in zones parameter to provision a volume with %d replicas. Found %d zones, need %d zones", numReplicas, zonesParameter.Len(), numReplicas)
+		}
+		// directly choose from zones parameter; no zone from node need to be considered
+		return ChooseZonesForVolume(zonesParameter, pvcName, numReplicas), nil
 	}
 
 	// pick zone from zones with nodes
-	return ChooseZoneForVolume(zonesWithNodes, pvcName), nil
+	if zonesWithNodes.Len() > 0 {
+		// If node != null (and thus zoneFromNode != ""), zoneFromNode will be member of zonesWithNodes
+		if zones, err := chooseZonesForVolumeIncludingZone(zonesWithNodes, pvcName, zoneFromNode, numReplicas); err != nil {
+			return nil, fmt.Errorf("cannot process zones where nodes exist in the cluster: %v", err)
+		} else {
+			return zones, nil
+		}
+	}
+	return nil, fmt.Errorf("cannot determine zones to provision volume in")
 }
 
 // ZonesFromAllowedTopologies returns a list of zones specified in allowedTopologies
-func ZonesFromAllowedTopologies(allowedTopologies []v1.TopologySelectorTerm) (*sets.String, error) {
+func ZonesFromAllowedTopologies(allowedTopologies []v1.TopologySelectorTerm) (sets.String, error) {
 	zones := make(sets.String)
 	for _, term := range allowedTopologies {
 		for _, exp := range term.MatchLabelExpressions {
@@ -388,7 +428,7 @@ func ZonesFromAllowedTopologies(allowedTopologies []v1.TopologySelectorTerm) (*s
 			}
 		}
 	}
-	return &zones, nil
+	return zones, nil
 }
 
 func ZonesSetToLabelValue(strSet sets.String) string {
@@ -397,7 +437,11 @@ func ZonesSetToLabelValue(strSet sets.String) string {
 
 // ZonesToSet converts a string containing a comma separated list of zones to set
 func ZonesToSet(zonesString string) (sets.String, error) {
-	return stringToSet(zonesString, ",")
+	zones, err := stringToSet(zonesString, ",")
+	if err != nil {
+		return nil, fmt.Errorf("error parsing zones %s, must be strings separated by commas: %v", zonesString, err)
+	}
+	return zones, nil
 }
 
 // LabelZonesToSet converts a PV label value from string containing a delimited list of zones to set
@@ -422,6 +466,27 @@ func stringToSet(str, delimiter string) (sets.String, error) {
 	return zonesSet, nil
 }
 
+// LabelZonesToList converts a PV label value from string containing a delimited list of zones to list
+func LabelZonesToList(labelZonesValue string) ([]string, error) {
+	return stringToList(labelZonesValue, kubeletapis.LabelMultiZoneDelimiter)
+}
+
+// StringToList converts a string containing list separated by specified delimiter to a list
+func stringToList(str, delimiter string) ([]string, error) {
+	zonesSlice := make([]string, 0)
+	for _, zone := range strings.Split(str, delimiter) {
+		trimmedZone := strings.TrimSpace(zone)
+		if trimmedZone == "" {
+			return nil, fmt.Errorf(
+				"%q separated list (%q) must not contain an empty string",
+				delimiter,
+				str)
+		}
+		zonesSlice = append(zonesSlice, trimmedZone)
+	}
+	return zonesSlice, nil
+}
+
 // CalculateTimeoutForVolume calculates time for a Recycler pod to complete a
 // recycle operation. The calculation and return value is either the
 // minimumTimeout or the timeoutIncrement per Gi of storage size, whichever is
@@ -539,6 +604,33 @@ func ChooseZoneForVolume(zones sets.String, pvcName string) string {
 	return zone
 }
 
+// chooseZonesForVolumeIncludingZone is a wrapper around ChooseZonesForVolume that ensures zoneToInclude is chosen
+// zoneToInclude can either be empty in which case it is ignored. If non-empty, zoneToInclude is expected to be member of zones.
+// numReplicas is expected to be > 0 and <= zones.Len()
+func chooseZonesForVolumeIncludingZone(zones sets.String, pvcName, zoneToInclude string, numReplicas uint32) (sets.String, error) {
+	if numReplicas == 0 {
+		return nil, fmt.Errorf("invalid number of replicas passed")
+	}
+	if uint32(zones.Len()) < numReplicas {
+		return nil, fmt.Errorf("not enough zones found to provision a volume with %d replicas. Need at least %d distinct zones for a volume with %d replicas", numReplicas, numReplicas, numReplicas)
+	}
+	if zoneToInclude != "" && !zones.Has(zoneToInclude) {
+		return nil, fmt.Errorf("zone to be included: %s needs to be member of set: %v", zoneToInclude, zones)
+	}
+	if uint32(zones.Len()) == numReplicas {
+		return zones, nil
+	}
+	if zoneToInclude != "" {
+		zones.Delete(zoneToInclude)
+		numReplicas = numReplicas - 1
+	}
+	zonesChosen := ChooseZonesForVolume(zones, pvcName, numReplicas)
+	if zoneToInclude != "" {
+		zonesChosen.Insert(zoneToInclude)
+	}
+	return zonesChosen, nil
+}
+
 // 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

pkg/volume/util/util_test.go

@@ -1460,6 +1460,866 @@ func TestSelectZoneForVolume(t *testing.T) {
 	}
 }
 
+func TestSelectZonesForVolume(t *testing.T) {
+
+	nodeWithZoneLabels := &v1.Node{}
+	nodeWithZoneLabels.Labels = map[string]string{kubeletapis.LabelZoneFailureDomain: "zoneX"}
+
+	nodeWithNoLabels := &v1.Node{}
+
+	tests := []struct {
+		// Parameters passed by test to SelectZonesForVolume
+		Name              string
+		ReplicaCount      uint32
+		ZonePresent       bool
+		Zone              string
+		ZonesPresent      bool
+		Zones             string
+		ZonesWithNodes    string
+		Node              *v1.Node
+		AllowedTopologies []v1.TopologySelectorTerm
+		// Expectations around returned zones from SelectZonesForVolume
+		Reject              bool   // expect error due to validation failing
+		ExpectSpecificZones bool   // expect set of returned zones to be equal to ExpectedZones (rather than subset of ExpectedZones)
+		ExpectedZones       string // set of zones that is a superset of returned zones or equal to returned zones (if ExpectSpecificZones is set)
+		ExpectSpecificZone  bool   // expect set of returned zones to include ExpectedZone as a member
+		ExpectedZone        string // zone that should be a member of returned zones
+	}{
+		// NEGATIVE TESTS
+
+		// Zone and Zones are both specified [Fail]
+		// [1] Node irrelevant
+		// [2] Zone and Zones parameters presents
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Nil_Node_with_Zone_Zones_parameters_present",
+			ZonePresent:  true,
+			Zone:         "zoneX",
+			ZonesPresent: true,
+			Zones:        "zoneX,zoneY",
+			Reject:       true,
+		},
+
+		// Node has no zone labels [Fail]
+		// [1] Node with no zone labels
+		// [2] Zone/Zones parameter irrelevant
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:   "Node_with_no_Zone_labels",
+			Node:   nodeWithNoLabels,
+			Reject: true,
+		},
+
+		// Node with Zone labels as well as Zone parameter specified [Fail]
+		// [1] Node with zone labels
+		// [2] Zone parameter specified
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:        "Node_with_Zone_labels_and_Zone_parameter_present",
+			Node:        nodeWithZoneLabels,
+			ZonePresent: true,
+			Zone:        "zoneX",
+			Reject:      true,
+		},
+
+		// Node with Zone labels as well as Zones parameter specified [Fail]
+		// [1] Node with zone labels
+		// [2] Zones parameter specified
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Node_with_Zone_labels_and_Zones_parameter_present",
+			Node:         nodeWithZoneLabels,
+			ZonesPresent: true,
+			Zones:        "zoneX,zoneY",
+			Reject:       true,
+		},
+
+		// Zone parameter as well as AllowedTopologies specified [Fail]
+		// [1] nil Node
+		// [2] Zone parameter specified
+		// [3] AllowedTopologies specified
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:        "Nil_Node_and_Zone_parameter_and_Allowed_Topology_term",
+			Node:        nil,
+			ZonePresent: true,
+			Zone:        "zoneX",
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+			},
+			Reject: true,
+		},
+
+		// Zones parameter as well as AllowedTopologies specified [Fail]
+		// [1] nil Node
+		// [2] Zones parameter specified
+		// [3] AllowedTopologies specified
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Nil_Node_and_Zones_parameter_and_Allowed_Topology_term",
+			Node:         nil,
+			ZonesPresent: true,
+			Zones:        "zoneX,zoneY",
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+			},
+			Reject: true,
+		},
+
+		// Key specified in AllowedTopologies is not LabelZoneFailureDomain [Fail]
+		// [1] nil Node
+		// [2] no Zone/Zones parameter
+		// [3] AllowedTopologies with invalid key specified
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name: "Nil_Node_and_Invalid_Allowed_Topology_Key",
+			Node: nil,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    "invalid_key",
+							Values: []string{"zoneX"},
+						},
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneY"},
+						},
+					},
+				},
+			},
+			Reject: true,
+		},
+
+		// AllowedTopologies without keys specifying LabelZoneFailureDomain [Fail]
+		// [1] nil Node
+		// [2] no Zone/Zones parameter
+		// [3] Invalid AllowedTopologies
+		// [4] ReplicaCount irrelevant
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name: "Nil_Node_and_Invalid_AllowedTopologies",
+			Node: nil,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{},
+				},
+			},
+			Reject: true,
+		},
+
+		// Zone specified with ReplicaCount > 1 [Fail]
+		// [1] nil Node
+		// [2] Zone/Zones parameter specified
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount > 1
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Zone_and_Replicas_mismatched",
+			Node:         nil,
+			ZonePresent:  true,
+			Zone:         "zoneX",
+			ReplicaCount: 2,
+			Reject:       true,
+		},
+
+		// Not enough zones in Zones parameter to satisfy ReplicaCount [Fail]
+		// [1] nil Node
+		// [2] Zone/Zones parameter specified
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount > zones
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Zones_and_Replicas_mismatched",
+			Node:         nil,
+			ZonesPresent: true,
+			Zones:        "zoneX",
+			ReplicaCount: 2,
+			Reject:       true,
+		},
+
+		// Not enough zones in ZonesWithNodes to satisfy ReplicaCount [Fail]
+		// [1] nil Node
+		// [2] no Zone/Zones parameter specified
+		// [3] AllowedTopologies irrelevant
+		// [4] ReplicaCount > ZonesWithNodes
+		// [5] ZonesWithNodes specified but not enough
+		{
+			Name:           "Zones_and_Replicas_mismatched",
+			Node:           nil,
+			ZonesWithNodes: "zoneX",
+			ReplicaCount:   2,
+			Reject:         true,
+		},
+
+		// Not enough zones in AllowedTopologies to satisfy ReplicaCount [Fail]
+		// [1] nil Node
+		// [2] Zone/Zones parameter specified
+		// [3] Invalid AllowedTopologies
+		// [4] ReplicaCount > zones
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "AllowedTopologies_and_Replicas_mismatched",
+			Node:         nil,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+			},
+			Reject: true,
+		},
+
+		// POSITIVE TESTS
+
+		// Select zones from zones parameter [Pass]
+		// [1] nil Node (Node irrelevant)
+		// [2] Zones parameter specified
+		// [3] no AllowedTopologies
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:          "Zones_parameter_Dual_replica_Superset",
+			ZonesPresent:  true,
+			Zones:         "zoneW,zoneX,zoneY,zoneZ",
+			ReplicaCount:  2,
+			Reject:        false,
+			ExpectedZones: "zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Select zones from zones parameter [Pass]
+		// [1] nil Node (Node irrelevant)
+		// [2] Zones parameter specified
+		// [3] no AllowedTopologies
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:                "Zones_parameter_Dual_replica_Match",
+			ZonesPresent:        true,
+			Zones:               "zoneX,zoneY",
+			ReplicaCount:        2,
+			Reject:              false,
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+
+		// Select zones from active zones with nodes [Pass]
+		// [1] nil Node (Node irrelevant)
+		// [2] no Zone parameter
+		// [3] no AllowedTopologies
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes specified
+		{
+			Name:           "Active_zones_Nil_node_Dual_replica_Superset",
+			ZonesWithNodes: "zoneW,zoneX,zoneY,zoneZ",
+			ReplicaCount:   2,
+			Reject:         false,
+			ExpectedZones:  "zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Select zones from active zones [Pass]
+		// [1] nil Node (Node irrelevant)
+		// [2] no Zone[s] parameter
+		// [3] no AllowedTopologies
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes specified
+		{
+			Name:                "Active_zones_Nil_node_Dual_replica_Match",
+			ZonesWithNodes:      "zoneW,zoneX",
+			ReplicaCount:        2,
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneW,zoneX",
+		},
+
+		// Select zones from node label and active zones [Pass]
+		// [1] Node with zone labels
+		// [2] no Zone parameter
+		// [3] no AllowedTopologies
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:               "Active_zones_Node_with_Zone_labels_Dual_replica_Superset",
+			Node:               nodeWithZoneLabels,
+			ZonesWithNodes:     "zoneW,zoneX,zoneY,zoneZ",
+			ReplicaCount:       2,
+			Reject:             false,
+			ExpectSpecificZone: true,
+			ExpectedZone:       "zoneX",
+			ExpectedZones:      "zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Select zones from node label and active zones [Pass]
+		// [1] Node with zone labels
+		// [2] no Zone[s] parameter
+		// [3] no AllowedTopologies
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:                "Active_zones_Node_with_Zone_labels_Dual_replica_Match",
+			Node:                nodeWithZoneLabels,
+			ZonesWithNodes:      "zoneW,zoneX",
+			ReplicaCount:        2,
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneW,zoneX",
+		},
+
+		// Select zones from node label and AllowedTopologies [Pass]
+		// [1] Node with zone labels
+		// [2] no Zone/Zones parameters
+		// [3] AllowedTopologies with single term with multiple values specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		// Note: the test Name suffix is used as the pvcname and it's suffix is important to influence ChooseZonesForVolume
+		// to NOT pick zoneX from AllowedTopologies if zoneFromNode is incorrectly set or not set at all.
+		{
+			Name:         "Node_with_Zone_labels_and_Multiple_Allowed_Topology_values_Superset-1",
+			Node:         nodeWithZoneLabels,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneV", "zoneW", "zoneX", "zoneY", "zoneZ"},
+						},
+					},
+				},
+			},
+			Reject:             false,
+			ExpectSpecificZone: true,
+			ExpectedZone:       "zoneX",
+			ExpectedZones:      "zoneV,zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Select zones from node label and AllowedTopologies [Pass]
+		// [1] Node with zone labels
+		// [2] no Zone/Zones parameters
+		// [3] AllowedTopologies with single term with multiple values specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Node_with_Zone_labels_and_Multiple_Allowed_Topology_values_Match",
+			Node:         nodeWithZoneLabels,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX", "zoneY"},
+						},
+					},
+				},
+			},
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+
+		// Select Zones from AllowedTopologies [Pass]
+		// [1] nil Node
+		// [2] no Zone/Zones parametes specified
+		// [3] AllowedTopologies with single term with multiple values specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Nil_Node_with_Multiple_Allowed_Topology_values_Superset",
+			Node:         nil,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX", "zoneY", "zoneZ"},
+						},
+					},
+				},
+			},
+			Reject:        false,
+			ExpectedZones: "zoneX,zoneY,zoneZ",
+		},
+
+		// Select Zones from AllowedTopologies [Pass]
+		// [1] nil Node
+		// [2] no Zone/Zones parametes specified
+		// [3] AllowedTopologies with single term with multiple values specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Nil_Node_with_Multiple_Allowed_Topology_values_Match",
+			Node:         nil,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX", "zoneY"},
+						},
+					},
+				},
+			},
+			Reject:              false,
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+
+		// Select zones from node label and AllowedTopologies [Pass]
+		// [1] Node with zone labels
+		// [2] no Zone/Zones parametes specified
+		// [3] AllowedTopologies with multiple terms specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		// Note: the test Name is used as the pvcname and it's hash is important to influence ChooseZonesForVolume
+		// to NOT pick zoneX from AllowedTopologies of 5 zones. If zoneFromNode (zoneX) is incorrectly set or
+		// not set at all in SelectZonesForVolume it will be detected.
+		{
+			Name:         "Node_with_Zone_labels_and_Multiple_Allowed_Topology_terms_Superset-2",
+			Node:         nodeWithZoneLabels,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneV"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneW"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneY"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneZ"},
+						},
+					},
+				},
+			},
+			Reject:             false,
+			ExpectSpecificZone: true,
+			ExpectedZone:       "zoneX",
+			ExpectedZones:      "zoneV,zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Select zones from node label and AllowedTopologies [Pass]
+		// [1] Node with zone labels
+		// [2] no Zone/Zones parametes specified
+		// [3] AllowedTopologies with multiple terms specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Node_with_Zone_labels_and_Multiple_Allowed_Topology_terms_Match",
+			Node:         nodeWithZoneLabels,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneY"},
+						},
+					},
+				},
+			},
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+
+		// Select zones from AllowedTopologies [Pass]
+		// [1] nil Node
+		// [2] no Zone/Zones parametes specified
+		// [3] AllowedTopologies with multiple terms specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Nil_Node_and_Multiple_Allowed_Topology_terms_Superset",
+			Node:         nil,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneY"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneZ"},
+						},
+					},
+				},
+			},
+			Reject:        false,
+			ExpectedZones: "zoneX,zoneY,zoneZ",
+		},
+
+		// Select zones from AllowedTopologies [Pass]
+		// [1] nil Node
+		// [2] no Zone/Zones parametes specified
+		// [3] AllowedTopologies with multiple terms specified
+		// [4] ReplicaCount specified
+		// [5] ZonesWithNodes irrelevant
+		{
+			Name:         "Nil_Node_and_Multiple_Allowed_Topology_terms_Match",
+			Node:         nil,
+			ReplicaCount: 2,
+			AllowedTopologies: []v1.TopologySelectorTerm{
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneX"},
+						},
+					},
+				},
+				{
+					MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{
+						{
+							Key:    kubeletapis.LabelZoneFailureDomain,
+							Values: []string{"zoneY"},
+						},
+					},
+				},
+			},
+			Reject:              false,
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+	}
+
+	for _, test := range tests {
+		var zonesParameter, zonesWithNodes sets.String
+		var err error
+
+		if test.Zones != "" {
+			zonesParameter, err = ZonesToSet(test.Zones)
+			if err != nil {
+				t.Errorf("Could not convert Zones to a set: %s. This is a test error %s", test.Zones, test.Name)
+				continue
+			}
+		}
+
+		if test.ZonesWithNodes != "" {
+			zonesWithNodes, err = ZonesToSet(test.ZonesWithNodes)
+			if err != nil {
+				t.Errorf("Could not convert specified ZonesWithNodes to a set: %s. This is a test error %s", test.ZonesWithNodes, test.Name)
+				continue
+			}
+		}
+
+		zones, err := SelectZonesForVolume(test.ZonePresent, test.ZonesPresent, test.Zone, zonesParameter, zonesWithNodes, test.Node, test.AllowedTopologies, test.Name, test.ReplicaCount)
+
+		if test.Reject && err == nil {
+			t.Errorf("Unexpected zones from SelectZonesForVolume in %s. Zones: %v", test.Name, zones)
+			continue
+		}
+
+		if !test.Reject {
+			if err != nil {
+				t.Errorf("Unexpected error from SelectZonesForVolume in %s; Error: %v", test.Name, err)
+				continue
+			}
+
+			if uint32(zones.Len()) != test.ReplicaCount {
+				t.Errorf("Number of elements in returned zones %v does not equal replica count %d in %s", zones, test.ReplicaCount, test.Name)
+				continue
+			}
+
+			expectedZones, err := ZonesToSet(test.ExpectedZones)
+			if err != nil {
+				t.Errorf("Could not convert ExpectedZones to a set: %v. This is a test error in %s", test.ExpectedZones, test.Name)
+				continue
+			}
+
+			if test.ExpectSpecificZones && !zones.Equal(expectedZones) {
+				t.Errorf("Expected zones %v does not match obtained zones %v in %s", expectedZones, zones, test.Name)
+				continue
+			}
+
+			if test.ExpectSpecificZone && !zones.Has(test.ExpectedZone) {
+				t.Errorf("Expected zone %s not found in obtained zones %v in %s", test.ExpectedZone, zones, test.Name)
+				continue
+			}
+
+			if !expectedZones.IsSuperset(zones) {
+				t.Errorf("Obtained zones %v not subset of of expectedZones %v in %s", zones, expectedZones, test.Name)
+			}
+		}
+	}
+}
+
+func TestChooseZonesForVolumeIncludingZone(t *testing.T) {
+	tests := []struct {
+		// Parameters passed by test to chooseZonesForVolumeIncludingZone
+		Name          string
+		ReplicaCount  uint32
+		Zones         string
+		ZoneToInclude string
+		// Expectations around returned zones from chooseZonesForVolumeIncludingZone
+		Reject              bool   // expect error due to validation failing
+		ExpectSpecificZones bool   // expect set of returned zones to be equal to ExpectedZones (rather than subset of ExpectedZones)
+		ExpectedZones       string // set of zones that is a superset of returned zones or equal to returned zones (if ExpectSpecificZones is set)
+		ExpectSpecificZone  bool   // expect set of returned zones to include ExpectedZone as a member
+		ExpectedZone        string // zone that should be a member of returned zones
+	}{
+		// NEGATIVE TESTS
+
+		// Not enough zones specified to fit ReplicaCount [Fail]
+		{
+			Name:         "Too_Few_Zones",
+			ReplicaCount: 2,
+			Zones:        "zoneX",
+			Reject:       true,
+		},
+
+		// Invalid ReplicaCount [Fail]
+		{
+			Name:         "Zero_Replica_Count",
+			ReplicaCount: 0,
+			Zones:        "zoneY,zoneZ",
+			Reject:       true,
+		},
+
+		// Invalid ZoneToInclude [Fail]
+		{
+			Name:          "ZoneToInclude_Not_In_Zones_Single_replica_Dual_zones",
+			ReplicaCount:  1,
+			ZoneToInclude: "zoneX",
+			Zones:         "zoneY,zoneZ",
+			Reject:        true,
+		},
+
+		// Invalid ZoneToInclude [Fail]
+		{
+			Name:          "ZoneToInclude_Not_In_Zones_Single_replica_Single_zone",
+			ReplicaCount:  1,
+			ZoneToInclude: "zoneX",
+			Zones:         "zoneY",
+			Reject:        true,
+		},
+
+		// Invalid ZoneToInclude [Fail]
+		{
+			Name:          "ZoneToInclude_Not_In_Zones_Dual_replica_Multiple_zones",
+			ReplicaCount:  2,
+			ZoneToInclude: "zoneX",
+			Zones:         "zoneY,zoneZ,zoneW",
+			Reject:        true,
+		},
+
+		// POSITIVE TESTS
+
+		// Pick any one zone from Zones
+		{
+			Name:          "No_zones_to_include_and_Single_replica_and_Superset",
+			ReplicaCount:  1,
+			Zones:         "zoneX,zoneY,zoneZ",
+			Reject:        false,
+			ExpectedZones: "zoneX,zoneY,zoneZ",
+		},
+
+		// Pick any two zones from Zones
+		{
+			Name:          "No_zones_to_include_and_Dual_replicas_and_Superset",
+			ReplicaCount:  2,
+			Zones:         "zoneW,zoneX,zoneY,zoneZ",
+			Reject:        false,
+			ExpectedZones: "zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Pick the two zones from Zones
+		{
+			Name:                "No_zones_to_include_and_Dual_replicas_and_Match",
+			ReplicaCount:        2,
+			Zones:               "zoneX,zoneY",
+			Reject:              false,
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+
+		// Pick one zone from ZoneToInclude (other zones ignored)
+		{
+			Name:                "Include_zone_and_Single_replica_and_Match",
+			ReplicaCount:        1,
+			Zones:               "zoneW,zoneX,zoneY,zoneZ",
+			ZoneToInclude:       "zoneX",
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX",
+		},
+
+		// Pick one zone from ZoneToInclude (other zones ignored)
+		{
+			Name:                "Include_zone_and_single_replica_and_Match",
+			ReplicaCount:        1,
+			Zones:               "zoneX",
+			ZoneToInclude:       "zoneX",
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX",
+		},
+
+		// Pick one zone from Zones and the other from ZoneToInclude
+		{
+			Name:               "Include_zone_and_dual_replicas_and_Superset",
+			ReplicaCount:       2,
+			Zones:              "zoneW,zoneX,zoneY,zoneZ",
+			ZoneToInclude:      "zoneX",
+			Reject:             false,
+			ExpectSpecificZone: true,
+			ExpectedZone:       "zoneX",
+			ExpectedZones:      "zoneW,zoneX,zoneY,zoneZ",
+		},
+
+		// Pick one zone from Zones and the other from ZoneToInclude
+		{
+			Name:                "Include_zone_and_dual_replicas_and_Match",
+			ReplicaCount:        2,
+			Zones:               "zoneX,zoneY",
+			ZoneToInclude:       "zoneX",
+			Reject:              false,
+			ExpectSpecificZone:  true,
+			ExpectedZone:        "zoneX",
+			ExpectSpecificZones: true,
+			ExpectedZones:       "zoneX,zoneY",
+		},
+	}
+	for _, test := range tests {
+		zonesParameter, err := ZonesToSet(test.Zones)
+		if err != nil {
+			t.Errorf("Could not convert Zones to a set: %s. This is a test error %s", test.Zones, test.Name)
+			continue
+		}
+
+		zones, err := chooseZonesForVolumeIncludingZone(zonesParameter, test.Name, test.ZoneToInclude, test.ReplicaCount)
+		if test.Reject && err == nil {
+			t.Errorf("Unexpected zones from chooseZonesForVolumeIncludingZone in %s. Zones: %v", test.Name, zones)
+			continue
+		}
+		if !test.Reject {
+			if err != nil {
+				t.Errorf("Unexpected error from chooseZonesForVolumeIncludingZone in %s; Error: %v", test.Name, err)
+				continue
+			}
+
+			if uint32(zones.Len()) != test.ReplicaCount {
+				t.Errorf("Number of elements in returned zones %v does not equal replica count %d in %s", zones, test.ReplicaCount, test.Name)
+				continue
+			}
+
+			expectedZones, err := ZonesToSet(test.ExpectedZones)
+			if err != nil {
+				t.Errorf("Could not convert ExpectedZones to a set: %v. This is a test error in %s", test.ExpectedZones, test.Name)
+				continue
+			}
+
+			if test.ExpectSpecificZones && !zones.Equal(expectedZones) {
+				t.Errorf("Expected zones %v does not match obtained zones %v in %s", expectedZones, zones, test.Name)
+				continue
+			}
+
+			if test.ExpectSpecificZone && !zones.Has(test.ExpectedZone) {
+				t.Errorf("Expected zone %s not found in obtained zones %v in %s", test.ExpectedZone, zones, test.Name)
+				continue
+			}
+
+			if !expectedZones.IsSuperset(zones) {
+				t.Errorf("Obtained zones %v not subset of of expectedZones %v in %s", zones, expectedZones, test.Name)
+			}
+		}
+	}
+}
+
 func TestGetWindowsPath(t *testing.T) {
 	tests := []struct {
 		path         string