diff --git a/cmd/csi-provisioner/csi-provisioner.go b/cmd/csi-provisioner/csi-provisioner.go index a07943b77b..126e8fe210 100644 --- a/cmd/csi-provisioner/csi-provisioner.go +++ b/cmd/csi-provisioner/csi-provisioner.go @@ -87,6 +87,7 @@ func init() { if err != nil { glog.Fatalf("Failed to create client: %v", err) } + // snapclientset.NewForConfig creates a new Clientset for VolumesnapshotV1alpha1Client snapClient, err := snapclientset.NewForConfig(config) if err != nil { glog.Fatalf("Failed to create snapshot client: %v", err) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index a38a994cbb..15711355d3 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -343,7 +343,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis return nil, fmt.Errorf("claim Selector is not supported") } - var needSnapshotSupport bool + var needSnapshotSupport bool = false if options.PVC.Spec.DataSource != nil { // PVC.Spec.DataSource.Name is the name of the VolumeSnapshot API object if options.PVC.Spec.DataSource.Name == "" { @@ -394,7 +394,7 @@ func (p *csiProvisioner) Provision(options controller.VolumeOptions) (*v1.Persis req.VolumeContentSource = volumeContentSource } - glog.Infof("CreateVolumeRequest %+v", req) + glog.V(5).Infof("CreateVolumeRequest %+v", req) rep := &csi.CreateVolumeResponse{} @@ -529,6 +529,10 @@ func (p *csiProvisioner) getVolumeContentSource(options controller.VolumeOptions } glog.V(5).Infof("VolumeSnapshotContent %+v", snapContentObj) + if snapContentObj.Spec.VolumeSnapshotSource.CSI == nil || len(snapContentObj.Spec.VolumeSnapshotSource.CSI.SnapshotHandle) == 0 { + return nil, fmt.Errorf("error getting snapshot handle from snapshot:snapshotcontent %s:%s", snapshotObj.Name, snapshotObj.Spec.SnapshotContentName) + } + snapshotSource := csi.VolumeContentSource_Snapshot{ Snapshot: &csi.VolumeContentSource_SnapshotSource{ Id: snapContentObj.Spec.VolumeSnapshotSource.CSI.SnapshotHandle, @@ -537,7 +541,10 @@ func (p *csiProvisioner) getVolumeContentSource(options controller.VolumeOptions glog.V(5).Infof("VolumeContentSource_Snapshot %+v", snapshotSource) if snapshotObj.Status.RestoreSize != nil { - capacity := options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] + capacity, exists := options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] + if !exists { + return nil, fmt.Errorf("error getting capacity for PVC %s when creating snapshot %s", options.PVC.Name, snapshotObj.Name) + } volSizeBytes := capacity.Value() glog.V(5).Infof("Requested volume size is %d and snapshot size is %d for the source snapshot %s", int64(volSizeBytes), int64(snapshotObj.Status.RestoreSize.Value()), snapshotObj.Name) // When restoring volume from a snapshot, the volume size should diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index cf46647d53..4d301d1bfa 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -620,6 +620,8 @@ func provisionMockServerSetupExpectations(identityServer *driver.MockIdentitySer }, nil).Times(1) } +// provisionFromSnapshotMockServerSetupExpectations mocks plugin and controller capabilities reported +// by a CSI plugin that supports the snapshot feature func provisionFromSnapshotMockServerSetupExpectations(identityServer *driver.MockIdentityServer, controllerServer *driver.MockControllerServer) { identityServer.EXPECT().GetPluginCapabilities(gomock.Any(), gomock.Any()).Return(&csi.GetPluginCapabilitiesResponse{ Capabilities: []*csi.PluginCapability{ @@ -1166,7 +1168,7 @@ func TestProvisionFromSnapshot(t *testing.T) { volOpts controller.VolumeOptions restoredVolSizeSmall bool wrongDataSource bool - validSnapshotStatus bool + snapshotStatusReady bool expectedPVSpec *pvSpec expectErr bool }{ @@ -1195,7 +1197,7 @@ func TestProvisionFromSnapshot(t *testing.T) { }, Parameters: map[string]string{}, }, - validSnapshotStatus: true, + snapshotStatusReady: true, expectedPVSpec: &pvSpec{ Name: "test-testi", ReclaimPolicy: v1.PersistentVolumeReclaimDelete, @@ -1238,7 +1240,7 @@ func TestProvisionFromSnapshot(t *testing.T) { Parameters: map[string]string{}, }, restoredVolSizeSmall: true, - validSnapshotStatus: true, + snapshotStatusReady: true, expectErr: true, }, "fail empty snapshot name": { @@ -1346,7 +1348,7 @@ func TestProvisionFromSnapshot(t *testing.T) { }, Parameters: map[string]string{}, }, - validSnapshotStatus: false, + snapshotStatusReady: false, expectErr: true, }, } @@ -1364,7 +1366,7 @@ func TestProvisionFromSnapshot(t *testing.T) { client := &fake.Clientset{} client.AddReactor("get", "volumesnapshots", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - snap := newSnapshot(snapName, snapClassName, "snapcontent-snapuid", "snapuid", "claim", tc.validSnapshotStatus, nil, metaTimeNowUnix, resource.NewQuantity(requestedBytes, resource.BinarySI)) + snap := newSnapshot(snapName, snapClassName, "snapcontent-snapuid", "snapuid", "claim", tc.snapshotStatusReady, nil, metaTimeNowUnix, resource.NewQuantity(requestedBytes, resource.BinarySI)) return true, snap, nil }) @@ -1382,11 +1384,19 @@ func TestProvisionFromSnapshot(t *testing.T) { }, } - // Setup mock call expectations. + // Setup mock call expectations. If tc.wrongDataSource is false, DataSource is valid + // and the controller will proceed to check whether the plugin supports snapshot. + // So in this case, we need the plugin to report snapshot support capabilities; + // Otherwise, the controller will fail the operation so it won't check the capabilities. if tc.wrongDataSource == false { provisionFromSnapshotMockServerSetupExpectations(identityServer, controllerServer) } - if tc.restoredVolSizeSmall == false && tc.wrongDataSource == false && tc.validSnapshotStatus { + // If tc.restoredVolSizeSmall is true, or tc.wrongDataSource is true, or + // tc.snapshotStatusReady is false, create volume from snapshot operation will fail + // early and therefore CreateVolume is not expected to be called. + // 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) }