Add label selector to match criteria for claims to volumes

pkg/controller/persistentvolume/binder_test.go

@@ -28,6 +28,11 @@ import (
 //    controllerTest.testCall *once*.
 // 3. Compare resulting volumes and claims with expected volumes and claims.
 func TestSync(t *testing.T) {
+	labels := map[string]string{
+		"foo": "true",
+		"bar": "false",
+	}
+
 	tests := []controllerTest{
 		// [Unit test set 1] User did not care which PV they get.
 		// Test the matching with no claim.Spec.VolumeName and with various
@@ -140,13 +145,32 @@ func TestSync(t *testing.T) {
 		{
 			// syncClaim completes binding - simulates controller crash after
 			// PVC.VolumeName is saved
-			"10 - complete bind after crash - PVC bound",
+			"1-10 - complete bind after crash - PVC bound",
 			newVolumeArray("volume1-10", "1Gi", "uid1-10", "claim1-10", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
 			newVolumeArray("volume1-10", "1Gi", "uid1-10", "claim1-10", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
 			newClaimArray("claim1-10", "uid1-10", "1Gi", "volume1-10", api.ClaimPending, annBoundByController, annBindCompleted),
 			newClaimArray("claim1-10", "uid1-10", "1Gi", "volume1-10", api.ClaimBound, annBoundByController, annBindCompleted),
 			noevents, noerrors, testSyncClaim,
 		},
+		{
+			// syncClaim binds a claim only when the label selector matches the volume
+			"1-11 - bind when selector matches",
+			withLabels(labels, newVolumeArray("volume1-1", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain)),
+			withLabels(labels, newVolumeArray("volume1-1", "1Gi", "uid1-1", "claim1-1", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController)),
+			withLabelSelector(labels, newClaimArray("claim1-1", "uid1-1", "1Gi", "", api.ClaimPending)),
+			withLabelSelector(labels, newClaimArray("claim1-1", "uid1-1", "1Gi", "volume1-1", api.ClaimBound, annBoundByController, annBindCompleted)),
+			noevents, noerrors, testSyncClaim,
+		},
+		{
+			// syncClaim does not bind a claim when the label selector doesn't match
+			"1-12 - do not bind when selector does not match",
+			newVolumeArray("volume1-1", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
+			newVolumeArray("volume1-1", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
+			withLabelSelector(labels, newClaimArray("claim1-1", "uid1-1", "1Gi", "", api.ClaimPending)),
+			withLabelSelector(labels, newClaimArray("claim1-1", "uid1-1", "1Gi", "", api.ClaimPending)),
+			noevents, noerrors, testSyncClaim,
+		},
+
 		// [Unit test set 2] User asked for a specific PV.
 		// Test the binding when pv.ClaimRef is already set by controller or
 		// by user.
@@ -220,6 +244,18 @@ func TestSync(t *testing.T) {
 			newClaimArray("claim2-7", "uid2-7", "10Gi", "volume2-7", api.ClaimBound, annBoundByController),
 			noevents, noerrors, testSyncClaimError,
 		},
+		{
+			// syncClaim with claim pre-bound to a PV that exists and is
+			// unbound, but does not match the selector. Check it gets bound
+			// and no annBoundByController is set.
+			"2-8 - claim prebound to unbound volume that does not match the selector",
+			newVolumeArray("volume2-3", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
+			newVolumeArray("volume2-3", "1Gi", "uid2-3", "claim2-3", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
+			withLabelSelector(labels, newClaimArray("claim2-3", "uid2-3", "10Gi", "volume2-3", api.ClaimPending)),
+			withLabelSelector(labels, newClaimArray("claim2-3", "uid2-3", "10Gi", "volume2-3", api.ClaimBound, annBindCompleted)),
+			noevents, noerrors, testSyncClaim,
+		},
+
 		// [Unit test set 3] Syncing bound claim
 		{
 			// syncClaim with claim  bound and its claim.Spec.VolumeName is
@@ -283,6 +319,17 @@ func TestSync(t *testing.T) {
 			newClaimArray("claim3-6", "uid3-6", "10Gi", "volume3-6", api.ClaimLost, annBindCompleted),
 			[]string{"Warning ClaimMisbound"}, noerrors, testSyncClaim,
 		},
+		{
+			// syncClaim with claim bound to unbound volume. Check it's bound
+			// even if the claim's selector doesn't match the volume. Also
+			// check that Pending phase is set to Bound
+			"3-7 - bound claim with unbound volume where selector doesn't match",
+			newVolumeArray("volume3-3", "10Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
+			newVolumeArray("volume3-3", "10Gi", "uid3-3", "claim3-3", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
+			withLabelSelector(labels, newClaimArray("claim3-3", "uid3-3", "10Gi", "volume3-3", api.ClaimPending, annBoundByController, annBindCompleted)),
+			withLabelSelector(labels, newClaimArray("claim3-3", "uid3-3", "10Gi", "volume3-3", api.ClaimBound, annBoundByController, annBindCompleted)),
+			noevents, noerrors, testSyncClaim,
+		},
 		// [Unit test set 4] All syncVolume tests.
 		{
 			// syncVolume with pending volume. Check it's marked as Available.

pkg/controller/persistentvolume/framework_test.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/client/cache"
 	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
@@ -575,6 +576,25 @@ func newVolume(name, capacity, boundToClaimUID, boundToClaimName string, phase a
 	return &volume
 }
 
+// withLabels applies the given labels to the first volume in the array and
+// returns the array.  Meant to be used to compose volumes specified inline in
+// a test.
+func withLabels(labels map[string]string, volumes []*api.PersistentVolume) []*api.PersistentVolume {
+	volumes[0].Labels = labels
+	return volumes
+}
+
+// withLabelSelector sets the label selector of the first claim in the array
+// to be MatchLabels of the given label set and returns the array.  Meant
+// to be used to compose claims specified inline in a test.
+func withLabelSelector(labels map[string]string, claims []*api.PersistentVolumeClaim) []*api.PersistentVolumeClaim {
+	claims[0].Spec.Selector = &unversioned.LabelSelector{
+		MatchLabels: labels,
+	}
+
+	return claims
+}
+
 // newVolumeArray returns array with a single volume that would be returned by
 // newVolume() with the same parameters.
 func newVolumeArray(name, capacity, boundToClaimUID, boundToClaimName string, phase api.PersistentVolumePhase, reclaimPolicy api.PersistentVolumeReclaimPolicy, annotations ...string) []*api.PersistentVolume {

pkg/controller/persistentvolume/index.go

@@ -21,7 +21,9 @@ import (
 	"sort"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/client/cache"
+	"k8s.io/kubernetes/pkg/labels"
 )
 
 // persistentVolumeOrderedIndex is a cache.Store that keeps persistent volumes indexed by AccessModes and ordered by storage capacity.
@@ -83,6 +85,16 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *api.PersistentVo
 	requestedQty := claim.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
 	requestedSize := requestedQty.Value()
 
+	var selector labels.Selector
+	if claim.Spec.Selector != nil {
+		internalSelector, err := unversioned.LabelSelectorAsSelector(claim.Spec.Selector)
+		if err != nil {
+			// should be unreachable code due to validation
+			return nil, fmt.Errorf("error creating internal label selector for claim: %v: %v", claimToClaimKey(claim), err)
+		}
+		selector = internalSelector
+	}
+
 	for _, modes := range allPossibleModes {
 		volumes, err := pvIndex.listByAccessModes(modes)
 		if err != nil {
@@ -97,14 +109,18 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *api.PersistentVo
 		//   the claim.
 		for _, volume := range volumes {
 			if isVolumeBoundToClaim(volume, claim) {
-				// Exact match! No search required. This catches both volumes
-				// pre-bound by user and volumes dynamically provisioned by the
-				// controller.
+				// this claim and volume are bound; return it,
+				// whether the claim is prebound or for volumes
+				// intended for dynamic provisioning v1
 				return volume, nil
 			}
 
+			// filter out:
+			// - volumes bound to another claim
+			// - volumes whose labels don't match the claim's selector, if specified
 			if volume.Spec.ClaimRef != nil {
-				// This volume waits for exact claim or is alredy bound.
+				continue
+			} else if selector != nil && !selector.Matches(labels.Set(volume.Labels)) {
 				continue
 			}
 

pkg/controller/persistentvolume/index_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/api/unversioned"
 )
 
 func TestMatchVolume(t *testing.T) {
@@ -103,6 +104,72 @@ func TestMatchVolume(t *testing.T) {
 				},
 			},
 		},
+		"successful-no-match-due-to-label": {
+			expectedMatch: "",
+			claim: &api.PersistentVolumeClaim{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "claim01",
+					Namespace: "myns",
+				},
+				Spec: api.PersistentVolumeClaimSpec{
+					Selector: &unversioned.LabelSelector{
+						MatchLabels: map[string]string{
+							"should-not-exist": "true",
+						},
+					},
+					AccessModes: []api.PersistentVolumeAccessMode{api.ReadOnlyMany, api.ReadWriteOnce},
+					Resources: api.ResourceRequirements{
+						Requests: api.ResourceList{
+							api.ResourceName(api.ResourceStorage): resource.MustParse("999G"),
+						},
+					},
+				},
+			},
+		},
+		"successful-no-match-due-to-size-constraint-with-label-selector": {
+			expectedMatch: "",
+			claim: &api.PersistentVolumeClaim{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "claim01",
+					Namespace: "myns",
+				},
+				Spec: api.PersistentVolumeClaimSpec{
+					Selector: &unversioned.LabelSelector{
+						MatchLabels: map[string]string{
+							"should-exist": "true",
+						},
+					},
+					AccessModes: []api.PersistentVolumeAccessMode{api.ReadOnlyMany, api.ReadWriteOnce},
+					Resources: api.ResourceRequirements{
+						Requests: api.ResourceList{
+							api.ResourceName(api.ResourceStorage): resource.MustParse("20000G"),
+						},
+					},
+				},
+			},
+		},
+		"successful-match-due-with-constraint-and-label-selector": {
+			expectedMatch: "gce-pd-2",
+			claim: &api.PersistentVolumeClaim{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "claim01",
+					Namespace: "myns",
+				},
+				Spec: api.PersistentVolumeClaimSpec{
+					Selector: &unversioned.LabelSelector{
+						MatchLabels: map[string]string{
+							"should-exist": "true",
+						},
+					},
+					AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce},
+					Resources: api.ResourceRequirements{
+						Requests: api.ResourceList{
+							api.ResourceName(api.ResourceStorage): resource.MustParse("10000G"),
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for name, scenario := range scenarios {
@@ -486,6 +553,26 @@ func createTestVolumes() []*api.PersistentVolume {
 				},
 			},
 		},
+		{
+			ObjectMeta: api.ObjectMeta{
+				UID:  "gce-pd-2",
+				Name: "gce0022",
+				Labels: map[string]string{
+					"should-exist": "true",
+				},
+			},
+			Spec: api.PersistentVolumeSpec{
+				Capacity: api.ResourceList{
+					api.ResourceName(api.ResourceStorage): resource.MustParse("10000G"),
+				},
+				PersistentVolumeSource: api.PersistentVolumeSource{
+					GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{},
+				},
+				AccessModes: []api.PersistentVolumeAccessMode{
+					api.ReadWriteOnce,
+				},
+			},
+		},
 	}
 }