From 24f57647870c770cbb4431bd7b1e4e8be0b1fb67 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 28 Oct 2020 10:40:42 +0100 Subject: [PATCH] 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. --- .../volume/persistentvolume/binder_test.go | 34 +++++++++++++++++++ .../volume/persistentvolume/framework_test.go | 11 ++++++ .../persistentvolume/pv_controller_test.go | 11 ++++++ 3 files changed, 56 insertions(+) diff --git a/pkg/controller/volume/persistentvolume/binder_test.go b/pkg/controller/volume/persistentvolume/binder_test.go index fb5f3ffadd2..3147b504cff 100644 --- a/pkg/controller/volume/persistentvolume/binder_test.go +++ b/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 { diff --git a/pkg/controller/volume/persistentvolume/framework_test.go b/pkg/controller/volume/persistentvolume/framework_test.go index cebdcff40f1..aeecce3e0aa 100644 --- a/pkg/controller/volume/persistentvolume/framework_test.go +++ b/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) diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 896148f0c2b..5c39e17a9dc 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/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",