Skip to content

Commit

Permalink
Merge pull request #325 from j-griffith/release-1.3
Browse files Browse the repository at this point in the history 
fix storageclass comparion for pvc datasource
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Aug 9, 2019
2 parents c4b733d + c76e342 commit 5c5deb3
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 17 deletions.
13 changes: 11 additions & 2 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,18 @@ func (p *csiProvisioner) getPVCSource(options controller.ProvisionOptions) (*csi
if sourcePVC.ObjectMeta.DeletionTimestamp != nil {
return nil, fmt.Errorf("the PVC DataSource %s is currently being deleted", options.PVC.Spec.DataSource.Name)
}
if sourcePVC.Spec.StorageClassName != options.PVC.Spec.StorageClassName {

if sourcePVC.Spec.StorageClassName == nil {
return nil, fmt.Errorf("the source PVC (%s) storageclass cannot be empty", sourcePVC.Name)
}

if options.PVC.Spec.StorageClassName == nil {
return nil, fmt.Errorf("the requested PVC (%s) storageclass cannot be empty", options.PVC.Name)
}

if *sourcePVC.Spec.StorageClassName != *options.PVC.Spec.StorageClassName {
return nil, fmt.Errorf("the source PVC and destination PVCs must be in the same storage class for cloning. Source is in %v, but new PVC is in %v",
sourcePVC.Spec.StorageClassName, options.PVC.Spec.StorageClassName)
*sourcePVC.Spec.StorageClassName, *options.PVC.Spec.StorageClassName)
}
capacity := options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
requestedSize := capacity.Value()
Expand Down
169 changes: 154 additions & 15 deletions pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controller

import (
"context"
"errors"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -2230,8 +2231,11 @@ func runDeleteTest(t *testing.T, k string, tc deleteTestcase) {
// TestProvisionFromPVC tests create volume clone
func TestProvisionFromPVC(t *testing.T) {
var requestedBytes int64 = 1000
invalidSCName := "invalid-sc"
fakeSc1 := "fake-sc-1"
fakeSc2 := "fake-sc-2"
srcName := "fake-pvc"
invalidPVC := "invalid-pv"
pvName := "test-testi"
deletePolicy := v1.PersistentVolumeReclaimDelete

type pvSpec struct {
Expand All @@ -2245,6 +2249,7 @@ func TestProvisionFromPVC(t *testing.T) {
testcases := map[string]struct {
volOpts controller.ProvisionOptions
restoredVolSizeSmall bool
restoredVolSizeBig bool
wrongDataSource bool
pvcStatusReady bool
expectedPVSpec *pvSpec
Expand All @@ -2263,7 +2268,8 @@ func TestProvisionFromPVC(t *testing.T) {
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
StorageClassName: &fakeSc1,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes, 10)),
Expand Down Expand Up @@ -2308,7 +2314,8 @@ func TestProvisionFromPVC(t *testing.T) {
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
StorageClassName: &fakeSc1,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes, 10)),
Expand Down Expand Up @@ -2339,7 +2346,7 @@ func TestProvisionFromPVC(t *testing.T) {
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &invalidSCName,
StorageClassName: &fakeSc2,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
Expand Down Expand Up @@ -2371,11 +2378,11 @@ func TestProvisionFromPVC(t *testing.T) {
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &invalidSCName,
StorageClassName: &fakeSc1,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes-1, 10)),
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes+1, 10)),
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
Expand All @@ -2386,12 +2393,78 @@ func TestProvisionFromPVC(t *testing.T) {
},
},
},
pvcStatusReady: true,
restoredVolSizeSmall: true,
expectedPVSpec: nil,
cloneSupport: true,
expectErr: true,
},
"provision with pvc data source destination too large": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes+1, 10)),
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
DataSource: &v1.TypedLocalObjectReference{
Name: srcName,
Kind: "PersistentVolumeClaim",
},
},
},
},
pvcStatusReady: true,
restoredVolSizeBig: true,
expectedPVSpec: nil,
cloneSupport: true,
expectErr: true,
},
"provision with pvc data source not found": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes-1, 10)),
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
DataSource: &v1.TypedLocalObjectReference{
Name: "source-not-found",
Kind: "PersistentVolumeClaim",
},
},
},
},
pvcStatusReady: true,
expectedPVSpec: nil,
cloneSupport: false,
expectErr: true,
},
"provision with pvc data source not found": {
"provision with source pvc storageclass nil": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Expand All @@ -2403,16 +2476,16 @@ func TestProvisionFromPVC(t *testing.T) {
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &invalidSCName,
Selector: nil,
StorageClassName: &fakeSc1,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestedBytes-1, 10)),
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
DataSource: &v1.TypedLocalObjectReference{
Name: "source-not-found",
Name: "pvc-sc-nil",
Kind: "PersistentVolumeClaim",
},
},
Expand All @@ -2423,19 +2496,50 @@ func TestProvisionFromPVC(t *testing.T) {
cloneSupport: false,
expectErr: true,
},
"provision with pvc data source destination too large": {
"provision with requested pvc storageclass nil": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
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-1, 10)),
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
DataSource: &v1.TypedLocalObjectReference{
Name: srcName,
Kind: "PersistentVolumeClaim",
},
},
},
},
pvcStatusReady: true,
expectedPVSpec: nil,
cloneSupport: true,
expectErr: true,
},
"provision with pvc data source when source pv not found": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "invalid-pv",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &invalidSCName,
StorageClassName: &fakeSc1,
Selector: nil,
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
Expand All @@ -2444,7 +2548,7 @@ func TestProvisionFromPVC(t *testing.T) {
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
DataSource: &v1.TypedLocalObjectReference{
Name: srcName,
Name: invalidPVC,
Kind: "PersistentVolumeClaim",
},
},
Expand Down Expand Up @@ -2479,9 +2583,31 @@ func TestProvisionFromPVC(t *testing.T) {

clientSet = fakeclientset.NewSimpleClientset()

pv := &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: pvName,
},
Spec: v1.PersistentVolumeSpec{
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{
Driver: driverName,
VolumeHandle: "test-volume-id",
FSType: "ext3",
VolumeAttributes: map[string]string{
"storage.kubernetes.io/csiProvisionerIdentity": "test-provisioner",
},
},
},
},
}

// Create a fake claim as our PVC DataSource
claim := fakeClaim(srcName, "fake-claim-uid", "1Gi", "test-pv", v1.ClaimBound, nil)
clientSet = fakeclientset.NewSimpleClientset(claim)
claim := fakeClaim(srcName, "fake-claim-uid", "1Gi", pvName, v1.ClaimBound, &fakeSc1)
// Create a fake claim with invalid PV
invalidClaim := fakeClaim(invalidPVC, "fake-claim-uid", "1Gi", "pv-not-present", v1.ClaimBound, &fakeSc1)
/// Create a fake claim as source PVC storageclass nil
scNilClaim := fakeClaim("pvc-sc-nil", "fake-claim-uid", "1Gi", pvName, v1.ClaimBound, nil)
clientSet = fakeclientset.NewSimpleClientset(claim, scNilClaim, pv, invalidClaim)

pluginCaps, controllerCaps := provisionFromPVCCapabilities()
if !tc.cloneSupport {
Expand All @@ -2492,6 +2618,19 @@ func TestProvisionFromPVC(t *testing.T) {
controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1)

}
if tc.restoredVolSizeSmall {
controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(out, nil).Times(1)
// if the volume created is less than the requested size,
// deletevolume will be called
controllerServer.EXPECT().DeleteVolume(gomock.Any(), &csi.DeleteVolumeRequest{
VolumeId: "test-volume-id",
}).Return(&csi.DeleteVolumeResponse{}, nil).Times(1)
}

if tc.restoredVolSizeBig {
controllerServer.EXPECT().CreateVolume(gomock.Any(), gomock.Any()).Return(nil, errors.New("source volume size is bigger than requested volume size")).Times(1)
}

csiProvisioner := NewCSIProvisioner(clientSet, 5*time.Second, "test-provisioner", "test", 5, csiConn.conn, nil, driverName, pluginCaps, controllerCaps, "", false)

pv, err := csiProvisioner.Provision(tc.volOpts)
Expand Down

0 comments on commit 5c5deb3

Please sign in to comment.