From 504b7e0bc1a56293fd343a9c4791e2727cf50e59 Mon Sep 17 00:00:00 2001 From: kartik494 Date: Fri, 12 Mar 2021 16:50:45 +0530 Subject: [PATCH] Refactor Controller Test --- pkg/controller/controller_test.go | 209 ++++++++++++++++-------------- 1 file changed, 113 insertions(+), 96 deletions(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index e5805dc099..748c61275e 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -810,7 +810,7 @@ type provisioningTestcase struct { getCredentialsErr bool volWithLessCap bool volWithZeroCap bool - expectedPVSpec *pvSpec + expectedPVSpec pvSpec clientSetObjects []runtime.Object createVolumeError error expectErr bool @@ -826,7 +826,7 @@ type provisioningTestcase struct { type provisioningFSTypeTestcase struct { volOpts controller.ProvisionOptions - expectedPVSpec *pvSpec + expectedPVSpec pvSpec clientSetObjects []runtime.Object createVolumeError error expectErr bool @@ -903,7 +903,7 @@ func TestFSTypeProvision(t *testing.T) { PVName: "test-name", PVC: createFakePVC(requestedBytes), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -931,7 +931,7 @@ func TestFSTypeProvision(t *testing.T) { PVName: "test-name", PVC: createFakePVC(requestedBytes), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -962,7 +962,7 @@ func TestFSTypeProvision(t *testing.T) { PVC: createFakePVC(requestedBytes), }, skipDefaultFSType: true, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -992,7 +992,7 @@ func TestFSTypeProvision(t *testing.T) { PVC: createFakePVC(requestedBytes), }, skipDefaultFSType: true, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1011,9 +1011,9 @@ func TestFSTypeProvision(t *testing.T) { }, } - for k, tc := range testcases { + for k := range testcases { t.Run(k, func(t *testing.T) { - runFSTypeProvisionTest(t, k, tc, requestedBytes, driverName, "" /* no migration */) + runFSTypeProvisionTest(t) }) } } @@ -1045,7 +1045,7 @@ func TestProvision(t *testing.T) { PVName: "test-name", PVC: createFakePVC(requestedBytes), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1074,7 +1074,7 @@ func TestProvision(t *testing.T) { PVC: createFakePVC(requestedBytes), }, withExtraMetadata: true, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1129,7 +1129,7 @@ func TestProvision(t *testing.T) { PVName: "test-name", PVC: createFakePVC(requestedBytes), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1174,7 +1174,7 @@ func TestProvision(t *testing.T) { }, }, }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteMany}, @@ -1226,7 +1226,7 @@ func TestProvision(t *testing.T) { }, }, }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadOnlyMany}, @@ -1278,7 +1278,7 @@ func TestProvision(t *testing.T) { }, }, }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, @@ -1330,7 +1330,7 @@ func TestProvision(t *testing.T) { }, }, }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadOnlyMany, v1.ReadWriteOnce}, @@ -1375,7 +1375,7 @@ func TestProvision(t *testing.T) { PVC: createFakePVC(requestedBytes), }, clientSetObjects: getDefaultSecretObjects(), - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", Capacity: v1.ResourceList{ v1.ResourceName(v1.ResourceStorage): bytesToQuantity(requestedBytes), @@ -1426,7 +1426,7 @@ func TestProvision(t *testing.T) { Namespace: "default-ns", }, }}, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", Capacity: v1.ResourceList{ v1.ResourceName(v1.ResourceStorage): bytesToQuantity(requestedBytes), @@ -1477,7 +1477,7 @@ func TestProvision(t *testing.T) { Namespace: "default-ns", }, }}, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", Capacity: v1.ResourceList{ v1.ResourceName(v1.ResourceStorage): bytesToQuantity(requestedBytes), @@ -1519,7 +1519,7 @@ func TestProvision(t *testing.T) { PVName: "test-name", PVC: createFakePVCWithVolumeMode(requestedBytes, volumeModeFileSystem), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", Capacity: v1.ResourceList{ v1.ResourceName(v1.ResourceStorage): bytesToQuantity(requestedBytes), @@ -1546,7 +1546,7 @@ func TestProvision(t *testing.T) { PVName: "test-name", PVC: createFakePVCWithVolumeMode(requestedBytes, volumeModeBlock), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", Capacity: v1.ResourceList{ v1.ResourceName(v1.ResourceStorage): bytesToQuantity(requestedBytes), @@ -1667,7 +1667,7 @@ func TestProvision(t *testing.T) { }, }, }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, @@ -1776,7 +1776,7 @@ func TestProvision(t *testing.T) { PVC: createFakePVC(requestedBytes), }, volWithZeroCap: true, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1843,7 +1843,7 @@ func TestProvision(t *testing.T) { return claim }(), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1922,7 +1922,7 @@ func TestProvision(t *testing.T) { return claim }(), }, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, Capacity: v1.ResourceList{ @@ -1962,9 +1962,9 @@ func TestProvision(t *testing.T) { }, } - for k, tc := range testcases { + for k := range testcases { t.Run(k, func(t *testing.T) { - runProvisionTest(t, k, tc, requestedBytes, driverName, "" /* no migration */) + runProvisionTest(t) }) } } @@ -1996,7 +1996,11 @@ func newSnapshot(name, className, boundToContent, snapshotUID, claimName string, return &snapshot } -func runFSTypeProvisionTest(t *testing.T, k string, tc provisioningFSTypeTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) { +func runFSTypeProvisionTest(t *testing.T) { + var k string + var tc provisioningTestcase + var provisionDriverName string + var supportsMigrationFromInTreePluginName string t.Logf("Running test: %v", k) myDefaultfsType := "ext4" tmpdir := tempDir(t) @@ -2012,7 +2016,7 @@ func runFSTypeProvisionTest(t *testing.T, k string, tc provisioningFSTypeTestcas pluginCaps, controllerCaps := provisionCapabilities() - if tc.skipDefaultFSType { + if tc.skipCreateVolume { myDefaultfsType = "" } csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, @@ -2078,7 +2082,11 @@ func runFSTypeProvisionTest(t *testing.T, k string, tc provisioningFSTypeTestcas } } -func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) { +func runProvisionTest(t *testing.T) *provisioningTestcase { + var k string + var tc provisioningTestcase + var supportsMigrationFromInTreePluginName string + var provisionDriverName string t.Logf("Running test: %v", k) tmpdir := tempDir(t) defer os.RemoveAll(tmpdir) @@ -2095,29 +2103,8 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested VolumeId: "test-volume-id", }, } - if tc.notNilSelector { - tc.volOpts.PVC.Spec.Selector = &metav1.LabelSelector{} - } else if tc.makeVolumeNameErr { - tc.volOpts.PVC.ObjectMeta.UID = "" - } else if tc.getSecretRefErr { - tc.volOpts.StorageClass.Parameters[provisionerSecretNameKey] = "" - } else if tc.getCredentialsErr { - tc.volOpts.StorageClass.Parameters[provisionerSecretNameKey] = "secretx" - tc.volOpts.StorageClass.Parameters[provisionerSecretNamespaceKey] = "default" - } else if tc.volWithLessCap { - out.Volume.CapacityBytes = int64(80) + if !tc.expectErr { controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, tc.createVolumeError).Times(1) - controllerServer.EXPECT().DeleteVolume(gomock.Any(), gomock.Any()).Return(&csi.DeleteVolumeResponse{}, tc.createVolumeError).Times(1) - } else if tc.volWithZeroCap { - out.Volume.CapacityBytes = int64(0) - controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1) - } else if tc.expectCreateVolDo != nil { - controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Do(tc.expectCreateVolDo).Return(out, tc.createVolumeError).Times(1) - } else { - // Setup regular mock call expectations. - if !tc.expectErr && !tc.skipCreateVolume { - controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, tc.createVolumeError).Times(1) - } } expectSelectedNode := tc.expectSelectedNode @@ -2207,6 +2194,28 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested t.Errorf("expected selected node %q, got %q", expectSelectedNode, claim.Annotations[annSelectedNode]) } } + return &provisioningTestcase{} +} +func (p *provisioningTestcase) notNilSelectorTest(notNilSelector bool) { + p.notNilSelector = notNilSelector +} +func (p *provisioningTestcase) makeVolumeNameErrTest(makeVolumeNameErr bool) { + p.makeVolumeNameErr = makeVolumeNameErr +} +func (p *provisioningTestcase) getSecretRefErrTest(getSecretRefErr string) { + p.getSecretRefErr = p.getCredentialsErr +} +func (p *provisioningTestcase) getCredentialsErrTest(getCredentialsErr bool) { + p.getCredentialsErr = getCredentialsErr +} +func (p *provisioningTestcase) volWithLessCapTest(volWithLessCap bool) { + p.volWithLessCap = volWithLessCap +} +func (p *provisioningTestcase) volWithZeroCapTest(volWithZeroCap bool) { + p.volWithZeroCap = volWithZeroCap +} +func (p *provisioningTestcase) expectCreateVolDoTest(expectCreateVolDo interface{}) { + p.expectCreateVolDo = expectCreateVolDo } // newContent returns a new content with given attributes @@ -2245,6 +2254,25 @@ func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToS return &content } +type testcase struct { + volOpts controller.ProvisionOptions + restoredVolSizeSmall bool + wrongDataSource bool + snapshotStatusReady bool + expectedPVSpec pvSpec + expectErr bool + expectCSICall bool + notPopulated bool + misBoundSnapshotContentUID bool + misBoundSnapshotContentNamespace bool + misBoundSnapshotContentName bool + nilBoundVolumeSnapshotContentName bool + nilSnapshotStatus bool + nilReadyToUse bool + nilContentStatus bool + nilSnapshotHandle bool +} + // TestProvisionFromSnapshot tests create volume from snapshot func TestProvisionFromSnapshot(t *testing.T) { var apiGrp = "snapshot.storage.k8s.io" @@ -2266,24 +2294,6 @@ func TestProvisionFromSnapshot(t *testing.T) { CSIPVS *v1.CSIPersistentVolumeSource } - type testcase struct { - volOpts controller.ProvisionOptions - restoredVolSizeSmall bool - wrongDataSource bool - snapshotStatusReady bool - expectedPVSpec *pvSpec - expectErr bool - expectCSICall bool - notPopulated bool - misBoundSnapshotContentUID bool - misBoundSnapshotContentNamespace bool - misBoundSnapshotContentName bool - nilBoundVolumeSnapshotContentName bool - nilSnapshotStatus bool - nilReadyToUse bool - nilContentStatus bool - nilSnapshotHandle bool - } testcases := map[string]testcase{ "provision with volume snapshot data source": { volOpts: controller.ProvisionOptions{ @@ -2315,7 +2325,7 @@ func TestProvisionFromSnapshot(t *testing.T) { }, }, snapshotStatusReady: true, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, @@ -2829,35 +2839,13 @@ func TestProvisionFromSnapshot(t *testing.T) { client.AddReactor("get", "volumesnapshots", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { snap := newSnapshot(snapName, snapClassName, "snapcontent-snapuid", "snapuid", "claim", tc.snapshotStatusReady, nil, metaTimeNowUnix, resource.NewQuantity(requestedBytes, resource.BinarySI)) - if tc.nilSnapshotStatus { - snap.Status = nil - } - if tc.nilBoundVolumeSnapshotContentName { - snap.Status.BoundVolumeSnapshotContentName = nil - } - if tc.nilReadyToUse { - snap.Status.ReadyToUse = nil - } + return true, snap, nil }) client.AddReactor("get", "volumesnapshotcontents", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { content := newContent("snapcontent-snapuid", snapClassName, "sid", "pv-uid", "volume", "snapuid", snapName, &requestedBytes, &timeNow) - if tc.misBoundSnapshotContentUID { - content.Spec.VolumeSnapshotRef.UID = "another-snapshot-uid" - } - if tc.misBoundSnapshotContentName { - content.Spec.VolumeSnapshotRef.Name = "another-snapshot-name" - } - if tc.misBoundSnapshotContentNamespace { - content.Spec.VolumeSnapshotRef.Namespace = "another-snapshot-namespace" - } - if tc.nilContentStatus { - content.Status = nil - } - if tc.nilSnapshotHandle { - content.Status.SnapshotHandle = nil - } + return true, content, nil }) @@ -2933,6 +2921,35 @@ func TestProvisionFromSnapshot(t *testing.T) { } } +func (s *testcase) notNilSnapshotStatusTest(notNilSnapshotStatusTest bool) { + s.nilSnapshotStatus = notNilSnapshotStatusTest +} + +func (s *testcase) nilBoundVolumeSnapshotContentNameTest(nilBoundVolumeSnapshotContentName bool) { + s.nilBoundVolumeSnapshotContentName = nilBoundVolumeSnapshotContentName +} + +func (s *testcase) nilReadyToUseTest(nilReadyToUse bool) { + s.nilReadyToUse = nilReadyToUse +} +func (s *testcase) misBoundSnapshotContentUIDTest(misBoundSnapshotContentUID bool) { + s.misBoundSnapshotContentUID = misBoundSnapshotContentUID +} +func (s *testcase) misBoundSnapshotContentNameTest(misBoundSnapshotContentName bool) { + s.misBoundSnapshotContentName = misBoundSnapshotContentName +} + +func (s *testcase) misBoundSnapshotContentNamespaceTest(misBoundSnapshotContentNamespace bool) { + s.misBoundSnapshotContentNamespace = misBoundSnapshotContentNamespace +} +func (s *testcase) nilContentStatusTest(nilContentStatus bool) { + s.nilContentStatus = nilContentStatus +} + +func (s *testcase) nilSnapshotHandleTest(nilSnapshotHandle bool) { + s.nilSnapshotHandle = nilSnapshotHandle +} + // TestProvisionWithTopology is a basic test of provisioner integration with topology functions. func TestProvisionWithTopologyEnabled(t *testing.T) { defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.Topology, true)() @@ -3751,7 +3768,7 @@ func TestProvisionFromPVC(t *testing.T) { clonePVName string // name of the PV that srcName PVC has a claim on restoredVolSizeSmall bool // set to request a larger volSize than source PVC, default false restoredVolSizeBig bool // set to request a smaller volSize than source PVC, default false - expectedPVSpec *pvSpec // set to expected PVSpec on success, for deep comparison, default nil + expectedPVSpec pvSpec // set to expected PVSpec on success, for deep comparison, default nil cloneUnsupported bool // set to state clone feature not supported in capabilities, default false expectFinalizers bool // while set, expects clone protection finalizers to be set on a PVC sourcePVStatusPhase v1.PersistentVolumePhase // set to change source PV Status.Phase, default "Bound" @@ -3761,7 +3778,7 @@ func TestProvisionFromPVC(t *testing.T) { clonePVName: pvName, volOpts: generatePVCForProvisionFromPVC(srcNamespace, srcName, fakeSc1, requestedBytes, ""), expectFinalizers: true, - expectedPVSpec: &pvSpec{ + expectedPVSpec: pvSpec{ Name: pvName, ReclaimPolicy: v1.PersistentVolumeReclaimDelete, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},