diff --git a/pkg/cloudprovider/providers/aws/BUILD b/pkg/cloudprovider/providers/aws/BUILD index eadeaa1f3c..eb41175c31 100644 --- a/pkg/cloudprovider/providers/aws/BUILD +++ b/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", diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 77e2689d4d..c41c15f0d0 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/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) diff --git a/pkg/cloudprovider/providers/gce/BUILD b/pkg/cloudprovider/providers/gce/BUILD index 32cd13fd40..81e169f642 100644 --- a/pkg/cloudprovider/providers/gce/BUILD +++ b/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", ], ) diff --git a/pkg/kubelet/apis/well_known_labels.go b/pkg/kubelet/apis/well_known_labels.go index b3d0be7275..5a0db552c8 100644 --- a/pkg/kubelet/apis/well_known_labels.go +++ b/pkg/kubelet/apis/well_known_labels.go @@ -17,9 +17,10 @@ limitations under the License. package apis const ( - LabelHostname = "kubernetes.io/hostname" - LabelZoneFailureDomain = "failure-domain.beta.kubernetes.io/zone" - LabelZoneRegion = "failure-domain.beta.kubernetes.io/region" + 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" diff --git a/pkg/volume/gce_pd/BUILD b/pkg/volume/gce_pd/BUILD index a3d28e7a1f..eb65ce3540 100644 --- a/pkg/volume/gce_pd/BUILD +++ b/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", ], diff --git a/pkg/volume/gce_pd/gce_util.go b/pkg/volume/gce_pd/gce_util.go index e1b181d302..026324e630 100644 --- a/pkg/volume/gce_pd/gce_util.go +++ b/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 { diff --git a/pkg/volume/util.go b/pkg/volume/util.go index acc8ca283e..b976ce947f 100644 --- a/pkg/volume/util.go +++ b/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 diff --git a/pkg/volume/util/BUILD b/pkg/volume/util/BUILD index 6c2c7ac2f1..445cb425fb 100644 --- a/pkg/volume/util/BUILD +++ b/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": [], diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 0d402bf194..976ad96890 100644 --- a/pkg/volume/util/util.go +++ b/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 +} diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 280895cee7..480e576273 100644 --- a/pkg/volume/util/util_test.go +++ b/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) + } + } +} diff --git a/pkg/volume/util_test.go b/pkg/volume/util_test.go index 28c0f7524f..6eeb00a754 100644 --- a/pkg/volume/util_test.go +++ b/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" diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index dc811b8e80..427700b530 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/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 } diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index 383107861a..4f5c877a86 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/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,