diff --git a/pkg/cloudprovider/providers/aws/BUILD b/pkg/cloudprovider/providers/aws/BUILD index 1c901bd97d..01b55fcd3c 100644 --- a/pkg/cloudprovider/providers/aws/BUILD +++ b/pkg/cloudprovider/providers/aws/BUILD @@ -28,7 +28,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/cloudprovider/providers/aws", deps = [ "//pkg/credentialprovider/aws:go_default_library", - "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", @@ -44,6 +43,8 @@ go_library( "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/node/helpers:go_default_library", "//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/errors: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", @@ -78,11 +79,11 @@ go_test( ], embed = [":go_default_library"], deps = [ - "//pkg/volume:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library", "//vendor/github.com/aws/aws-sdk-go/service/elb:go_default_library", diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 94c3a8ef25..98ec2d7fe3 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -60,7 +60,8 @@ import ( cloudprovider "k8s.io/cloud-provider" nodehelpers "k8s.io/cloud-provider/node/helpers" servicehelpers "k8s.io/cloud-provider/service/helpers" - "k8s.io/kubernetes/pkg/volume" + cloudvolume "k8s.io/cloud-provider/volume" + volerr "k8s.io/cloud-provider/volume/errors" volumeutil "k8s.io/kubernetes/pkg/volume/util" ) @@ -2035,7 +2036,7 @@ func (d *awsDisk) deleteVolume() (bool, error) { } if awsError, ok := err.(awserr.Error); ok { if awsError.Code() == "VolumeInUse" { - return false, volume.NewDeletedVolumeInUseError(err.Error()) + return false, volerr.NewDeletedVolumeInUseError(err.Error()) } } return false, fmt.Errorf("error deleting EBS volume %q: %q", d.awsID, err) @@ -2424,7 +2425,7 @@ func (c *Cloud) checkIfAvailable(disk *awsDisk, opName string, instance string) devicePath := aws.StringValue(attachment.Device) nodeName := mapInstanceToNodeName(attachedInstance) - danglingErr := volumeutil.NewDanglingError(attachErr, nodeName, devicePath) + danglingErr := volerr.NewDanglingError(attachErr, nodeName, devicePath) return false, danglingErr } @@ -2443,7 +2444,7 @@ func (c *Cloud) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) } // Ignore any volumes that are being provisioned - if pv.Spec.AWSElasticBlockStore.VolumeID == volume.ProvisionedVolumeName { + if pv.Spec.AWSElasticBlockStore.VolumeID == cloudvolume.ProvisionedVolumeName { return nil, nil } diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index 613d1baefa..d37630e837 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -35,7 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/kubernetes/pkg/volume" + cloudvolume "k8s.io/cloud-provider/volume" ) const TestClusterID = "clusterid.test" @@ -1221,7 +1221,7 @@ func TestGetLabelsForVolume(t *testing.T) { Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ AWSElasticBlockStore: &v1.AWSElasticBlockStoreVolumeSource{ - VolumeID: volume.ProvisionedVolumeName, + VolumeID: cloudvolume.ProvisionedVolumeName, }, }, }, diff --git a/pkg/cloudprovider/providers/azure/BUILD b/pkg/cloudprovider/providers/azure/BUILD index b0f6e887d2..bf805343e8 100644 --- a/pkg/cloudprovider/providers/azure/BUILD +++ b/pkg/cloudprovider/providers/azure/BUILD @@ -37,7 +37,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/cloudprovider/providers/azure", deps = [ "//pkg/cloudprovider/providers/azure/auth:go_default_library", - "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", @@ -57,6 +56,8 @@ go_library( "//staging/src/k8s.io/client-go/util/flowcontrol:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/errors:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2017-09-01/network:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage:go_default_library", diff --git a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go index 7cb8a756df..1ff0a892cd 100644 --- a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go @@ -31,10 +31,10 @@ import ( azstorage "github.com/Azure/azure-sdk-for-go/storage" "github.com/Azure/go-autorest/autorest/to" "github.com/rubiojr/go-vhd/vhd" - "k8s.io/klog" kwait "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/kubernetes/pkg/volume" + volerr "k8s.io/cloud-provider/volume/errors" + "k8s.io/klog" ) const ( @@ -119,7 +119,7 @@ func (c *BlobDiskController) DeleteVolume(diskURI string) error { if strings.Contains(detail, errLeaseIDMissing) { // disk is still being used // see https://msdn.microsoft.com/en-us/library/microsoft.windowsazure.storage.blob.protocol.bloberrorcodestrings.leaseidmissing.aspx - return volume.NewDeletedVolumeInUseError(fmt.Sprintf("disk %q is still in use while being deleted", diskURI)) + return volerr.NewDeletedVolumeInUseError(fmt.Sprintf("disk %q is still in use while being deleted", diskURI)) } return fmt.Errorf("failed to delete vhd %v, account %s, blob %s, err: %v", diskURI, accountName, blob, err) } diff --git a/pkg/cloudprovider/providers/azure/azure_managedDiskController.go b/pkg/cloudprovider/providers/azure/azure_managedDiskController.go index 1e296ba475..200ff94bc4 100644 --- a/pkg/cloudprovider/providers/azure/azure_managedDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_managedDiskController.go @@ -30,7 +30,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" kwait "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/kubernetes/pkg/volume" + cloudvolume "k8s.io/cloud-provider/volume" "k8s.io/kubernetes/pkg/volume/util" ) @@ -281,7 +281,7 @@ func (c *Cloud) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) } // Ignore any volumes that are being provisioned - if pv.Spec.AzureDisk.DiskName == volume.ProvisionedVolumeName { + if pv.Spec.AzureDisk.DiskName == cloudvolume.ProvisionedVolumeName { return nil, nil } diff --git a/pkg/cloudprovider/providers/gce/BUILD b/pkg/cloudprovider/providers/gce/BUILD index efd9e148dc..0548c0078d 100644 --- a/pkg/cloudprovider/providers/gce/BUILD +++ b/pkg/cloudprovider/providers/gce/BUILD @@ -41,7 +41,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/cloudprovider/providers/gce", visibility = ["//visibility:public"], deps = [ - "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", @@ -66,6 +65,9 @@ go_library( "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/features:go_default_library", "//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/errors:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/cloud.google.com/go/compute/metadata:go_default_library", "//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud:go_default_library", "//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/filter:go_default_library", diff --git a/pkg/cloudprovider/providers/gce/gce_disks.go b/pkg/cloudprovider/providers/gce/gce_disks.go index 8968b1fbc3..43a1cc20d9 100644 --- a/pkg/cloudprovider/providers/gce/gce_disks.go +++ b/pkg/cloudprovider/providers/gce/gce_disks.go @@ -29,7 +29,9 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" cloudprovider "k8s.io/cloud-provider" - "k8s.io/kubernetes/pkg/volume" + cloudvolume "k8s.io/cloud-provider/volume" + volerr "k8s.io/cloud-provider/volume/errors" + volumehelpers "k8s.io/cloud-provider/volume/helpers" volumeutil "k8s.io/kubernetes/pkg/volume/util" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" @@ -504,7 +506,7 @@ func newDiskMetricContextRegional(request, region string) *metricContext { // GetLabelsForVolume retrieved the label info for the provided volume func (g *Cloud) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned - if pv.Spec.GCEPersistentDisk.PDName == volume.ProvisionedVolumeName { + if pv.Spec.GCEPersistentDisk.PDName == cloudvolume.ProvisionedVolumeName { return nil, nil } @@ -730,7 +732,7 @@ func getDiskType(diskType string) (string, error) { func (g *Cloud) DeleteDisk(diskToDelete string) error { err := g.doDeleteDisk(diskToDelete) if isGCEError(err, "resourceInUseByAnotherResource") { - return volume.NewDeletedVolumeInUseError(err.Error()) + return volerr.NewDeletedVolumeInUseError(err.Error()) } if err == cloudprovider.DiskNotFound { @@ -811,7 +813,7 @@ func (g *Cloud) GetAutoLabelsForPD(name string, zone string) (map[string]string, // However it is more consistent to ensure the disk exists, // and in future we may gather addition information (e.g. disk type, IOPS etc) if utilfeature.DefaultFeatureGate.Enabled(cloudfeatures.GCERegionalPersistentDisk) { - zoneSet, err := volumeutil.LabelZonesToSet(zone) + zoneSet, err := volumehelpers.LabelZonesToSet(zone) if err != nil { klog.Warningf("Failed to parse zone field: %q. Will use raw field.", zone) } @@ -852,7 +854,7 @@ func (g *Cloud) GetAutoLabelsForPD(name string, zone string) (map[string]string, return nil, fmt.Errorf("PD is regional but does not have any replicaZones specified: %v", disk) } labels[v1.LabelZoneFailureDomain] = - volumeutil.ZonesSetToLabelValue(zoneInfo.replicaZones) + volumehelpers.ZonesSetToLabelValue(zoneInfo.replicaZones) labels[v1.LabelZoneRegion] = disk.Region case nil: // Unexpected, but sanity-check diff --git a/pkg/cloudprovider/providers/openstack/BUILD b/pkg/cloudprovider/providers/openstack/BUILD index 61b99b62cf..a0a96dece6 100644 --- a/pkg/cloudprovider/providers/openstack/BUILD +++ b/pkg/cloudprovider/providers/openstack/BUILD @@ -21,7 +21,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/cloudprovider/providers/openstack", deps = [ "//pkg/util/mount:go_default_library", - "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", @@ -33,6 +32,8 @@ go_library( "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/node/helpers:go_default_library", "//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/errors:go_default_library", "//vendor/github.com/gophercloud/gophercloud:go_default_library", "//vendor/github.com/gophercloud/gophercloud/openstack:go_default_library", "//vendor/github.com/gophercloud/gophercloud/openstack/blockstorage/extensions/volumeactions:go_default_library", diff --git a/pkg/cloudprovider/providers/openstack/openstack_volumes.go b/pkg/cloudprovider/providers/openstack/openstack_volumes.go index b6b0fc5976..410b985c6f 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_volumes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_volumes.go @@ -30,7 +30,8 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" - k8s_volume "k8s.io/kubernetes/pkg/volume" + cloudvolume "k8s.io/cloud-provider/volume" + volerr "k8s.io/cloud-provider/volume/errors" volumeutil "k8s.io/kubernetes/pkg/volume/util" "github.com/gophercloud/gophercloud" @@ -344,7 +345,7 @@ func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) { } // using volume.AttachedDevice may cause problems because cinder does not report device path correctly see issue #33128 devicePath := volume.AttachedDevice - danglingErr := volumeutil.NewDanglingError(attachErr, nodeName, devicePath) + danglingErr := volerr.NewDanglingError(attachErr, nodeName, devicePath) klog.V(2).Infof("Found dangling volume %s attached to node %s", volumeID, nodeName) return "", danglingErr } @@ -571,7 +572,7 @@ func (os *OpenStack) DeleteVolume(volumeID string) error { } if used { msg := fmt.Sprintf("Cannot delete the volume %q, it's still attached to a node", volumeID) - return k8s_volume.NewDeletedVolumeInUseError(msg) + return volerr.NewDeletedVolumeInUseError(msg) } volumes, err := os.volumeService("") @@ -702,7 +703,7 @@ func (os *OpenStack) GetLabelsForVolume(ctx context.Context, pv *v1.PersistentVo } // Ignore any volumes that are being provisioned - if pv.Spec.Cinder.VolumeID == k8s_volume.ProvisionedVolumeName { + if pv.Spec.Cinder.VolumeID == cloudvolume.ProvisionedVolumeName { return nil, nil } diff --git a/pkg/controller/cloud/BUILD b/pkg/controller/cloud/BUILD index 1348d836c5..1997df8099 100644 --- a/pkg/controller/cloud/BUILD +++ b/pkg/controller/cloud/BUILD @@ -21,7 +21,6 @@ go_library( "//pkg/kubelet/apis:go_default_library", "//pkg/scheduler/api:go_default_library", "//pkg/util/node:go_default_library", - "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", @@ -42,6 +41,7 @@ go_library( "//staging/src/k8s.io/client-go/util/retry:go_default_library", "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], ) @@ -60,7 +60,6 @@ go_test( "//pkg/controller/testutil:go_default_library", "//pkg/kubelet/apis:go_default_library", "//pkg/scheduler/api:go_default_library", - "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", @@ -73,6 +72,7 @@ go_test( "//staging/src/k8s.io/client-go/testing:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], diff --git a/pkg/controller/cloud/pvlcontroller.go b/pkg/controller/cloud/pvlcontroller.go index bf81539448..3bb7dc72a3 100644 --- a/pkg/controller/cloud/pvlcontroller.go +++ b/pkg/controller/cloud/pvlcontroller.go @@ -38,9 +38,9 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" cloudprovider "k8s.io/cloud-provider" + volumehelpers "k8s.io/cloud-provider/volume/helpers" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/controller" - volumeutil "k8s.io/kubernetes/pkg/volume/util" ) // PersistentVolumeLabelController handles adding labels to persistent volumes when they are created @@ -210,7 +210,7 @@ func (pvlc *PersistentVolumeLabelController) createPatch(vol *v1.PersistentVolum if populateAffinity { var values []string if k == v1.LabelZoneFailureDomain { - zones, err := volumeutil.LabelZonesToSet(v) + zones, err := volumehelpers.LabelZonesToSet(v) if err != nil { return nil, fmt.Errorf("failed to convert label string for Zone: %s to a Set", v) } diff --git a/pkg/controller/cloud/pvlcontroller_test.go b/pkg/controller/cloud/pvlcontroller_test.go index 280af04a95..6be5447703 100644 --- a/pkg/controller/cloud/pvlcontroller_test.go +++ b/pkg/controller/cloud/pvlcontroller_test.go @@ -29,7 +29,7 @@ import ( sets "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" - volumeutil "k8s.io/kubernetes/pkg/volume/util" + volumehelpers "k8s.io/cloud-provider/volume/helpers" fakecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/fake" ) @@ -368,7 +368,7 @@ func TestCreatePatch(t *testing.T) { }, } - zones, _ := volumeutil.ZonesToSet("1,2,3") + zones, _ := volumehelpers.ZonesToSet("1,2,3") testCases := map[string]struct { vol v1.PersistentVolume labels map[string]string @@ -416,12 +416,12 @@ func TestCreatePatch(t *testing.T) { }, "cloudprovider multizone": { vol: awsPV, - labels: map[string]string{v1.LabelZoneFailureDomain: volumeutil.ZonesSetToLabelValue(zones)}, + labels: map[string]string{v1.LabelZoneFailureDomain: volumehelpers.ZonesSetToLabelValue(zones)}, expectedAffinity: &expectedAffinityZonesMergedWithAWSPV, }, "cloudprovider multizone pre-existing affinity non-conflicting": { vol: awsPVWithAffinity, - labels: map[string]string{v1.LabelZoneFailureDomain: volumeutil.ZonesSetToLabelValue(zones)}, + labels: map[string]string{v1.LabelZoneFailureDomain: volumehelpers.ZonesSetToLabelValue(zones)}, expectedAffinity: &expectedAffinityZonesMergedWithAWSPVWithAffinity, }, } diff --git a/pkg/controller/volume/persistentvolume/BUILD b/pkg/controller/volume/persistentvolume/BUILD index 4bf32fe7af..6f46984a64 100644 --- a/pkg/controller/volume/persistentvolume/BUILD +++ b/pkg/controller/volume/persistentvolume/BUILD @@ -59,6 +59,7 @@ go_library( "//staging/src/k8s.io/client-go/tools/reference:go_default_library", "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/errors:go_default_library", "//staging/src/k8s.io/csi-api/pkg/client/clientset/versioned:go_default_library", "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index 16d1ee02d1..5a3151b3e6 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -38,6 +38,7 @@ import ( ref "k8s.io/client-go/tools/reference" "k8s.io/client-go/util/workqueue" cloudprovider "k8s.io/cloud-provider" + volerr "k8s.io/cloud-provider/volume/errors" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/controller/volume/events" "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/metrics" @@ -1177,7 +1178,7 @@ func (ctrl *PersistentVolumeController) deleteVolumeOperation(volume *v1.Persist if err != nil { // Delete failed, update the volume and emit an event. klog.V(3).Infof("deletion of volume %q failed: %v", volume.Name, err) - if vol.IsDeletedVolumeInUse(err) { + if volerr.IsDeletedVolumeInUse(err) { // The plugin needs more time, don't mark the volume as Failed // and send Normal event only ctrl.eventRecorder.Event(volume, v1.EventTypeNormal, events.VolumeDelete, err.Error()) diff --git a/pkg/scheduler/algorithm/predicates/BUILD b/pkg/scheduler/algorithm/predicates/BUILD index 778ec193fb..2e2853ccf9 100644 --- a/pkg/scheduler/algorithm/predicates/BUILD +++ b/pkg/scheduler/algorithm/predicates/BUILD @@ -40,6 +40,7 @@ go_library( "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/client-go/listers/storage/v1:go_default_library", "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index f0fc2807db..ec30d9a288 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -36,6 +36,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" corelisters "k8s.io/client-go/listers/core/v1" storagelisters "k8s.io/client-go/listers/storage/v1" + volumehelpers "k8s.io/cloud-provider/volume/helpers" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" v1qos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos" "k8s.io/kubernetes/pkg/features" @@ -691,7 +692,7 @@ func (c *VolumeZoneChecker) predicate(pod *v1.Pod, meta PredicateMetadata, nodeI continue } nodeV, _ := nodeConstraints[k] - volumeVSet, err := volumeutil.LabelZonesToSet(v) + volumeVSet, err := volumehelpers.LabelZonesToSet(v) if err != nil { klog.Warningf("Failed to parse label for %q: %q. Ignoring the label. err=%v. ", k, v, err) continue diff --git a/pkg/volume/awsebs/BUILD b/pkg/volume/awsebs/BUILD index 92bd082e7a..2186097271 100644 --- a/pkg/volume/awsebs/BUILD +++ b/pkg/volume/awsebs/BUILD @@ -30,6 +30,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/utils/strings:go_default_library", ], diff --git a/pkg/volume/awsebs/aws_util.go b/pkg/volume/awsebs/aws_util.go index 812c061c0f..67fe8d9cb4 100644 --- a/pkg/volume/awsebs/aws_util.go +++ b/pkg/volume/awsebs/aws_util.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" cloudprovider "k8s.io/cloud-provider" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/cloudprovider/providers/aws" "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" @@ -132,7 +133,7 @@ func (util *AWSDiskUtil) CreateVolume(c *awsElasticBlockStoreProvisioner, node * // returns volumeOptions for EBS based on storageclass parameters and node configuration func populateVolumeOptions(pluginName, pvcName string, capacityGB resource.Quantity, tags map[string]string, storageParams map[string]string, node *v1.Node, allowedTopologies []v1.TopologySelectorTerm, zonesWithNodes sets.String) (*aws.VolumeOptions, error) { - requestGiB, err := volumeutil.RoundUpToGiBInt(capacityGB) + requestGiB, err := volumehelpers.RoundUpToGiBInt(capacityGB) if err != nil { return nil, err } @@ -157,7 +158,7 @@ func populateVolumeOptions(pluginName, pvcName string, capacityGB resource.Quant zone = v case "zones": zonesPresent = true - zones, err = volumeutil.ZonesToSet(v) + zones, err = volumehelpers.ZonesToSet(v) if err != nil { return nil, fmt.Errorf("error parsing zones %s, must be strings separated by commas: %v", zones, err) } @@ -180,7 +181,7 @@ func populateVolumeOptions(pluginName, pvcName string, capacityGB resource.Quant } } - volumeOptions.AvailabilityZone, err = volumeutil.SelectZoneForVolume(zonePresent, zonesPresent, zone, zones, zonesWithNodes, node, allowedTopologies, pvcName) + volumeOptions.AvailabilityZone, err = volumehelpers.SelectZoneForVolume(zonePresent, zonesPresent, zone, zones, zonesWithNodes, node, allowedTopologies, pvcName) if err != nil { return nil, err } diff --git a/pkg/volume/azure_dd/BUILD b/pkg/volume/azure_dd/BUILD index 8888651b9c..d45498cd24 100644 --- a/pkg/volume/azure_dd/BUILD +++ b/pkg/volume/azure_dd/BUILD @@ -36,6 +36,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/pkg/volume/azure_dd/azure_provision.go b/pkg/volume/azure_dd/azure_provision.go index 15f543469a..ffe3e77e8b 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/cloudprovider/providers/azure" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" @@ -134,7 +135,7 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie // maxLength = 79 - (4 for ".vhd") = 75 name := util.GenerateVolumeName(p.options.ClusterName, p.options.PVName, 75) capacity := p.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] - requestGiB, err := util.RoundUpToGiBInt(capacity) + requestGiB, err := volumehelpers.RoundUpToGiBInt(capacity) if err != nil { return nil, err } @@ -162,7 +163,7 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie availabilityZone = v case "zones": zonesPresent = true - availabilityZones, err = util.ZonesToSet(v) + availabilityZones, err = volumehelpers.ZonesToSet(v) if err != nil { return nil, fmt.Errorf("error parsing zones %s, must be strings separated by commas: %v", v, err) } @@ -224,7 +225,7 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie } if availabilityZone != "" || availabilityZones.Len() != 0 || activeZones.Len() != 0 || len(allowedTopologies) != 0 { - selectedAvailabilityZone, err = util.SelectZoneForVolume(zonePresent, zonesPresent, availabilityZone, availabilityZones, activeZones, selectedNode, allowedTopologies, p.options.PVC.Name) + selectedAvailabilityZone, err = volumehelpers.SelectZoneForVolume(zonePresent, zonesPresent, availabilityZone, availabilityZones, activeZones, selectedNode, allowedTopologies, p.options.PVC.Name) if err != nil { return nil, err } diff --git a/pkg/volume/cinder/BUILD b/pkg/volume/cinder/BUILD index a594d03866..3c955991a0 100644 --- a/pkg/volume/cinder/BUILD +++ b/pkg/volume/cinder/BUILD @@ -32,6 +32,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", "//vendor/k8s.io/utils/keymutex:go_default_library", diff --git a/pkg/volume/cinder/cinder_util.go b/pkg/volume/cinder/cinder_util.go index c082dad0b4..61dc4ffcd5 100644 --- a/pkg/volume/cinder/cinder_util.go +++ b/pkg/volume/cinder/cinder_util.go @@ -30,6 +30,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" clientset "k8s.io/client-go/kubernetes" + cloudvolumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/volume" volutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/utils/exec" @@ -206,7 +207,7 @@ func (util *DiskUtil) CreateVolume(c *cinderVolumeProvisioner, node *v1.Node, al // if we did not get any zones, lets leave it blank and gophercloud will // use zone "nova" as default if len(zones) > 0 { - availability, err = volutil.SelectZoneForVolume(false, false, "", nil, zones, node, allowedTopologies, c.options.PVC.Name) + availability, err = cloudvolumehelpers.SelectZoneForVolume(false, false, "", nil, zones, node, allowedTopologies, c.options.PVC.Name) if err != nil { klog.V(2).Infof("error selecting zone for volume: %v", err) return "", 0, nil, "", err diff --git a/pkg/volume/gcepd/BUILD b/pkg/volume/gcepd/BUILD index 04ba262e9d..f3e9f90b7c 100644 --- a/pkg/volume/gcepd/BUILD +++ b/pkg/volume/gcepd/BUILD @@ -31,6 +31,8 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/features:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", "//vendor/k8s.io/utils/path:go_default_library", @@ -59,6 +61,8 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/util/testing:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/pkg/volume/gcepd/attacher_test.go b/pkg/volume/gcepd/attacher_test.go index 9ab284d009..c067f07c27 100644 --- a/pkg/volume/gcepd/attacher_test.go +++ b/pkg/volume/gcepd/attacher_test.go @@ -24,9 +24,9 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" + cloudvolume "k8s.io/cloud-provider/volume" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" - volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/apimachinery/pkg/types" "k8s.io/klog" @@ -284,7 +284,7 @@ func createPVSpec(name string, readOnly bool, zones []string) *volume.Spec { } if zones != nil { - zonesLabel := strings.Join(zones, volumeutil.LabelMultiZoneDelimiter) + zonesLabel := strings.Join(zones, cloudvolume.LabelMultiZoneDelimiter) spec.PersistentVolume.ObjectMeta.Labels = map[string]string{ v1.LabelZoneFailureDomain: zonesLabel, } diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index 1ed7661208..a39085efb6 100644 --- a/pkg/volume/gcepd/gce_pd.go +++ b/pkg/volume/gcepd/gce_pd.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" utilfeature "k8s.io/apiserver/pkg/util/feature" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/klog" gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" "k8s.io/kubernetes/pkg/features" @@ -544,7 +545,7 @@ func (c *gcePersistentDiskProvisioner) Provision(selectedNode *v1.Node, allowedT pv.Labels[k] = v var values []string if k == v1.LabelZoneFailureDomain { - values, err = util.LabelZonesToList(v) + values, err = volumehelpers.LabelZonesToList(v) if err != nil { return nil, fmt.Errorf("failed to convert label string for Zone: %s to a List: %v", v, err) } diff --git a/pkg/volume/gcepd/gce_pd_test.go b/pkg/volume/gcepd/gce_pd_test.go index 41f1b43c9e..b1724f7f69 100644 --- a/pkg/volume/gcepd/gce_pd_test.go +++ b/pkg/volume/gcepd/gce_pd_test.go @@ -29,11 +29,11 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" utiltesting "k8s.io/client-go/util/testing" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "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) { @@ -190,7 +190,7 @@ func TestPlugin(t *testing.T) { } cap := persistentSpec.Spec.Capacity[v1.ResourceStorage] size := cap.Value() - if size != 100*util.GIB { + if size != 100*volumehelpers.GiB { t.Errorf("Provision() returned unexpected volume size: %v", size) } @@ -216,7 +216,7 @@ func TestPlugin(t *testing.T) { 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") + zones, _ := volumehelpers.ZonesToSet("zone1,zone2") r, _ = getNodeSelectorRequirementWithKey(v1.LabelZoneFailureDomain, term) if r == nil { t.Errorf("NodeSelectorRequirement %s-in-%v not found in volume NodeAffinity", v1.LabelZoneFailureDomain, zones) diff --git a/pkg/volume/gcepd/gce_util.go b/pkg/volume/gcepd/gce_util.go index a1cb2cf9ed..a53eaf23da 100644 --- a/pkg/volume/gcepd/gce_util.go +++ b/pkg/volume/gcepd/gce_util.go @@ -29,6 +29,8 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" cloudprovider "k8s.io/cloud-provider" cloudfeatures "k8s.io/cloud-provider/features" + cloudvolume "k8s.io/cloud-provider/volume" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/klog" gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" "k8s.io/kubernetes/pkg/util/mount" @@ -99,7 +101,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. name := volumeutil.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 63) // GCE PD name can have up to 63 characters capacity := c.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] // GCE PDs are allocated in chunks of GiBs - requestGB := volumeutil.RoundUpToGiB(capacity) + requestGB := volumehelpers.RoundUpToGiB(capacity) // Apply Parameters. // Values for parameter "replication-type" are canonicalized to lower case. @@ -121,7 +123,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. configuredZone = v case "zones": zonesPresent = true - configuredZones, err = volumeutil.ZonesToSet(v) + configuredZones, err = volumehelpers.ZonesToSet(v) if err != nil { return "", 0, nil, "", err } @@ -152,7 +154,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. switch replicationType { case replicationTypeRegionalPD: - selectedZones, err := volumeutil.SelectZonesForVolume(zonePresent, zonesPresent, configuredZone, configuredZones, activezones, node, allowedTopologies, c.options.PVC.Name, maxRegionalPDZones) + selectedZones, err := volumehelpers.SelectZonesForVolume(zonePresent, zonesPresent, configuredZone, configuredZones, activezones, node, allowedTopologies, c.options.PVC.Name, maxRegionalPDZones) if err != nil { klog.V(2).Infof("Error selecting zones for regional GCE PD volume: %v", err) return "", 0, nil, "", err @@ -169,7 +171,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1. klog.V(2).Infof("Successfully created Regional GCE PD volume %s", name) case replicationTypeNone: - selectedZone, err := volumeutil.SelectZoneForVolume(zonePresent, zonesPresent, configuredZone, configuredZones, activezones, node, allowedTopologies, c.options.PVC.Name) + selectedZone, err := volumehelpers.SelectZoneForVolume(zonePresent, zonesPresent, configuredZone, configuredZones, activezones, node, allowedTopologies, c.options.PVC.Name) if err != nil { return "", 0, nil, "", err } @@ -354,7 +356,7 @@ func udevadmChangeToDrive(drivePath string) error { func isRegionalPD(spec *volume.Spec) bool { if spec.PersistentVolume != nil { zonesLabel := spec.PersistentVolume.Labels[v1.LabelZoneFailureDomain] - zones := strings.Split(zonesLabel, volumeutil.LabelMultiZoneDelimiter) + zones := strings.Split(zonesLabel, cloudvolume.LabelMultiZoneDelimiter) return len(zones) > 1 } return false diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 7625d86bbc..f0ca617485 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -188,12 +188,6 @@ type DeletableVolumePlugin interface { NewDeleter(spec *Spec) (Deleter, error) } -const ( - // Name of a volume in external cloud that is being provisioned and thus - // should be ignored by rest of Kubernetes. - ProvisionedVolumeName = "placeholder-for-provisioning" -) - // ProvisionableVolumePlugin is an extended interface of VolumePlugin and is // used to create volumes for the cluster. type ProvisionableVolumePlugin interface { diff --git a/pkg/volume/util/BUILD b/pkg/volume/util/BUILD index a23800eb26..dd48f977e1 100644 --- a/pkg/volume/util/BUILD +++ b/pkg/volume/util/BUILD @@ -9,7 +9,6 @@ go_library( "device_util_linux.go", "device_util_unsupported.go", "doc.go", - "error.go", "finalizer.go", "io_util.go", "metrics.go", diff --git a/pkg/volume/util/operationexecutor/BUILD b/pkg/volume/util/operationexecutor/BUILD index a4a97f9db8..919e5934ef 100644 --- a/pkg/volume/util/operationexecutor/BUILD +++ b/pkg/volume/util/operationexecutor/BUILD @@ -30,6 +30,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/errors:go_default_library", "//staging/src/k8s.io/csi-translation-lib:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index fdbb6f7fa8..e1add696a4 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -29,6 +29,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" + volerr "k8s.io/cloud-provider/volume/errors" csilib "k8s.io/csi-translation-lib" "k8s.io/klog" expandcache "k8s.io/kubernetes/pkg/controller/volume/expand/cache" @@ -316,7 +317,7 @@ func (og *operationGenerator) GenerateAttachVolumeFunc( volumeToAttach.VolumeSpec, volumeToAttach.NodeName) if attachErr != nil { - if derr, ok := attachErr.(*util.DanglingAttachError); ok { + if derr, ok := attachErr.(*volerr.DanglingAttachError); ok { addErr := actualStateOfWorld.MarkVolumeAsAttached( v1.UniqueVolumeName(""), volumeToAttach.VolumeSpec, diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 21c1956bcb..5417616272 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -18,14 +18,11 @@ package util import ( "fmt" - "hash/fnv" "io/ioutil" - "math/rand" "os" "path" "path/filepath" "reflect" - "strconv" "strings" v1 "k8s.io/api/core/v1" @@ -73,21 +70,8 @@ const ( // VolumeDynamicallyCreatedByKey is the key of the annotation on PersistentVolume // object created dynamically VolumeDynamicallyCreatedByKey = "kubernetes.io/createdby" - - // LabelMultiZoneDelimiter separates zones for volumes - LabelMultiZoneDelimiter = "__" ) -// VolumeZoneConfig contains config information about zonal volume. -type VolumeZoneConfig struct { - ZonePresent bool - ZonesPresent bool - ReplicaZoneFromNodePresent bool - Zone string - Zones string - ReplicaZoneFromNode string -} - // IsReady checks for the existence of a regular file // called 'ready' in the given directory and returns // true if that file exists. @@ -219,174 +203,6 @@ func LoadPodFromFile(filePath string) (*v1.Pod, error) { return pod, nil } -// SelectZoneForVolume is a wrapper around SelectZonesForVolume -// to select a single zone for a volume based on parameters -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 nil, fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time") - } - - var zoneFromNode string - // pick one zone from node if present - if node != nil { - // VolumeScheduling implicit since node is not nil - if zoneParameterPresent || zonesParameterPresent { - 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 for one of the replicas - var ok bool - zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] - if !ok { - return nil, fmt.Errorf("%s Label for node missing", v1.LabelZoneFailureDomain) - } - // if single replica volume and node with zone found, return immediately - if numReplicas == 1 { - return sets.NewString(zoneFromNode), nil - } - } - - // pick zone from allowedZones if specified - allowedZones, err := ZonesFromAllowedTopologies(allowedTopologies) - if err != nil { - return nil, err - } - - if (len(allowedTopologies) > 0) && (allowedZones.Len() == 0) { - return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomain) - } - - if allowedZones.Len() > 0 { - // VolumeScheduling implicit since allowedZones present - if zoneParameterPresent || zonesParameterPresent { - 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. - zones, err := chooseZonesForVolumeIncludingZone(allowedZones, pvcName, zoneFromNode, numReplicas) - if err != nil { - return nil, fmt.Errorf("cannot process zones in allowedTopologies: %v", err) - } - return zones, nil - } - - // pick zone from parameters if present - if zoneParameterPresent { - 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 { - 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 - if zonesWithNodes.Len() > 0 { - // If node != null (and thus zoneFromNode != ""), zoneFromNode will be member of zonesWithNodes - zones, err := chooseZonesForVolumeIncludingZone(zonesWithNodes, pvcName, zoneFromNode, numReplicas) - if err != nil { - return nil, fmt.Errorf("cannot process zones where nodes exist in the cluster: %v", err) - } - 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) { - zones := make(sets.String) - for _, term := range allowedTopologies { - for _, exp := range term.MatchLabelExpressions { - if exp.Key == v1.LabelZoneFailureDomain { - for _, value := range exp.Values { - zones.Insert(value) - } - } else { - return nil, fmt.Errorf("unsupported key found in matchLabelExpressions: %s", exp.Key) - } - } - } - return zones, nil -} - -// ZonesSetToLabelValue converts zones set to label value -func ZonesSetToLabelValue(strSet sets.String) string { - return strings.Join(strSet.UnsortedList(), LabelMultiZoneDelimiter) -} - -// ZonesToSet converts a string containing a comma separated list of zones to set -func ZonesToSet(zonesString string) (sets.String, error) { - 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 -func LabelZonesToSet(labelZonesValue string) (sets.String, error) { - return stringToSet(labelZonesValue, LabelMultiZoneDelimiter) -} - -// StringToSet converts a string containing list separated by specified delimiter 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 -} - -// 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, 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 @@ -479,148 +295,6 @@ func GetPath(mounter volume.Mounter) (string, error) { return path, nil } -// ChooseZoneForVolume implements our heuristics for choosing a zone for volume creation based on the volume name -// Volumes are generally round-robin-ed across all active zones, using the hash of the PVC Name. -// However, if the PVCName ends with `-`, we will hash the prefix, and then add the integer to the hash. -// This means that a StatefulSet's volumes (`claimname-statefulsetname-id`) will spread across available zones, -// assuming the id values are consecutive. -func ChooseZoneForVolume(zones sets.String, pvcName string) string { - // No zones available, return empty string. - if zones.Len() == 0 { - return "" - } - - // 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() - zone := zoneSlice[(hash+index)%uint32(len(zoneSlice))] - - klog.V(2).Infof("Creating volume for PVC %q; chose zone=%q from zones=%q", pvcName, zone, zoneSlice) - 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 { - // No zones available, return empty set. - replicaZones := sets.NewString() - if zones.Len() == 0 { - return replicaZones - } - - // 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() - - startingIndex := index * numZones - for index = startingIndex; index < startingIndex+numZones; index++ { - zone := zoneSlice[(hash+index)%uint32(len(zoneSlice))] - replicaZones.Insert(zone) - } - - klog.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 - klog.Warningf("No name defined during volume create; choosing random zone") - - hash = rand.Uint32() - } else { - hashString := pvcName - - // Heuristic to make sure that volumes in a StatefulSet are spread across zones - // StatefulSet PVCs are (currently) named ClaimName-StatefulSetName-Id, - // where Id is an integer index. - // Note though that if a StatefulSet pod has multiple claims, we need them to be - // in the same zone, because otherwise the pod will be unable to mount both volumes, - // and will be unschedulable. So we hash _only_ the "StatefulSetName" portion when - // it looks like `ClaimName-StatefulSetName-Id`. - // We continue to round-robin volume names that look like `Name-Id` also; this is a useful - // feature for users that are creating statefulset-like functionality without using statefulsets. - lastDash := strings.LastIndexByte(pvcName, '-') - if lastDash != -1 { - statefulsetIDString := pvcName[lastDash+1:] - statefulsetID, err := strconv.ParseUint(statefulsetIDString, 10, 32) - if err == nil { - // Offset by the statefulsetID, so we round-robin across zones - index = uint32(statefulsetID) - // We still hash the volume name, but only the prefix - hashString = pvcName[:lastDash] - - // In the special case where it looks like `ClaimName-StatefulSetName-Id`, - // hash only the StatefulSetName, so that different claims on the same StatefulSet - // member end up in the same zone. - // Note that StatefulSetName (and ClaimName) might themselves both have dashes. - // We actually just take the portion after the final - of ClaimName-StatefulSetName. - // For our purposes it doesn't much matter (just suboptimal spreading). - lastDash := strings.LastIndexByte(hashString, '-') - if lastDash != -1 { - hashString = hashString[lastDash+1:] - } - - klog.V(2).Infof("Detected StatefulSet-style volume name %q; index=%d", pvcName, index) - } - } - - // We hash the (base) volume name, so we don't bias towards the first N zones - h := fnv.New32() - h.Write([]byte(hashString)) - hash = h.Sum32() - } - - return hash, index -} - // UnmountViaEmptyDir delegates the tear down operation for secret, configmap, git_repo and downwardapi // to empty_dir func UnmountViaEmptyDir(dir string, host volume.VolumeHost, volName string, volSpec volume.Spec, podUID utypes.UID) error { @@ -669,16 +343,6 @@ func JoinMountOptions(userOptions []string, systemOptions []string) []string { return allMountOptions.List() } -// ValidateZone returns: -// - an error in case zone is an empty string or contains only any combination of spaces and tab characters -// - nil otherwise -func ValidateZone(zone string) error { - if strings.TrimSpace(zone) == "" { - return fmt.Errorf("the provided %q zone is not valid, it's an empty string or contains only spaces and tab characters", zone) - } - return nil -} - // AccessModesContains returns whether the requested mode is contained by modes func AccessModesContains(modes []v1.PersistentVolumeAccessMode, mode v1.PersistentVolumeAccessMode) bool { for _, m := range modes { diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 4fa3dc95bc..3ad893f82a 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -23,11 +23,6 @@ import ( "testing" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/sets" - - // util.go uses api.Codecs.LegacyCodec so import this package to do some - // resource initialization. - "hash/fnv" _ "k8s.io/kubernetes/pkg/apis/core/install" @@ -317,39 +312,6 @@ 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) - } - } -} func TestCalculateTimeoutForVolume(t *testing.T) { pv := &v1.PersistentVolume{ @@ -470,1753 +432,6 @@ func createVolumeSpecWithMountOption(name string, mountOptions string, spec v1.P return &volume.Spec{PersistentVolume: pv} } -func checkFnv32(t *testing.T, s string, expected uint32) { - h := fnv.New32() - h.Write([]byte(s)) - h.Sum32() - - if h.Sum32() != expected { - t.Fatalf("hash of %q was %v, expected %v", s, h.Sum32(), expected) - } -} - -func TestChooseZoneForVolume(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 - Expected string - }{ - // Test for PVC names that don't have a dash - { - 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: sets.NewString("a", "b", "c"), - VolumeName: "henley-0", - Expected: "a", // hash("henley") == 0 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "henley-1", - Expected: "b", // hash("henley") + 1 == 1 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "henley-2", - Expected: "c", // hash("henley") + 2 == 2 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "henley-3", - Expected: "a", // hash("henley") + 3 == 3 === 0 mod 3 - }, - { - 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: sets.NewString("a", "b", "c"), - VolumeName: "henley-", - Expected: "c", // hash("henley-") = 2652299129 === 2 mod 3 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "henley-a", - Expected: "c", // hash("henley-a") = 1459735322 === 2 mod 3 - }, - { - 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: sets.NewString("a", "b", "c"), - VolumeName: "medium-henley-1", - Expected: "b", // hash("henley") + 1 == 1 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "loud-henley-1", - Expected: "b", // hash("henley") + 1 == 1 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "quiet-henley-2", - Expected: "c", // hash("henley") + 2 == 2 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "medium-henley-2", - Expected: "c", // hash("henley") + 2 == 2 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "medium-henley-3", - Expected: "a", // hash("henley") + 3 == 3 === 0 mod 3 - }, - { - 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: sets.NewString("a", "b", "c"), - VolumeName: "medium-alpha-henley-2", - Expected: "c", // hash("henley") + 2 == 2 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "medium-beta-henley-3", - Expected: "a", // hash("henley") + 3 == 3 === 0 mod 3 - }, - { - 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: sets.NewString("a", "b", "c"), - VolumeName: "medium-henley--2", - Expected: "a", // hash("") + 2 == 0 mod 3 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "medium-henley--3", - Expected: "b", // hash("") + 3 == 1 mod 3 - }, - { - Zones: sets.NewString("a", "b", "c"), - VolumeName: "medium-henley--4", - Expected: "c", // hash("") + 4 == 2 mod 3 - }, - // Test for no zones - { - Zones: sets.NewString(), - VolumeName: "medium-henley--1", - Expected: "", - }, - { - Zones: nil, - VolumeName: "medium-henley--2", - Expected: "", - }, - } - - for _, test := range tests { - actual := ChooseZoneForVolume(test.Zones, test.VolumeName) - - if 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"), - }, - // Test for no zones - { - Zones: sets.NewString(), - VolumeName: "henley-1", - Expected: sets.NewString(), - }, - { - Zones: nil, - VolumeName: "henley-2", - Expected: sets.NewString(), - }, - } - - 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 TestValidateZone(t *testing.T) { - functionUnderTest := "ValidateZone" - - // First part: want an error - errCases := []string{"", " "} - for _, errCase := range errCases { - if got := ValidateZone(errCase); got == nil { - t.Errorf("%v(%v) returned (%v), want (%v)", functionUnderTest, errCase, got, "an error") - } - } - - // Second part: want no error - succCases := []string{" us-east-1a "} - for _, succCase := range succCases { - if got := ValidateZone(succCase); got != nil { - t.Errorf("%v(%v) returned (%v), want (%v)", functionUnderTest, succCase, got, nil) - } - } -} - -func TestSelectZoneForVolume(t *testing.T) { - - nodeWithZoneLabels := &v1.Node{} - nodeWithZoneLabels.Labels = map[string]string{v1.LabelZoneFailureDomain: "zoneX"} - - nodeWithNoLabels := &v1.Node{} - - tests := []struct { - // Parameters passed by test to SelectZoneForVolume - Name string - ZonePresent bool - Zone string - ZonesPresent bool - Zones string - ZonesWithNodes string - Node *v1.Node - AllowedTopologies []v1.TopologySelectorTerm - // Expectations around returned zone from SelectZoneForVolume - Reject bool // expect error due to validation failing - ExpectSpecificZone bool // expect returned zone to specifically match a single zone (rather than one from a set) - ExpectedZone string // single zone that should perfectly match returned zone (requires ExpectSpecificZone to be true) - ExpectedZones string // set of zones one of whose members should match returned zone (requires ExpectSpecificZone to be false) - }{ - // NEGATIVE TESTS - - // Zone and Zones are both specified [Fail] - // [1] Node irrelevant - // [2] Zone and Zones parameters presents - // [3] AllowedTopologies 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 - { - 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 - { - 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 - { - 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 - { - Name: "Nil_Node_and_Zone_parameter_and_Allowed_Topology_term", - Node: nil, - ZonePresent: true, - Zone: "zoneX", - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - }, - Reject: true, - }, - - // Zones parameter as well as AllowedTopologies specified [Fail] - // [1] nil Node - // [2] Zones parameter specified - // [3] AllowedTopologies specified - { - Name: "Nil_Node_and_Zones_parameter_and_Allowed_Topology_term", - Node: nil, - ZonesPresent: true, - Zones: "zoneX,zoneY", - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.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 - { - Name: "Nil_Node_and_Invalid_Allowed_Topology_Key", - Node: nil, - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: "invalid_key", - Values: []string{"zoneX"}, - }, - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneY"}, - }, - }, - }, - }, - Reject: true, - }, - - // AllowedTopologies without keys specifying LabelZoneFailureDomain [Fail] - // [1] nil Node - // [2] no Zone/Zones parameter - // [3] Invalid AllowedTopologies - { - Name: "Nil_Node_and_Invalid_AllowedTopologies", - Node: nil, - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{}, - }, - }, - Reject: true, - }, - - // POSITIVE TESTS WITH VolumeScheduling ENABLED - - // Select zone from active zones [Pass] - // [1] nil Node - // [2] no Zone parameter specified - // [3] no AllowedTopologies - { - Name: "Nil_Node_and_No_Zone_Zones_parameter_and_no_Allowed_topologies_and_VolumeScheduling_enabled", - Node: nil, - ZonesWithNodes: "zoneX,zoneY", - Reject: false, - ExpectedZones: "zoneX,zoneY", - }, - - // Select zone from single zone parameter [Pass] - // [1] nil Node - // [2] Zone parameter specified - // [3] no AllowedTopology specified - { - Name: "Nil_Node_and_Zone_parameter_present_and_VolumeScheduling_enabled", - ZonePresent: true, - Zone: "zoneX", - Node: nil, - Reject: false, - ExpectSpecificZone: true, - ExpectedZone: "zoneX", - }, - - // Select zone from zones parameter [Pass] - // [1] nil Node - // [2] Zones parameter specified - // [3] no AllowedTopology - { - Name: "Nil_Node_and_Zones_parameter_present_and_VolumeScheduling_enabled", - ZonesPresent: true, - Zones: "zoneX,zoneY", - Node: nil, - Reject: false, - ExpectedZones: "zoneX,zoneY", - }, - - // Select zone from node label [Pass] - // [1] Node with zone labels - // [2] no zone/zones parameters - // [3] no AllowedTopology - { - Name: "Node_with_Zone_labels_and_VolumeScheduling_enabled", - Node: nodeWithZoneLabels, - Reject: false, - ExpectSpecificZone: true, - ExpectedZone: "zoneX", - }, - - // Select zone from node label [Pass] - // [1] Node with zone labels - // [2] no Zone/Zones parameters - // [3] AllowedTopology with single term with multiple values specified (ignored) - { - Name: "Node_with_Zone_labels_and_Multiple_Allowed_Topology_values_and_VolumeScheduling_enabled", - Node: nodeWithZoneLabels, - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneZ", "zoneY"}, - }, - }, - }, - }, - Reject: false, - ExpectSpecificZone: true, - ExpectedZone: "zoneX", - }, - - // Select Zone from AllowedTopologies [Pass] - // [1] nil Node - // [2] no Zone/Zones parametes specified - // [3] AllowedTopologies with single term with multiple values specified - { - Name: "Nil_Node_with_Multiple_Allowed_Topology_values_and_VolumeScheduling_enabled", - Node: nil, - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneX", "zoneY"}, - }, - }, - }, - }, - Reject: false, - ExpectedZones: "zoneX,zoneY", - }, - - // Select zone from AllowedTopologies [Pass] - // [1] nil Node - // [2] no Zone/Zones parametes specified - // [3] AllowedTopologies with multiple terms specified - { - Name: "Nil_Node_and_Multiple_Allowed_Topology_terms_and_VolumeScheduling_enabled", - Node: nil, - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneY"}, - }, - }, - }, - }, - Reject: false, - ExpectedZones: "zoneX,zoneY", - }, - - // Select Zone from AllowedTopologies [Pass] - // Note: Dual replica with same AllowedTopologies will fail: Nil_Node_and_Single_Allowed_Topology_term_value_and_Dual_replicas - // [1] nil Node - // [2] no Zone/Zones parametes specified - // [3] AllowedTopologies with single term and value specified - { - Name: "Nil_Node_and_Single_Allowed_Topology_term_value_and_VolumeScheduling_enabled", - Node: nil, - AllowedTopologies: []v1.TopologySelectorTerm{ - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - }, - Reject: false, - ExpectSpecificZone: true, - ExpectedZone: "zoneX", - }, - } - - for _, test := range tests { - t.Run(test.Name, func(t *testing.T) { - var zonesParameter, zonesWithNodes sets.String - var err error - - if test.Zones != "" { - zonesParameter, err = ZonesToSet(test.Zones) - if err != nil { - t.Fatalf("Could not convert Zones to a set: %s. This is a test error %s", test.Zones, test.Name) - } - } - - if test.ZonesWithNodes != "" { - zonesWithNodes, err = ZonesToSet(test.ZonesWithNodes) - if err != nil { - t.Fatalf("Could not convert specified ZonesWithNodes to a set: %s. This is a test error %s", test.ZonesWithNodes, test.Name) - } - } - - zone, err := SelectZoneForVolume(test.ZonePresent, test.ZonesPresent, test.Zone, zonesParameter, zonesWithNodes, test.Node, test.AllowedTopologies, test.Name) - - if test.Reject && err == nil { - t.Errorf("Unexpected zone from SelectZoneForVolume for %s", zone) - } - - if !test.Reject { - if err != nil { - t.Errorf("Unexpected error from SelectZoneForVolume for %s; Error: %v", test.Name, err) - } - - if test.ExpectSpecificZone == true { - if zone != test.ExpectedZone { - t.Errorf("Expected zone %v does not match obtained zone %v for %s", test.ExpectedZone, zone, test.Name) - } - } - - if test.ExpectedZones != "" { - expectedZones, err := ZonesToSet(test.ExpectedZones) - if err != nil { - t.Fatalf("Could not convert ExpectedZones to a set: %s. This is a test error", test.ExpectedZones) - } - if !expectedZones.Has(zone) { - t.Errorf("Obtained zone %s not member of expectedZones %s", zone, expectedZones) - } - } - } - }) - } -} - -func TestSelectZonesForVolume(t *testing.T) { - - nodeWithZoneLabels := &v1.Node{} - nodeWithZoneLabels.Labels = map[string]string{v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.LabelZoneFailureDomain, - Values: []string{"zoneV"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneW"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneY"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.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: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.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: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.LabelZoneFailureDomain, - Values: []string{"zoneY"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.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: v1.LabelZoneFailureDomain, - Values: []string{"zoneX"}, - }, - }, - }, - { - MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ - { - Key: v1.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 diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 013e0dc419..f597ba1662 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -260,28 +260,3 @@ type DeviceUnmounter interface { // unmounted. UnmountDevice(deviceMountPath string) error } - -// NewDeletedVolumeInUseError returns a new instance of DeletedVolumeInUseError -// error. -func NewDeletedVolumeInUseError(message string) error { - return deletedVolumeInUseError(message) -} - -type deletedVolumeInUseError string - -var _ error = deletedVolumeInUseError("") - -// IsDeletedVolumeInUse returns true if an error returned from Delete() is -// deletedVolumeInUseError -func IsDeletedVolumeInUse(err error) bool { - switch err.(type) { - case deletedVolumeInUseError: - return true - default: - return false - } -} - -func (err deletedVolumeInUseError) Error() string { - return string(err) -} diff --git a/plugin/pkg/admission/storage/persistentvolume/label/BUILD b/plugin/pkg/admission/storage/persistentvolume/label/BUILD index e5cb1db920..ba34eb7fb1 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/BUILD +++ b/plugin/pkg/admission/storage/persistentvolume/label/BUILD @@ -17,11 +17,11 @@ go_library( "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", - "//pkg/volume:go_default_library", - "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/cloud-provider:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/plugin/pkg/admission/storage/persistentvolume/label/admission.go b/plugin/pkg/admission/storage/persistentvolume/label/admission.go index 037dcd420a..7d993f525f 100644 --- a/plugin/pkg/admission/storage/persistentvolume/label/admission.go +++ b/plugin/pkg/admission/storage/persistentvolume/label/admission.go @@ -27,12 +27,12 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apiserver/pkg/admission" cloudprovider "k8s.io/cloud-provider" + cloudvolume "k8s.io/cloud-provider/volume" + volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/klog" api "k8s.io/kubernetes/pkg/apis/core" k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" - vol "k8s.io/kubernetes/pkg/volume" - volumeutil "k8s.io/kubernetes/pkg/volume/util" ) const ( @@ -153,7 +153,7 @@ func (l *persistentVolumeLabel) Admit(a admission.Attributes) (err error) { // Set NodeSelectorRequirements based on the labels var values []string if k == v1.LabelZoneFailureDomain { - zones, err := volumeutil.LabelZonesToSet(v) + zones, err := volumehelpers.LabelZonesToSet(v) if err != nil { return admission.NewForbidden(a, fmt.Errorf("failed to convert label string for Zone: %s to a Set", v)) } @@ -192,7 +192,7 @@ func (l *persistentVolumeLabel) Admit(a admission.Attributes) (err error) { func (l *persistentVolumeLabel) findAWSEBSLabels(volume *api.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned - if volume.Spec.AWSElasticBlockStore.VolumeID == vol.ProvisionedVolumeName { + if volume.Spec.AWSElasticBlockStore.VolumeID == cloudvolume.ProvisionedVolumeName { return nil, nil } pvlabler, err := l.getAWSPVLabeler() @@ -240,7 +240,7 @@ func (l *persistentVolumeLabel) getAWSPVLabeler() (cloudprovider.PVLabeler, erro func (l *persistentVolumeLabel) findGCEPDLabels(volume *api.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned - if volume.Spec.GCEPersistentDisk.PDName == vol.ProvisionedVolumeName { + if volume.Spec.GCEPersistentDisk.PDName == cloudvolume.ProvisionedVolumeName { return nil, nil } @@ -315,7 +315,7 @@ func (l *persistentVolumeLabel) getAzurePVLabeler() (cloudprovider.PVLabeler, er func (l *persistentVolumeLabel) findAzureDiskLabels(volume *api.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned - if volume.Spec.AzureDisk.DiskName == vol.ProvisionedVolumeName { + if volume.Spec.AzureDisk.DiskName == cloudvolume.ProvisionedVolumeName { return nil, nil } @@ -364,7 +364,7 @@ func (l *persistentVolumeLabel) getOpenStackPVLabeler() (cloudprovider.PVLabeler func (l *persistentVolumeLabel) findCinderDiskLabels(volume *api.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned - if volume.Spec.Cinder.VolumeID == vol.ProvisionedVolumeName { + if volume.Spec.Cinder.VolumeID == cloudvolume.ProvisionedVolumeName { return nil, nil } diff --git a/staging/publishing/import-restrictions.yaml b/staging/publishing/import-restrictions.yaml index 2ed1a8a1f6..135d86454b 100644 --- a/staging/publishing/import-restrictions.yaml +++ b/staging/publishing/import-restrictions.yaml @@ -190,6 +190,7 @@ - k8s.io/apimachinery - k8s.io/apiserver - k8s.io/client-go + - k8s.io/cloud-provider - k8s.io/klog - k8s.io/utils @@ -213,4 +214,5 @@ - k8s.io/api - k8s.io/apimachinery - k8s.io/klog + - k8s.io/cloud-provider - k8s.io/csi-translation-lib diff --git a/staging/publishing/rules.yaml b/staging/publishing/rules.yaml index d2f5735e39..250912eed5 100644 --- a/staging/publishing/rules.yaml +++ b/staging/publishing/rules.yaml @@ -989,3 +989,5 @@ rules: branch: master - repository: apimachinery branch: master + - repository: cloud-provider + branch: master diff --git a/staging/src/k8s.io/cloud-provider/BUILD b/staging/src/k8s.io/cloud-provider/BUILD index 7a5b27310b..15b2786595 100644 --- a/staging/src/k8s.io/cloud-provider/BUILD +++ b/staging/src/k8s.io/cloud-provider/BUILD @@ -38,6 +38,7 @@ filegroup( "//staging/src/k8s.io/cloud-provider/features:all-srcs", "//staging/src/k8s.io/cloud-provider/node:all-srcs", "//staging/src/k8s.io/cloud-provider/service/helpers:all-srcs", + "//staging/src/k8s.io/cloud-provider/volume:all-srcs", ], tags = ["automanaged"], ) diff --git a/staging/src/k8s.io/cloud-provider/volume/BUILD b/staging/src/k8s.io/cloud-provider/volume/BUILD new file mode 100644 index 0000000000..7e447e59b0 --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/volume/BUILD @@ -0,0 +1,27 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["constants.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/cloud-provider/volume", + importpath = "k8s.io/cloud-provider/volume", + visibility = ["//visibility:public"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [ + ":package-srcs", + "//staging/src/k8s.io/cloud-provider/volume/errors:all-srcs", + "//staging/src/k8s.io/cloud-provider/volume/helpers:all-srcs", + ], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/cloud-provider/volume/constants.go b/staging/src/k8s.io/cloud-provider/volume/constants.go new file mode 100644 index 0000000000..d05f64ae25 --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/volume/constants.go @@ -0,0 +1,26 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package volume + +const ( + // ProvisionedVolumeName is the name of a volume in an external cloud + // that is being provisioned and thus should be ignored by rest of Kubernetes. + ProvisionedVolumeName = "placeholder-for-provisioning" + + // LabelMultiZoneDelimiter separates zones for volumes + LabelMultiZoneDelimiter = "__" +) diff --git a/staging/src/k8s.io/cloud-provider/volume/errors/BUILD b/staging/src/k8s.io/cloud-provider/volume/errors/BUILD new file mode 100644 index 0000000000..30deba0b0d --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/volume/errors/BUILD @@ -0,0 +1,24 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["errors.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/cloud-provider/volume/errors", + importpath = "k8s.io/cloud-provider/volume/errors", + visibility = ["//visibility:public"], + deps = ["//staging/src/k8s.io/apimachinery/pkg/types:go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/volume/util/error.go b/staging/src/k8s.io/cloud-provider/volume/errors/errors.go similarity index 65% rename from pkg/volume/util/error.go rename to staging/src/k8s.io/cloud-provider/volume/errors/errors.go index a52a1b65ce..5202b4eb31 100644 --- a/pkg/volume/util/error.go +++ b/staging/src/k8s.io/cloud-provider/volume/errors/errors.go @@ -14,12 +14,37 @@ See the License for the specific language governing permissions and limitations under the License. */ -package util +package errors import ( k8stypes "k8s.io/apimachinery/pkg/types" ) +// NewDeletedVolumeInUseError returns a new instance of DeletedVolumeInUseError +// error. +func NewDeletedVolumeInUseError(message string) error { + return deletedVolumeInUseError(message) +} + +type deletedVolumeInUseError string + +var _ error = deletedVolumeInUseError("") + +// IsDeletedVolumeInUse returns true if an error returned from Delete() is +// deletedVolumeInUseError +func IsDeletedVolumeInUse(err error) bool { + switch err.(type) { + case deletedVolumeInUseError: + return true + default: + return false + } +} + +func (err deletedVolumeInUseError) Error() string { + return string(err) +} + // DanglingAttachError indicates volume is attached to a different node // than we expected. type DanglingAttachError struct { diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/BUILD b/staging/src/k8s.io/cloud-provider/volume/helpers/BUILD new file mode 100644 index 0000000000..9f2f2dcb2a --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/BUILD @@ -0,0 +1,39 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["zones.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/cloud-provider/volume/helpers", + importpath = "k8s.io/cloud-provider/volume/helpers", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", + "//vendor/k8s.io/klog:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["zones_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go new file mode 100644 index 0000000000..ec9c5d9915 --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones.go @@ -0,0 +1,310 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helpers + +import ( + "fmt" + "hash/fnv" + "math/rand" + "strconv" + "strings" + + "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" + cloudvolume "k8s.io/cloud-provider/volume" + "k8s.io/klog" +) + +// 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, cloudvolume.LabelMultiZoneDelimiter) +} + +// ZonesSetToLabelValue converts zones set to label value +func ZonesSetToLabelValue(strSet sets.String) string { + return strings.Join(strSet.UnsortedList(), cloudvolume.LabelMultiZoneDelimiter) +} + +// ZonesToSet converts a string containing a comma separated list of zones to set +func ZonesToSet(zonesString string) (sets.String, error) { + 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 +} + +// StringToSet converts a string containing list separated by specified delimiter 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 +} + +// 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, cloudvolume.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 +} + +// SelectZoneForVolume is a wrapper around SelectZonesForVolume +// to select a single zone for a volume based on parameters +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 nil, fmt.Errorf("both zone and zones StorageClass parameters must not be used at the same time") + } + + var zoneFromNode string + // pick one zone from node if present + if node != nil { + // VolumeScheduling implicit since node is not nil + if zoneParameterPresent || zonesParameterPresent { + 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 for one of the replicas + var ok bool + zoneFromNode, ok = node.ObjectMeta.Labels[v1.LabelZoneFailureDomain] + if !ok { + return nil, fmt.Errorf("%s Label for node missing", v1.LabelZoneFailureDomain) + } + // if single replica volume and node with zone found, return immediately + if numReplicas == 1 { + return sets.NewString(zoneFromNode), nil + } + } + + // pick zone from allowedZones if specified + allowedZones, err := ZonesFromAllowedTopologies(allowedTopologies) + if err != nil { + return nil, err + } + + if (len(allowedTopologies) > 0) && (allowedZones.Len() == 0) { + return nil, fmt.Errorf("no matchLabelExpressions with %s key found in allowedTopologies. Please specify matchLabelExpressions with %s key", v1.LabelZoneFailureDomain, v1.LabelZoneFailureDomain) + } + + if allowedZones.Len() > 0 { + // VolumeScheduling implicit since allowedZones present + if zoneParameterPresent || zonesParameterPresent { + 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. + zones, err := chooseZonesForVolumeIncludingZone(allowedZones, pvcName, zoneFromNode, numReplicas) + if err != nil { + return nil, fmt.Errorf("cannot process zones in allowedTopologies: %v", err) + } + return zones, nil + } + + // pick zone from parameters if present + if zoneParameterPresent { + 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 { + 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 + if zonesWithNodes.Len() > 0 { + // If node != null (and thus zoneFromNode != ""), zoneFromNode will be member of zonesWithNodes + zones, err := chooseZonesForVolumeIncludingZone(zonesWithNodes, pvcName, zoneFromNode, numReplicas) + if err != nil { + return nil, fmt.Errorf("cannot process zones where nodes exist in the cluster: %v", err) + } + 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) { + zones := make(sets.String) + for _, term := range allowedTopologies { + for _, exp := range term.MatchLabelExpressions { + if exp.Key == v1.LabelZoneFailureDomain { + for _, value := range exp.Values { + zones.Insert(value) + } + } else { + return nil, fmt.Errorf("unsupported key found in matchLabelExpressions: %s", exp.Key) + } + } + } + return zones, nil +} + +// 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 { + // No zones available, return empty set. + replicaZones := sets.NewString() + if zones.Len() == 0 { + return replicaZones + } + + // 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() + + startingIndex := index * numZones + for index = startingIndex; index < startingIndex+numZones; index++ { + zone := zoneSlice[(hash+index)%uint32(len(zoneSlice))] + replicaZones.Insert(zone) + } + + klog.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 + klog.Warningf("No name defined during volume create; choosing random zone") + + hash = rand.Uint32() + } else { + hashString := pvcName + + // Heuristic to make sure that volumes in a StatefulSet are spread across zones + // StatefulSet PVCs are (currently) named ClaimName-StatefulSetName-Id, + // where Id is an integer index. + // Note though that if a StatefulSet pod has multiple claims, we need them to be + // in the same zone, because otherwise the pod will be unable to mount both volumes, + // and will be unschedulable. So we hash _only_ the "StatefulSetName" portion when + // it looks like `ClaimName-StatefulSetName-Id`. + // We continue to round-robin volume names that look like `Name-Id` also; this is a useful + // feature for users that are creating statefulset-like functionality without using statefulsets. + lastDash := strings.LastIndexByte(pvcName, '-') + if lastDash != -1 { + statefulsetIDString := pvcName[lastDash+1:] + statefulsetID, err := strconv.ParseUint(statefulsetIDString, 10, 32) + if err == nil { + // Offset by the statefulsetID, so we round-robin across zones + index = uint32(statefulsetID) + // We still hash the volume name, but only the prefix + hashString = pvcName[:lastDash] + + // In the special case where it looks like `ClaimName-StatefulSetName-Id`, + // hash only the StatefulSetName, so that different claims on the same StatefulSet + // member end up in the same zone. + // Note that StatefulSetName (and ClaimName) might themselves both have dashes. + // We actually just take the portion after the final - of ClaimName-StatefulSetName. + // For our purposes it doesn't much matter (just suboptimal spreading). + lastDash := strings.LastIndexByte(hashString, '-') + if lastDash != -1 { + hashString = hashString[lastDash+1:] + } + + klog.V(2).Infof("Detected StatefulSet-style volume name %q; index=%d", pvcName, index) + } + } + + // We hash the (base) volume name, so we don't bias towards the first N zones + h := fnv.New32() + h.Write([]byte(hashString)) + hash = h.Sum32() + } + + return hash, index +} diff --git a/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go b/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go new file mode 100644 index 0000000000..e7a5e5b32f --- /dev/null +++ b/staging/src/k8s.io/cloud-provider/volume/helpers/zones_test.go @@ -0,0 +1,1639 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helpers + +import ( + "hash/fnv" + "testing" + + "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" +) + +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 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"), + }, + // Test for no zones + { + Zones: sets.NewString(), + VolumeName: "henley-1", + Expected: sets.NewString(), + }, + { + Zones: nil, + VolumeName: "henley-2", + Expected: sets.NewString(), + }, + } + + 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 TestSelectZoneForVolume(t *testing.T) { + + nodeWithZoneLabels := &v1.Node{} + nodeWithZoneLabels.Labels = map[string]string{v1.LabelZoneFailureDomain: "zoneX"} + + nodeWithNoLabels := &v1.Node{} + + tests := []struct { + // Parameters passed by test to SelectZoneForVolume + Name string + ZonePresent bool + Zone string + ZonesPresent bool + Zones string + ZonesWithNodes string + Node *v1.Node + AllowedTopologies []v1.TopologySelectorTerm + // Expectations around returned zone from SelectZoneForVolume + Reject bool // expect error due to validation failing + ExpectSpecificZone bool // expect returned zone to specifically match a single zone (rather than one from a set) + ExpectedZone string // single zone that should perfectly match returned zone (requires ExpectSpecificZone to be true) + ExpectedZones string // set of zones one of whose members should match returned zone (requires ExpectSpecificZone to be false) + }{ + // NEGATIVE TESTS + + // Zone and Zones are both specified [Fail] + // [1] Node irrelevant + // [2] Zone and Zones parameters presents + // [3] AllowedTopologies 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 + { + 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 + { + 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 + { + 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 + { + Name: "Nil_Node_and_Zone_parameter_and_Allowed_Topology_term", + Node: nil, + ZonePresent: true, + Zone: "zoneX", + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + }, + Reject: true, + }, + + // Zones parameter as well as AllowedTopologies specified [Fail] + // [1] nil Node + // [2] Zones parameter specified + // [3] AllowedTopologies specified + { + Name: "Nil_Node_and_Zones_parameter_and_Allowed_Topology_term", + Node: nil, + ZonesPresent: true, + Zones: "zoneX,zoneY", + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.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 + { + Name: "Nil_Node_and_Invalid_Allowed_Topology_Key", + Node: nil, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: "invalid_key", + Values: []string{"zoneX"}, + }, + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneY"}, + }, + }, + }, + }, + Reject: true, + }, + + // AllowedTopologies without keys specifying LabelZoneFailureDomain [Fail] + // [1] nil Node + // [2] no Zone/Zones parameter + // [3] Invalid AllowedTopologies + { + Name: "Nil_Node_and_Invalid_AllowedTopologies", + Node: nil, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{}, + }, + }, + Reject: true, + }, + + // POSITIVE TESTS WITH VolumeScheduling ENABLED + + // Select zone from active zones [Pass] + // [1] nil Node + // [2] no Zone parameter specified + // [3] no AllowedTopologies + { + Name: "Nil_Node_and_No_Zone_Zones_parameter_and_no_Allowed_topologies_and_VolumeScheduling_enabled", + Node: nil, + ZonesWithNodes: "zoneX,zoneY", + Reject: false, + ExpectedZones: "zoneX,zoneY", + }, + + // Select zone from single zone parameter [Pass] + // [1] nil Node + // [2] Zone parameter specified + // [3] no AllowedTopology specified + { + Name: "Nil_Node_and_Zone_parameter_present_and_VolumeScheduling_enabled", + ZonePresent: true, + Zone: "zoneX", + Node: nil, + Reject: false, + ExpectSpecificZone: true, + ExpectedZone: "zoneX", + }, + + // Select zone from zones parameter [Pass] + // [1] nil Node + // [2] Zones parameter specified + // [3] no AllowedTopology + { + Name: "Nil_Node_and_Zones_parameter_present_and_VolumeScheduling_enabled", + ZonesPresent: true, + Zones: "zoneX,zoneY", + Node: nil, + Reject: false, + ExpectedZones: "zoneX,zoneY", + }, + + // Select zone from node label [Pass] + // [1] Node with zone labels + // [2] no zone/zones parameters + // [3] no AllowedTopology + { + Name: "Node_with_Zone_labels_and_VolumeScheduling_enabled", + Node: nodeWithZoneLabels, + Reject: false, + ExpectSpecificZone: true, + ExpectedZone: "zoneX", + }, + + // Select zone from node label [Pass] + // [1] Node with zone labels + // [2] no Zone/Zones parameters + // [3] AllowedTopology with single term with multiple values specified (ignored) + { + Name: "Node_with_Zone_labels_and_Multiple_Allowed_Topology_values_and_VolumeScheduling_enabled", + Node: nodeWithZoneLabels, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneZ", "zoneY"}, + }, + }, + }, + }, + Reject: false, + ExpectSpecificZone: true, + ExpectedZone: "zoneX", + }, + + // Select Zone from AllowedTopologies [Pass] + // [1] nil Node + // [2] no Zone/Zones parametes specified + // [3] AllowedTopologies with single term with multiple values specified + { + Name: "Nil_Node_with_Multiple_Allowed_Topology_values_and_VolumeScheduling_enabled", + Node: nil, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneX", "zoneY"}, + }, + }, + }, + }, + Reject: false, + ExpectedZones: "zoneX,zoneY", + }, + + // Select zone from AllowedTopologies [Pass] + // [1] nil Node + // [2] no Zone/Zones parametes specified + // [3] AllowedTopologies with multiple terms specified + { + Name: "Nil_Node_and_Multiple_Allowed_Topology_terms_and_VolumeScheduling_enabled", + Node: nil, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneY"}, + }, + }, + }, + }, + Reject: false, + ExpectedZones: "zoneX,zoneY", + }, + + // Select Zone from AllowedTopologies [Pass] + // Note: Dual replica with same AllowedTopologies will fail: Nil_Node_and_Single_Allowed_Topology_term_value_and_Dual_replicas + // [1] nil Node + // [2] no Zone/Zones parametes specified + // [3] AllowedTopologies with single term and value specified + { + Name: "Nil_Node_and_Single_Allowed_Topology_term_value_and_VolumeScheduling_enabled", + Node: nil, + AllowedTopologies: []v1.TopologySelectorTerm{ + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + }, + Reject: false, + ExpectSpecificZone: true, + ExpectedZone: "zoneX", + }, + } + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + var zonesParameter, zonesWithNodes sets.String + var err error + + if test.Zones != "" { + zonesParameter, err = ZonesToSet(test.Zones) + if err != nil { + t.Fatalf("Could not convert Zones to a set: %s. This is a test error %s", test.Zones, test.Name) + } + } + + if test.ZonesWithNodes != "" { + zonesWithNodes, err = ZonesToSet(test.ZonesWithNodes) + if err != nil { + t.Fatalf("Could not convert specified ZonesWithNodes to a set: %s. This is a test error %s", test.ZonesWithNodes, test.Name) + } + } + + zone, err := SelectZoneForVolume(test.ZonePresent, test.ZonesPresent, test.Zone, zonesParameter, zonesWithNodes, test.Node, test.AllowedTopologies, test.Name) + + if test.Reject && err == nil { + t.Errorf("Unexpected zone from SelectZoneForVolume for %s", zone) + } + + if !test.Reject { + if err != nil { + t.Errorf("Unexpected error from SelectZoneForVolume for %s; Error: %v", test.Name, err) + } + + if test.ExpectSpecificZone == true { + if zone != test.ExpectedZone { + t.Errorf("Expected zone %v does not match obtained zone %v for %s", test.ExpectedZone, zone, test.Name) + } + } + + if test.ExpectedZones != "" { + expectedZones, err := ZonesToSet(test.ExpectedZones) + if err != nil { + t.Fatalf("Could not convert ExpectedZones to a set: %s. This is a test error", test.ExpectedZones) + } + if !expectedZones.Has(zone) { + t.Errorf("Obtained zone %s not member of expectedZones %s", zone, expectedZones) + } + } + } + }) + } +} + +func TestSelectZonesForVolume(t *testing.T) { + + nodeWithZoneLabels := &v1.Node{} + nodeWithZoneLabels.Labels = map[string]string{v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.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: v1.LabelZoneFailureDomain, + Values: []string{"zoneV"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneW"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneY"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.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: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.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: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.LabelZoneFailureDomain, + Values: []string{"zoneY"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.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: v1.LabelZoneFailureDomain, + Values: []string{"zoneX"}, + }, + }, + }, + { + MatchLabelExpressions: []v1.TopologySelectorLabelRequirement{ + { + Key: v1.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 checkFnv32(t *testing.T, s string, expected uint32) { + h := fnv.New32() + h.Write([]byte(s)) + h.Sum32() + + if h.Sum32() != expected { + t.Fatalf("hash of %q was %v, expected %v", s, h.Sum32(), expected) + } +} diff --git a/staging/src/k8s.io/csi-translation-lib/Godeps/Godeps.json b/staging/src/k8s.io/csi-translation-lib/Godeps/Godeps.json index e338516539..75a383f117 100644 --- a/staging/src/k8s.io/csi-translation-lib/Godeps/Godeps.json +++ b/staging/src/k8s.io/csi-translation-lib/Godeps/Godeps.json @@ -142,6 +142,10 @@ "ImportPath": "k8s.io/apimachinery/third_party/forked/golang/reflect", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/cloud-provider/volume", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/klog", "Rev": "8139d8cb77af419532b33dfa7dd09fbc5f1d344f" diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/BUILD b/staging/src/k8s.io/csi-translation-lib/plugins/BUILD index 3afa8d26e4..683440587d 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/BUILD +++ b/staging/src/k8s.io/csi-translation-lib/plugins/BUILD @@ -14,6 +14,7 @@ go_library( deps = [ "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume:go_default_library", ], ) diff --git a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go index 9082b73936..ddf73ff137 100644 --- a/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go +++ b/staging/src/k8s.io/csi-translation-lib/plugins/gce_pd.go @@ -23,6 +23,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" + cloudvolume "k8s.io/cloud-provider/volume" ) const ( @@ -39,10 +40,6 @@ const ( volIDDiskNameValue = 5 volIDTotalElements = 6 - // LabelZoneFailureDomain is the label on PVs indicating the zone they are provisioned in - LabelZoneFailureDomain = "failure-domain.beta.kubernetes.io/zone" - // LabelMultiZoneDelimiter separates zones for RePD volumes - LabelMultiZoneDelimiter = "__" // UnspecifiedValue is used for an unknown zone string UnspecifiedValue = "UNSPECIFIED" ) @@ -72,8 +69,8 @@ func (g *gcePersistentDiskCSITranslator) TranslateInTreePVToCSI(pv *v1.Persisten return nil, fmt.Errorf("pv is nil or GCE Persistent Disk source not defined on pv") } - zonesLabel := pv.Labels[LabelZoneFailureDomain] - zones := strings.Split(zonesLabel, LabelMultiZoneDelimiter) + zonesLabel := pv.Labels[v1.LabelZoneFailureDomain] + zones := strings.Split(zonesLabel, cloudvolume.LabelMultiZoneDelimiter) if len(zones) == 1 && len(zones[0]) != 0 { // Zonal volID = fmt.Sprintf(volIDZonalFmt, UnspecifiedValue, zones[0], pv.Spec.GCEPersistentDisk.PDName) diff --git a/test/e2e/storage/BUILD b/test/e2e/storage/BUILD index 78579bae5f..c3f048265e 100644 --- a/test/e2e/storage/BUILD +++ b/test/e2e/storage/BUILD @@ -63,6 +63,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library", + "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1:go_default_library", "//staging/src/k8s.io/csi-api/pkg/client/clientset/versioned:go_default_library", "//test/e2e/framework:go_default_library", diff --git a/test/e2e/storage/regional_pd.go b/test/e2e/storage/regional_pd.go index 176d380f88..cc8b0a1785 100644 --- a/test/e2e/storage/regional_pd.go +++ b/test/e2e/storage/regional_pd.go @@ -37,8 +37,8 @@ import ( "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" + volumehelpers "k8s.io/cloud-provider/volume/helpers" podutil "k8s.io/kubernetes/pkg/api/v1/pod" - "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/e2e/storage/testsuites" "k8s.io/kubernetes/test/e2e/storage/utils" @@ -583,7 +583,7 @@ func waitForStatefulSetReplicasNotReady(statefulSetName, ns string, c clientset. // If match is true, check if zones in PV exactly match zones given. // Otherwise, check whether zones in PV is superset of zones given. func verifyZonesInPV(volume *v1.PersistentVolume, zones sets.String, match bool) error { - pvZones, err := util.LabelZonesToSet(volume.Labels[v1.LabelZoneFailureDomain]) + pvZones, err := volumehelpers.LabelZonesToSet(volume.Labels[v1.LabelZoneFailureDomain]) if err != nil { return err } diff --git a/test/e2e/storage/volume_provisioning.go b/test/e2e/storage/volume_provisioning.go index 983c507040..cabe110389 100644 --- a/test/e2e/storage/volume_provisioning.go +++ b/test/e2e/storage/volume_provisioning.go @@ -43,8 +43,8 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/serviceaccount" clientset "k8s.io/client-go/kubernetes" + volumehelpers "k8s.io/cloud-provider/volume/helpers" storageutil "k8s.io/kubernetes/pkg/apis/storage/v1/util" - volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/e2e/framework/providers/gce" "k8s.io/kubernetes/test/e2e/storage/testsuites" @@ -75,7 +75,7 @@ func checkZonesFromLabelAndAffinity(pv *v1.PersistentVolume, zones sets.String, framework.Failf("label %s not found on PV", v1.LabelZoneFailureDomain) } - zonesFromLabel, err := volumeutil.LabelZonesToSet(pvLabel) + zonesFromLabel, err := volumehelpers.LabelZonesToSet(pvLabel) if err != nil { framework.Failf("unable to parse zone labels %s: %v", pvLabel, err) }