fix comments

kubernetes-csi · Jun 3, 2019 · 1351dec · 1351dec
1 parent d95d0a7
commit 1351dec
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 4 deletions.
diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -512,7 +512,7 @@ func (p *csiProvisioner) Provision(options controller.ProvisionOptions) (*v1.Per
 	if needSnapshotSupport {
 		contentSource := rep.GetVolume().ContentSource
 		if contentSource == nil {
-			sourceErr := fmt.Errorf("created volume contentsource missing")
+			sourceErr := fmt.Errorf("volume content source missing")
 			delReq := &csi.DeleteVolumeRequest{
 				VolumeId: rep.GetVolume().GetVolumeId(),
 			}
@@ -521,12 +521,12 @@ func (p *csiProvisioner) Provision(options controller.ProvisionOptions) (*v1.Per
 			defer cancel()
 			_, err := p.csiClient.DeleteVolume(ctx, delReq)
 			if err != nil {
-				sourceErr = fmt.Errorf("%v. Cleanup of volume %s failed, volume is orphaned: %v", sourceErr, pvName, err)
+				sourceErr = fmt.Errorf("%v. cleanup of volume %s failed, volume is orphaned: %v", sourceErr, pvName, err)
 			}
 			return nil, sourceErr
 		}
 
-		klog.Infof("created volume %s from snapshot %s is successful", pvName, contentSource.GetSnapshot().SnapshotId)
+		klog.Infof("created volume %s from snapshot %s successfully", pvName, contentSource.GetSnapshot().SnapshotId)
 	}
 
 	pv := &v1.PersistentVolume{

diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
@@ -1431,6 +1431,7 @@ func TestProvisionFromSnapshot(t *testing.T) {
 		snapshotStatusReady  bool
 		expectedPVSpec       *pvSpec
 		expectErr            bool
+		notPopulated         bool
 	}{
 		"provision with volume snapshot data source": {
 			volOpts: controller.ProvisionOptions{
@@ -1623,6 +1624,36 @@ func TestProvisionFromSnapshot(t *testing.T) {
 			snapshotStatusReady: false,
 			expectErr:           true,
 		},
+		"fail not populated volume content source": {
+			volOpts: controller.ProvisionOptions{
+				StorageClass: &storagev1.StorageClass{
+					Parameters: map[string]string{},
+				},
+				PVName: "test-name",
+				PVC: &v1.PersistentVolumeClaim{
+					ObjectMeta: metav1.ObjectMeta{
+						UID: "testid",
+					},
+					Spec: v1.PersistentVolumeClaimSpec{
+						Selector: nil,
+						Resources: v1.ResourceRequirements{
+							Requests: v1.ResourceList{
+								v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes, 10)),
+							},
+						},
+						AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
+						DataSource: &v1.TypedLocalObjectReference{
+							Name:     snapName,
+							Kind:     "VolumeSnapshot",
+							APIGroup: &apiGrp,
+						},
+					},
+				},
+			},
+			snapshotStatusReady: true,
+			expectErr:           true,
+			notPopulated:        true,
+		},
 	}
 
 	tmpdir := tempDir(t)
@@ -1666,7 +1697,13 @@ func TestProvisionFromSnapshot(t *testing.T) {
 		// When the following if condition is met, it is a valid create volume from snapshot
 		// operation and CreateVolume is expected to be called.
 		if tc.restoredVolSizeSmall == false && tc.wrongDataSource == false && tc.snapshotStatusReady {
-			controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1)
+			if tc.notPopulated {
+				out.Volume.ContentSource = nil
+				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 {
+				controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1)
+			}
 		}
 
 		pv, err := csiProvisioner.Provision(tc.volOpts)