From 2b1cf72a26d901f8035d691790d7b72025e5465d Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 29 Nov 2018 15:22:26 -0800 Subject: [PATCH] Refactor Provision tests and add some more for fsType --- pkg/controller/controller.go | 4 +- pkg/controller/controller_test.go | 331 +++++++++++++++++------------- 2 files changed, 193 insertions(+), 142 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index e8550973b..696eb6234 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -57,7 +57,7 @@ import ( ) const ( - parameterFsTypeKey = "csiFsType" + parameterFsTypeKey = "csifstype" provisionerSecretNameKey = "csiProvisionerSecretName" provisionerSecretNamespaceKey = "csiProvisionerSecretNamespace" @@ -430,7 +430,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis } } if fsTypesFound > 1 { - return nil, fmt.Errorf("fsType specified in paramaters with both \"fsType\" and \"csiFsType\" keys") + return nil, fmt.Errorf("fstype specified in paramaters with both \"fstype\" and \"csifstype\" keys") } if len(fsType) == 0 { fsType = defaultFSType diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index b20fe7280..f809d4bca 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -52,6 +52,11 @@ const ( timeout = 10 * time.Second ) +var ( + volumeModeFileSystem = v1.PersistentVolumeFilesystem + volumeModeBlock = v1.PersistentVolumeBlock +) + type csiConnection struct { conn *grpc.ClientConn } @@ -760,36 +765,32 @@ func TestGetSecretReference(t *testing.T) { } } -func TestProvision(t *testing.T) { - - var ( - requestedBytes int64 = 100 - volumeModeFileSystem = v1.PersistentVolumeFilesystem - volumeModeBlock = v1.PersistentVolumeBlock - ) +type provisioningTestcase struct { + volOpts controller.VolumeOptions + notNilSelector bool + driverNotReady bool + makeVolumeNameErr bool + getSecretRefErr bool + getCredentialsErr bool + volWithLessCap bool + expectedPVSpec *pvSpec + withSecretRefs bool + expectErr bool + expectCreateVolDo interface{} +} - type pvSpec struct { - Name string - ReclaimPolicy v1.PersistentVolumeReclaimPolicy - AccessModes []v1.PersistentVolumeAccessMode - VolumeMode *v1.PersistentVolumeMode - Capacity v1.ResourceList - CSIPVS *v1.CSIPersistentVolumeSource - } +type pvSpec struct { + Name string + ReclaimPolicy v1.PersistentVolumeReclaimPolicy + AccessModes []v1.PersistentVolumeAccessMode + VolumeMode *v1.PersistentVolumeMode + Capacity v1.ResourceList + CSIPVS *v1.CSIPersistentVolumeSource +} - testcases := map[string]struct { - volOpts controller.VolumeOptions - notNilSelector bool - driverNotReady bool - makeVolumeNameErr bool - getSecretRefErr bool - getCredentialsErr bool - volWithLessCap bool - expectedPVSpec *pvSpec - withSecretRefs bool - expectErr bool - expectCreateVolDo interface{} - }{ +func TestProvision(t *testing.T) { + var requestedBytes int64 = 100 + testcases := map[string]provisioningTestcase{ "normal provision": { volOpts: controller.VolumeOptions{ PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete, @@ -815,6 +816,48 @@ func TestProvision(t *testing.T) { }, }, }, + "multiple fsType provision": { + volOpts: controller.VolumeOptions{ + PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete, + PVName: "test-name", + PVC: createFakePVC(requestedBytes), + Parameters: map[string]string{ + "fstype": "ext3", + parameterFsTypeKey: "ext4", + }, + }, + expectErr: true, + }, + "provision with csiFsKey": { + volOpts: controller.VolumeOptions{ + PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete, + PVName: "test-name", + PVC: createFakePVC(requestedBytes), + Parameters: map[string]string{ + parameterFsTypeKey: "ext3", + }, + }, + expectedPVSpec: &pvSpec{ + Name: "test-testi", + ReclaimPolicy: v1.PersistentVolumeReclaimDelete, + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): bytesToGiQuantity(requestedBytes), + }, + CSIPVS: &v1.CSIPersistentVolumeSource{ + Driver: "test-driver", + VolumeHandle: "test-volume-id", + FSType: "ext3", + VolumeAttributes: map[string]string{ + "storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner", + }, + }, + }, + expectCreateVolDo: func(ctx context.Context, req *csi.CreateVolumeRequest) { + if len(req.Parameters) != 0 { + t.Errorf("Parameters should have been stripped") + } + }, + }, "provision with access mode multi node multi writer": { volOpts: controller.VolumeOptions{ PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete, @@ -1199,119 +1242,8 @@ func TestProvision(t *testing.T) { }, } - mockController, driver, identityServer, controllerServer, csiConn, err := createMockServer(t) - if err != nil { - t.Fatal(err) - } - defer mockController.Finish() - defer driver.Stop() - for k, tc := range testcases { - var clientSet kubernetes.Interface - - if tc.withSecretRefs { - clientSet = fakeclientset.NewSimpleClientset(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ctrlpublishsecret", - Namespace: "default", - }, - }, &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nodestagesecret", - Namespace: "default", - }, - }, &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nodepublishsecret", - Namespace: "default", - }, - }) - } else { - clientSet = fakeclientset.NewSimpleClientset() - } - - csiProvisioner := NewCSIProvisioner(clientSet, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil) - - out := &csi.CreateVolumeResponse{ - Volume: &csi.Volume{ - CapacityBytes: requestedBytes, - VolumeId: "test-volume-id", - }, - } - - if tc.withSecretRefs { - tc.volOpts.Parameters[controllerPublishSecretNameKey] = "ctrlpublishsecret" - tc.volOpts.Parameters[controllerPublishSecretNamespaceKey] = "default" - tc.volOpts.Parameters[nodeStageSecretNameKey] = "nodestagesecret" - tc.volOpts.Parameters[nodeStageSecretNamespaceKey] = "default" - tc.volOpts.Parameters[nodePublishSecretNameKey] = "nodepublishsecret" - tc.volOpts.Parameters[nodePublishSecretNamespaceKey] = "default" - } - - if tc.notNilSelector { - tc.volOpts.PVC.Spec.Selector = &metav1.LabelSelector{} - } else if tc.driverNotReady { - identityServer.EXPECT().GetPluginCapabilities(gomock.Any(), gomock.Any()).Return(nil, errors.New("driver not ready")).Times(1) - } else if tc.makeVolumeNameErr { - tc.volOpts.PVC.ObjectMeta.UID = "" - provisionMockServerSetupExpectations(identityServer, controllerServer) - } else if tc.getSecretRefErr { - tc.volOpts.Parameters[provisionerSecretNameKey] = "" - provisionMockServerSetupExpectations(identityServer, controllerServer) - } else if tc.getCredentialsErr { - tc.volOpts.Parameters[provisionerSecretNameKey] = "secretx" - tc.volOpts.Parameters[provisionerSecretNamespaceKey] = "default" - provisionMockServerSetupExpectations(identityServer, controllerServer) - } else if tc.volWithLessCap { - out.Volume.CapacityBytes = int64(80) - provisionMockServerSetupExpectations(identityServer, controllerServer) - controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1) - controllerServer.EXPECT().DeleteVolume(gomock.Any(), gomock.Any()).Return(&csi.DeleteVolumeResponse{}, nil).Times(1) - } else if tc.expectCreateVolDo != nil { - provisionMockServerSetupExpectations(identityServer, controllerServer) - controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Do(tc.expectCreateVolDo).Return(out, nil).Times(1) - } else { - // Setup regular mock call expectations. - provisionMockServerSetupExpectations(identityServer, controllerServer) - controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1) - } - - pv, err := csiProvisioner.Provision(tc.volOpts) - if tc.expectErr && err == nil { - t.Errorf("test %q: Expected error, got none", k) - } - if !tc.expectErr && err != nil { - t.Errorf("test %q: got error: %v", k, err) - } - - if tc.expectedPVSpec != nil { - if pv.Name != tc.expectedPVSpec.Name { - t.Errorf("test %q: expected PV name: %q, got: %q", k, tc.expectedPVSpec.Name, pv.Name) - } - - if pv.Spec.PersistentVolumeReclaimPolicy != tc.expectedPVSpec.ReclaimPolicy { - t.Errorf("test %q: expected reclaim policy: %v, got: %v", k, tc.expectedPVSpec.ReclaimPolicy, pv.Spec.PersistentVolumeReclaimPolicy) - } - - if !reflect.DeepEqual(pv.Spec.AccessModes, tc.expectedPVSpec.AccessModes) { - t.Errorf("test %q: expected access modes: %v, got: %v", k, tc.expectedPVSpec.AccessModes, pv.Spec.AccessModes) - } - - if !reflect.DeepEqual(pv.Spec.VolumeMode, tc.expectedPVSpec.VolumeMode) { - t.Errorf("test %q: expected volumeMode: %v, got: %v", k, tc.expectedPVSpec.VolumeMode, pv.Spec.VolumeMode) - } - - if !reflect.DeepEqual(pv.Spec.Capacity, tc.expectedPVSpec.Capacity) { - t.Errorf("test %q: expected capacity: %v, got: %v", k, tc.expectedPVSpec.Capacity, pv.Spec.Capacity) - } - - if tc.expectedPVSpec.CSIPVS != nil { - if !reflect.DeepEqual(pv.Spec.PersistentVolumeSource.CSI, tc.expectedPVSpec.CSIPVS) { - t.Errorf("test %q: expected PV: %v, got: %v", k, tc.expectedPVSpec.CSIPVS, pv.Spec.PersistentVolumeSource.CSI) - } - } - - } + runProvisionTest(t, k, tc, requestedBytes) } } @@ -1344,6 +1276,125 @@ func newSnapshot(name, className, boundToContent, snapshotUID, claimName string, return &snapshot } +func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64) { + t.Logf("Running test: %v", k) + + mockController, driver, identityServer, controllerServer, csiConn, err := createMockServer(t) + if err != nil { + t.Fatal(err) + } + defer mockController.Finish() + defer driver.Stop() + + var clientSet kubernetes.Interface + + if tc.withSecretRefs { + clientSet = fakeclientset.NewSimpleClientset(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ctrlpublishsecret", + Namespace: "default", + }, + }, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodestagesecret", + Namespace: "default", + }, + }, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodepublishsecret", + Namespace: "default", + }, + }) + } else { + clientSet = fakeclientset.NewSimpleClientset() + } + + csiProvisioner := NewCSIProvisioner(clientSet, nil, driver.Address(), 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil) + + out := &csi.CreateVolumeResponse{ + Volume: &csi.Volume{ + CapacityBytes: requestedBytes, + VolumeId: "test-volume-id", + }, + } + + if tc.withSecretRefs { + tc.volOpts.Parameters[controllerPublishSecretNameKey] = "ctrlpublishsecret" + tc.volOpts.Parameters[controllerPublishSecretNamespaceKey] = "default" + tc.volOpts.Parameters[nodeStageSecretNameKey] = "nodestagesecret" + tc.volOpts.Parameters[nodeStageSecretNamespaceKey] = "default" + tc.volOpts.Parameters[nodePublishSecretNameKey] = "nodepublishsecret" + tc.volOpts.Parameters[nodePublishSecretNamespaceKey] = "default" + } + + if tc.notNilSelector { + tc.volOpts.PVC.Spec.Selector = &metav1.LabelSelector{} + } else if tc.driverNotReady { + identityServer.EXPECT().GetPluginCapabilities(gomock.Any(), gomock.Any()).Return(nil, errors.New("driver not ready")).Times(1) + } else if tc.makeVolumeNameErr { + tc.volOpts.PVC.ObjectMeta.UID = "" + provisionMockServerSetupExpectations(identityServer, controllerServer) + } else if tc.getSecretRefErr { + tc.volOpts.Parameters[provisionerSecretNameKey] = "" + provisionMockServerSetupExpectations(identityServer, controllerServer) + } else if tc.getCredentialsErr { + tc.volOpts.Parameters[provisionerSecretNameKey] = "secretx" + tc.volOpts.Parameters[provisionerSecretNamespaceKey] = "default" + provisionMockServerSetupExpectations(identityServer, controllerServer) + } else if tc.volWithLessCap { + out.Volume.CapacityBytes = int64(80) + provisionMockServerSetupExpectations(identityServer, controllerServer) + controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1) + controllerServer.EXPECT().DeleteVolume(gomock.Any(), gomock.Any()).Return(&csi.DeleteVolumeResponse{}, nil).Times(1) + } else if tc.expectCreateVolDo != nil { + provisionMockServerSetupExpectations(identityServer, controllerServer) + controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Do(tc.expectCreateVolDo).Return(out, nil).Times(1) + } else { + // Setup regular mock call expectations. + provisionMockServerSetupExpectations(identityServer, controllerServer) + if !tc.expectErr { + controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1) + } + } + + pv, err := csiProvisioner.Provision(tc.volOpts) + if tc.expectErr && err == nil { + t.Errorf("test %q: Expected error, got none", k) + } + if !tc.expectErr && err != nil { + t.Errorf("test %q: got error: %v", k, err) + } + + if tc.expectedPVSpec != nil { + if pv.Name != tc.expectedPVSpec.Name { + t.Errorf("test %q: expected PV name: %q, got: %q", k, tc.expectedPVSpec.Name, pv.Name) + } + + if pv.Spec.PersistentVolumeReclaimPolicy != tc.expectedPVSpec.ReclaimPolicy { + t.Errorf("test %q: expected reclaim policy: %v, got: %v", k, tc.expectedPVSpec.ReclaimPolicy, pv.Spec.PersistentVolumeReclaimPolicy) + } + + if !reflect.DeepEqual(pv.Spec.AccessModes, tc.expectedPVSpec.AccessModes) { + t.Errorf("test %q: expected access modes: %v, got: %v", k, tc.expectedPVSpec.AccessModes, pv.Spec.AccessModes) + } + + if !reflect.DeepEqual(pv.Spec.VolumeMode, tc.expectedPVSpec.VolumeMode) { + t.Errorf("test %q: expected volumeMode: %v, got: %v", k, tc.expectedPVSpec.VolumeMode, pv.Spec.VolumeMode) + } + + if !reflect.DeepEqual(pv.Spec.Capacity, tc.expectedPVSpec.Capacity) { + t.Errorf("test %q: expected capacity: %v, got: %v", k, tc.expectedPVSpec.Capacity, pv.Spec.Capacity) + } + + if tc.expectedPVSpec.CSIPVS != nil { + if !reflect.DeepEqual(pv.Spec.PersistentVolumeSource.CSI, tc.expectedPVSpec.CSIPVS) { + t.Errorf("test %q: expected PV: %v, got: %v", k, tc.expectedPVSpec.CSIPVS, pv.Spec.PersistentVolumeSource.CSI) + } + } + + } +} + // newContent returns a new content with given attributes func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *int64, creationTime *int64) *crdv1.VolumeSnapshotContent { content := crdv1.VolumeSnapshotContent{