Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use volume ID for creating PVC from another PVC #310

Merged
merged 1 commit into from
Jul 8, 2019
Merged

use volume ID for creating PVC from another PVC #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion pkg/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,9 +671,22 @@ func (p *csiProvisioner) getPVCSource(options controller.ProvisionOptions) (*csi
return nil, fmt.Errorf("error, new PVC request must be greater than or equal in size to the specified PVC data source, requested %v but source is %v", requestedSize, sourcePVC.Spec.Size())
}

if sourcePVC.Spec.VolumeName == "" {
return nil, fmt.Errorf("volume name is empty in source PVC %s", sourcePVC.Name)
}

sourcePV, err := p.client.CoreV1().PersistentVolumes().Get(sourcePVC.Spec.VolumeName, metav1.GetOptions{})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PVC.Spec.VolumeName field is "optional", would it be more reliable to just use the Volume field of the PVC directly? Then you can do something like volumeHandleToId to set the VolumeId field below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have volume field in PVC spec?

The approach I followed here is to get the PV name from PVC and then fetch volumeID from PV spec. do we have any other way of getting the volume name from the PVC?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you're correct here, Spec.VolumeName is the only handle; and the CO should set this when the claim is bound so this should be fine. You might want to add a check that the Spec.VolumeName value is not nil before issuing the Get call for the PV though.

if err != nil {
return nil, fmt.Errorf("error getting PV %s from api server: %v", sourcePVC.Spec.VolumeName, err)
}

if sourcePV.Spec.CSI == nil {
return nil, fmt.Errorf("error getting volume source from persistantVolumeClaim:persistanceVolume %s:%s", sourcePVC.Name, sourcePVC.Spec.VolumeName)
}

volumeSource := csi.VolumeContentSource_Volume{
Volume: &csi.VolumeContentSource_VolumeSource{
VolumeId: sourcePVC.Spec.VolumeName,
VolumeId: sourcePV.Spec.CSI.VolumeHandle,
msau42 marked this conversation as resolved.
Show resolved Hide resolved
},
}
klog.V(5).Infof("VolumeContentSource_Volume %+v", volumeSource)
Expand Down
77 changes: 64 additions & 13 deletions pkg/controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2146,6 +2146,7 @@ func TestProvisionFromPVC(t *testing.T) {
var requestedBytes int64 = 1000
invalidSCName := "invalid-sc"
srcName := "fake-pvc"
pvName := "test-testi"
deletePolicy := v1.PersistentVolumeReclaimDelete

type pvSpec struct {
Expand All @@ -2171,7 +2172,7 @@ func TestProvisionFromPVC(t *testing.T) {
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Expand All @@ -2193,7 +2194,7 @@ func TestProvisionFromPVC(t *testing.T) {
},
pvcStatusReady: true,
expectedPVSpec: &pvSpec{
Name: "test-testi",
Name: pvName,
ReclaimPolicy: v1.PersistentVolumeReclaimDelete,
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce},
Capacity: v1.ResourceList{
Expand All @@ -2216,7 +2217,7 @@ func TestProvisionFromPVC(t *testing.T) {
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Expand Down Expand Up @@ -2247,7 +2248,7 @@ func TestProvisionFromPVC(t *testing.T) {
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Expand All @@ -2270,7 +2271,7 @@ func TestProvisionFromPVC(t *testing.T) {
},
pvcStatusReady: true,
expectedPVSpec: nil,
cloneSupport: false,
cloneSupport: true,
expectErr: true,
},
"provision with pvc data source destination too small": {
Expand All @@ -2279,7 +2280,7 @@ func TestProvisionFromPVC(t *testing.T) {
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Expand All @@ -2302,7 +2303,7 @@ func TestProvisionFromPVC(t *testing.T) {
},
pvcStatusReady: true,
expectedPVSpec: nil,
cloneSupport: false,
cloneSupport: true,
expectErr: true,
},
"provision with pvc data source not found": {
Expand All @@ -2311,7 +2312,7 @@ func TestProvisionFromPVC(t *testing.T) {
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Expand All @@ -2334,7 +2335,7 @@ func TestProvisionFromPVC(t *testing.T) {
},
pvcStatusReady: true,
expectedPVSpec: nil,
cloneSupport: false,
cloneSupport: true,
expectErr: true,
},
"provision with pvc data source destination too large": {
Expand All @@ -2343,7 +2344,7 @@ func TestProvisionFromPVC(t *testing.T) {
ReclaimPolicy: &deletePolicy,
Parameters: map[string]string{},
},
PVName: "test-name",
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Expand All @@ -2366,7 +2367,39 @@ func TestProvisionFromPVC(t *testing.T) {
},
pvcStatusReady: true,
expectedPVSpec: nil,
cloneSupport: false,
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,
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,
},
}
Expand Down Expand Up @@ -2394,8 +2427,26 @@ func TestProvisionFromPVC(t *testing.T) {
clientSet = fakeclientset.NewSimpleClientset()

// 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, nil)

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

pluginCaps, controllerCaps := provisionFromPVCCapabilities()
if !tc.cloneSupport {
Expand Down