Modify VolumeZonePredicate to handle multi-zone PV

Modifies the VolumeZonePredicate to handle a PV that belongs to more
then one zone or region. This is indicated by the zone or region label
value containing a comma separated list.

pkg/cloudprovider/providers/aws/BUILD

@@ -31,6 +31,7 @@ go_library(
         "//pkg/credentialprovider/aws:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
         "//pkg/volume:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/aws/awserr:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/aws/credentials:go_default_library",

pkg/cloudprovider/providers/aws/aws.go

@@ -42,6 +42,8 @@ import (
 	"github.com/golang/glog"
 	"github.com/prometheus/client_golang/prometheus"
 
+	"path"
+
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -51,7 +53,7 @@ import (
 	"k8s.io/kubernetes/pkg/controller"
 	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	"k8s.io/kubernetes/pkg/volume"
-	"path"
+	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 )
 
 // ProviderName is the name of this cloud provider.
@@ -1806,7 +1808,7 @@ func (c *Cloud) CreateDisk(volumeOptions *VolumeOptions) (KubernetesVolumeID, er
 		createAZ = volume.ChooseZoneForVolume(allZones, volumeOptions.PVCName)
 	}
 	if !volumeOptions.ZonePresent && volumeOptions.ZonesPresent {
-		if adminSetOfZones, err := volume.ZonesToSet(volumeOptions.AvailabilityZones); err != nil {
+		if adminSetOfZones, err := volumeutil.ZonesToSet(volumeOptions.AvailabilityZones); err != nil {
 			return "", err
 		} else {
 			createAZ = volume.ChooseZoneForVolume(adminSetOfZones, volumeOptions.PVCName)

pkg/cloudprovider/providers/gce/BUILD

@@ -106,6 +106,7 @@ go_test(
         "//vendor/google.golang.org/api/googleapi:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
     ],
 )
 

pkg/kubelet/apis/well_known_labels.go

@@ -19,6 +19,7 @@ package apis
 const (
 	LabelHostname           = "kubernetes.io/hostname"
 	LabelZoneFailureDomain  = "failure-domain.beta.kubernetes.io/zone"
+	LabelMultiZoneDelimiter = "__"
 	LabelZoneRegion         = "failure-domain.beta.kubernetes.io/region"
 
 	LabelInstanceType = "beta.kubernetes.io/instance-type"

pkg/volume/gce_pd/BUILD

@@ -47,6 +47,7 @@ go_test(
         "//vendor/k8s.io/api/core/v1: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",
         "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//vendor/k8s.io/client-go/util/testing:go_default_library",
     ],

pkg/volume/gce_pd/gce_util.go

@@ -128,7 +128,7 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 
 	if !zonePresent && !zonesPresent && replicaZonesPresent {
 		// 001 - "replica-zones" specified
-		replicaZones, err := volume.ZonesToSet(configuredReplicaZones)
+		replicaZones, err := volumeutil.ZonesToSet(configuredReplicaZones)
 		if err != nil {
 			return "", 0, nil, "", err
 		}
@@ -161,7 +161,7 @@ func (gceutil *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner) (strin
 		} else if !zonePresent && zonesPresent {
 			// 010 - "zones" specified
 			// Pick a zone randomly selected from specified set.
-			if zones, err = volume.ZonesToSet(configuredZones); err != nil {
+			if zones, err = volumeutil.ZonesToSet(configuredZones); err != nil {
 				return "", 0, nil, "", err
 			}
 		} else if zonePresent && !zonesPresent {

pkg/volume/util.go

@@ -450,20 +450,6 @@ 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
-}
-
 // ValidateZone returns:
 // - an error in case zone is an empty string or contains only any combination of spaces and tab characters
 // - nil otherwise

pkg/volume/util/BUILD

@@ -30,6 +30,7 @@ go_library(
     deps = [
         "//pkg/api:go_default_library",
         "//pkg/api/v1/helper:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
         "//pkg/util/mount:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library",
@@ -69,9 +70,9 @@ go_test(
         "//pkg/api/v1/helper:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
     ] + select({
         "@io_bazel_rules_go//go/platform:linux_amd64": [
-            "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
             "//vendor/k8s.io/client-go/util/testing:go_default_library",
         ],
         "//conditions:default": [],

pkg/volume/util/util.go

@@ -22,15 +22,19 @@ import (
 	"os"
 	"path"
 
+	"strings"
+
 	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/pkg/api"
 	v1helper "k8s.io/kubernetes/pkg/api/v1/helper"
+	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	"k8s.io/kubernetes/pkg/util/mount"
 )
 
@@ -235,3 +239,34 @@ func LoadPodFromFile(filePath string) (*v1.Pod, error) {
 	}
 	return pod, nil
 }
+
+func ZonesSetToLabelValue(strSet sets.String) string {
+	return strings.Join(strSet.UnsortedList(), kubeletapis.LabelMultiZoneDelimiter)
+}
+
+// ZonesToSet converts a string containing a comma separated list of zones to set
+func ZonesToSet(zonesString string) (sets.String, error) {
+	return stringToSet(zonesString, ",")
+}
+
+// LabelZonesToSet converts a PV label value from string containing a delimited list of zones to set
+func LabelZonesToSet(labelZonesValue string) (sets.String, error) {
+	return stringToSet(labelZonesValue, kubeletapis.LabelMultiZoneDelimiter)
+}
+
+// StringToSet converts a string containing list separated by specified delimiter to to a set
+func stringToSet(str, delimiter string) (sets.String, error) {
+	zonesSlice := strings.Split(str, delimiter)
+	zonesSet := make(sets.String)
+	for _, zone := range zonesSlice {
+		trimmedZone := strings.TrimSpace(zone)
+		if trimmedZone == "" {
+			return make(sets.String), fmt.Errorf(
+				"%q separated list (%q) must not contain an empty string",
+				delimiter,
+				str)
+		}
+		zonesSet.Insert(trimmedZone)
+	}
+	return zonesSet, nil
+}

pkg/volume/util/util_test.go

@@ -23,6 +23,7 @@ import (
 
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	// util.go uses api.Codecs.LegacyCodec so import this package to do some
 	// resource initialization.
 	_ "k8s.io/kubernetes/pkg/api/install"
@@ -229,3 +230,36 @@ spec:
 		}
 	}
 }
+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)
+		}
+	}
+}

pkg/volume/util_test.go

@@ -867,40 +867,6 @@ func TestChooseZonesForVolume(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)
-		}
-	}
-}
-
 func TestValidateZone(t *testing.T) {
 	functionUnderTest := "ValidateZone"
 

plugin/pkg/scheduler/algorithm/predicates/predicates.go

@@ -435,7 +435,13 @@ func (c *VolumeZoneChecker) predicate(pod *v1.Pod, meta interface{}, nodeInfo *s
 					continue
 				}
 				nodeV, _ := nodeConstraints[k]
-				if v != nodeV {
+				volumeVSet, err := volumeutil.LabelZonesToSet(v)
+				if err != nil {
+					glog.Warningf("Failed to parse label for %q: %q. Ignoring the label. err=%v. ", k, v, err)
+					continue
+				}
+
+				if !volumeVSet.Has(nodeV) {
 					glog.V(10).Infof("Won't schedule pod %q onto node %q due to volume %q (mismatch on %q)", pod.Name, node.Name, pvName, k)
 					return false, []algorithm.PredicateFailureReason{ErrVolumeZoneConflict}, nil
 				}

plugin/pkg/scheduler/algorithm/predicates/predicates_test.go

@@ -3495,13 +3495,13 @@ func createPodWithVolume(pod, pv, pvc string) *v1.Pod {
 func TestVolumeZonePredicate(t *testing.T) {
 	pvInfo := FakePersistentVolumeInfo{
 		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_1", Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "zone_1"}},
+			ObjectMeta: metav1.ObjectMeta{Name: "Vol_1", Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-a"}},
 		},
 		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_2", Labels: map[string]string{kubeletapis.LabelZoneRegion: "zone_2", "uselessLabel": "none"}},
+			ObjectMeta: metav1.ObjectMeta{Name: "Vol_2", Labels: map[string]string{kubeletapis.LabelZoneRegion: "us-west1-b", "uselessLabel": "none"}},
 		},
 		{
-			ObjectMeta: metav1.ObjectMeta{Name: "Vol_3", Labels: map[string]string{kubeletapis.LabelZoneRegion: "zone_3"}},
+			ObjectMeta: metav1.ObjectMeta{Name: "Vol_3", Labels: map[string]string{kubeletapis.LabelZoneRegion: "us-west1-c"}},
 		},
 	}
 
@@ -3538,7 +3538,7 @@ func TestVolumeZonePredicate(t *testing.T) {
 			Node: &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "host1",
-					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "zone_1"},
+					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-a"},
 				},
 			},
 			Fits: true,
@@ -3559,7 +3559,7 @@ func TestVolumeZonePredicate(t *testing.T) {
 			Node: &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "host1",
-					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "zone_1", "uselessLabel": "none"},
+					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-a", "uselessLabel": "none"},
 				},
 			},
 			Fits: true,
@@ -3570,7 +3570,7 @@ func TestVolumeZonePredicate(t *testing.T) {
 			Node: &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "host1",
-					Labels: map[string]string{kubeletapis.LabelZoneRegion: "zone_2", "uselessLabel": "none"},
+					Labels: map[string]string{kubeletapis.LabelZoneRegion: "us-west1-b", "uselessLabel": "none"},
 				},
 			},
 			Fits: true,
@@ -3581,7 +3581,7 @@ func TestVolumeZonePredicate(t *testing.T) {
 			Node: &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "host1",
-					Labels: map[string]string{kubeletapis.LabelZoneRegion: "no_zone_2", "uselessLabel": "none"},
+					Labels: map[string]string{kubeletapis.LabelZoneRegion: "no_us-west1-b", "uselessLabel": "none"},
 				},
 			},
 			Fits: false,
@@ -3592,7 +3592,100 @@ func TestVolumeZonePredicate(t *testing.T) {
 			Node: &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:   "host1",
-					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "no_zone_1", "uselessLabel": "none"},
+					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "no_us-west1-a", "uselessLabel": "none"},
+				},
+			},
+			Fits: false,
+		},
+	}
+
+	expectedFailureReasons := []algorithm.PredicateFailureReason{ErrVolumeZoneConflict}
+
+	for _, test := range tests {
+		fit := NewVolumeZonePredicate(pvInfo, pvcInfo)
+		node := &schedulercache.NodeInfo{}
+		node.SetNode(test.Node)
+
+		fits, reasons, err := fit(test.Pod, nil, node)
+		if err != nil {
+			t.Errorf("%s: unexpected error: %v", test.Name, err)
+		}
+		if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) {
+			t.Errorf("%s: unexpected failure reasons: %v, want: %v", test.Name, reasons, expectedFailureReasons)
+		}
+		if fits != test.Fits {
+			t.Errorf("%s: expected %v got %v", test.Name, test.Fits, fits)
+		}
+
+	}
+}
+
+func TestVolumeZonePredicateMultiZone(t *testing.T) {
+	pvInfo := FakePersistentVolumeInfo{
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "Vol_1", Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-a"}},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "Vol_2", Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-b", "uselessLabel": "none"}},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "Vol_3", Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-c__us-west1-a"}},
+		},
+	}
+
+	pvcInfo := FakePersistentVolumeClaimInfo{
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "PVC_1", Namespace: "default"},
+			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_1"},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "PVC_2", Namespace: "default"},
+			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_2"},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "PVC_3", Namespace: "default"},
+			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_3"},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: "PVC_4", Namespace: "default"},
+			Spec:       v1.PersistentVolumeClaimSpec{VolumeName: "Vol_not_exist"},
+		},
+	}
+
+	tests := []struct {
+		Name string
+		Pod  *v1.Pod
+		Fits bool
+		Node *v1.Node
+	}{
+		{
+			Name: "node without labels",
+			Pod:  createPodWithVolume("pod_1", "Vol_3", "PVC_3"),
+			Node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "host1",
+				},
+			},
+			Fits: true,
+		},
+		{
+			Name: "label zone failure domain matched",
+			Pod:  createPodWithVolume("pod_1", "Vol_3", "PVC_3"),
+			Node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "host1",
+					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-a", "uselessLabel": "none"},
+				},
+			},
+			Fits: true,
+		},
+		{
+			Name: "label zone failure domain failed match",
+			Pod:  createPodWithVolume("pod_1", "vol_1", "PVC_1"),
+			Node: &v1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "host1",
+					Labels: map[string]string{kubeletapis.LabelZoneFailureDomain: "us-west1-b", "uselessLabel": "none"},
 				},
 			},
 			Fits: false,