pv controller test: more test cases

The main goal was to cover retrieval of a PVC from the apiserver when
it isn't known yet. This is achieved by adding PVCs and (for the sake
of completeness) PVs to the reactor, but not the controller, when a
special annotation is set. The approach with a special annotation was
chosen because it doesn't affect other tests.

The other test cases were added while checking the existing tests
because (at least at first glance) the situations seemed to be not
covered.

pkg/controller/volume/persistentvolume/binder_test.go

@@ -528,6 +528,40 @@ func TestSync(t *testing.T) {
 			newClaimArray("claim4-8", "uid4-8", "10Gi", "volume4-8-x", v1.ClaimBound, nil),
 			noevents, noerrors, testSyncVolume,
 		},
+		{
+			// syncVolume with volume bound to bound claim.
+			// Check that the volume is not deleted.
+			"4-9 - volume bound to bound claim, with PersistentVolumeReclaimDelete",
+			newVolumeArray("volume4-9", "10Gi", "uid4-9", "claim4-9", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty),
+			newVolumeArray("volume4-9", "10Gi", "uid4-9", "claim4-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			newClaimArray("claim4-9", "uid4-9", "10Gi", "volume4-9", v1.ClaimBound, nil),
+			newClaimArray("claim4-9", "uid4-9", "10Gi", "volume4-9", v1.ClaimBound, nil),
+			noevents, noerrors, testSyncVolume,
+		},
+		{
+			// syncVolume with volume bound to missing claim.
+			// Check that a volume deletion is attempted. It fails because there is no deleter.
+			"4-10 - volume bound to missing claim",
+			newVolumeArray("volume4-10", "10Gi", "uid4-10", "claim4-10", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty),
+			func() []*v1.PersistentVolume {
+				volumes := newVolumeArray("volume4-10", "10Gi", "uid4-10", "claim4-10", v1.VolumeFailed, v1.PersistentVolumeReclaimDelete, classEmpty)
+				volumes[0].Status.Message = `Error getting deleter volume plugin for volume "volume4-10": no volume plugin matched`
+				return volumes
+			}(),
+			noclaims,
+			noclaims,
+			noevents, noerrors, testSyncVolume,
+		},
+		{
+			// syncVolume with volume bound to claim which exists in etcd but not in the local cache.
+			// Check that nothing changes, in contrast to case 4-10 above.
+			"4-11 - volume bound to unknown claim",
+			newVolumeArray("volume4-11", "10Gi", "uid4-11", "claim4-11", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty),
+			newVolumeArray("volume4-11", "10Gi", "uid4-11", "claim4-11", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
+			newClaimArray("claim4-11", "uid4-11", "10Gi", "volume4-11", v1.ClaimBound, nil, annSkipLocalStore),
+			newClaimArray("claim4-11", "uid4-11", "10Gi", "volume4-11", v1.ClaimBound, nil, annSkipLocalStore),
+			noevents, noerrors, testSyncVolume,
+		},
 
 		// PVC with class
 		{

pkg/controller/volume/persistentvolume/framework_test.go

@@ -95,6 +95,11 @@ type controllerTest struct {
 	test testCall
 }
 
+// annSkipLocalStore can be used to mark initial PVs or PVCs that are meant to be added only
+// to the fake apiserver (i.e. available via Get) but not to the local store (i.e. the controller
+// won't have them in its cache).
+const annSkipLocalStore = "pv-testing-skip-local-store"
+
 type testCall func(ctrl *PersistentVolumeController, reactor *pvtesting.VolumeReactor, test controllerTest) error
 
 const testNamespace = "default"
@@ -641,9 +646,15 @@ func runSyncTests(t *testing.T, tests []controllerTest, storageClasses []*storag
 		}
 		reactor := newVolumeReactor(client, ctrl, nil, nil, test.errors)
 		for _, claim := range test.initialClaims {
+			if metav1.HasAnnotation(claim.ObjectMeta, annSkipLocalStore) {
+				continue
+			}
 			ctrl.claims.Add(claim)
 		}
 		for _, volume := range test.initialVolumes {
+			if metav1.HasAnnotation(volume.ObjectMeta, annSkipLocalStore) {
+				continue
+			}
 			ctrl.volumes.store.Add(volume)
 		}
 		reactor.AddClaims(test.initialClaims)

pkg/controller/volume/persistentvolume/pv_controller_test.go

@@ -78,6 +78,17 @@ func TestControllerSync(t *testing.T) {
 				return nil
 			},
 		},
+		{
+			"5-2-2 - complete bind when PV and PVC both exist",
+			newVolumeArray("volume5-2", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimRetain, classEmpty),
+			newVolumeArray("volume5-2", "1Gi", "uid5-2", "claim5-2", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, pvutil.AnnBoundByController),
+			newClaimArray("claim5-2", "uid5-2", "1Gi", "", v1.ClaimPending, nil),
+			newClaimArray("claim5-2", "uid5-2", "1Gi", "volume5-2", v1.ClaimBound, nil, pvutil.AnnBoundByController, pvutil.AnnBindCompleted),
+			noevents, noerrors,
+			func(ctrl *PersistentVolumeController, reactor *pvtesting.VolumeReactor, test controllerTest) error {
+				return nil
+			},
+		},
 		{
 			// deleteClaim with a bound claim makes bound volume released.
 			"5-3 - delete claim",