Skip to content

Commit d933f30

Browse files
committed
Cherry-pick: Verify PV/PVC binding and driver
Manually resolved conflicts in the following files: pkg/controller/snapshot_controller.go pkg/controller/snapshot_create_test.go pkg/controller/snapshot_ready_test.go
1 parent 5bad087 commit d933f30

File tree

5 files changed

+156
-34
lines changed

5 files changed

+156
-34
lines changed

Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
REGISTRY_NAME=quay.io/k8scsi
1818
IMAGE_NAME=csi-snapshotter
19-
IMAGE_VERSION=v0.4.1
19+
IMAGE_VERSION=v0.4-canary
2020
IMAGE_TAG=$(REGISTRY_NAME)/$(IMAGE_NAME):$(IMAGE_VERSION)
2121

2222
REV=$(shell git describe --long --tags --match='v*' --dirty)

pkg/controller/framework_test.go

+13-5
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,15 @@ func newClaimArray(name, claimUID, capacity, boundToVolume string, phase v1.Pers
875875
}
876876

877877
// newVolume returns a new volume with given attributes
878-
func newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundToClaimName string, phase v1.PersistentVolumePhase, reclaimPolicy v1.PersistentVolumeReclaimPolicy, class string, annotations ...string) *v1.PersistentVolume {
878+
func newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundToClaimName string, phase v1.PersistentVolumePhase, reclaimPolicy v1.PersistentVolumeReclaimPolicy, class string, driver string, namespace string, annotations ...string) *v1.PersistentVolume {
879+
inDriverName := mockDriverName
880+
if driver != "" {
881+
inDriverName = driver
882+
}
883+
inNamespace := testNamespace
884+
if namespace != "" {
885+
inNamespace = namespace
886+
}
879887
volume := v1.PersistentVolume{
880888
ObjectMeta: metav1.ObjectMeta{
881889
Name: name,
@@ -889,7 +897,7 @@ func newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundTo
889897
},
890898
PersistentVolumeSource: v1.PersistentVolumeSource{
891899
CSI: &v1.CSIPersistentVolumeSource{
892-
Driver: mockDriverName,
900+
Driver: inDriverName,
893901
VolumeHandle: volumeHandle,
894902
},
895903
},
@@ -907,7 +915,7 @@ func newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundTo
907915
Kind: "PersistentVolumeClaim",
908916
APIVersion: "v1",
909917
UID: types.UID(boundToClaimUID),
910-
Namespace: testNamespace,
918+
Namespace: inNamespace,
911919
Name: boundToClaimName,
912920
}
913921
}
@@ -917,9 +925,9 @@ func newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundTo
917925

918926
// newVolumeArray returns array with a single volume that would be returned by
919927
// newVolume() with the same parameters.
920-
func newVolumeArray(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundToClaimName string, phase v1.PersistentVolumePhase, reclaimPolicy v1.PersistentVolumeReclaimPolicy, class string) []*v1.PersistentVolume {
928+
func newVolumeArray(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundToClaimName string, phase v1.PersistentVolumePhase, reclaimPolicy v1.PersistentVolumeReclaimPolicy, class string, driver string, namespace string) []*v1.PersistentVolume {
921929
return []*v1.PersistentVolume{
922-
newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundToClaimName, phase, reclaimPolicy, class),
930+
newVolume(name, volumeUID, volumeHandle, capacity, boundToClaimUID, boundToClaimName, phase, reclaimPolicy, class, driver, namespace),
923931
}
924932
}
925933

pkg/controller/snapshot_controller.go

+29
Original file line numberDiff line numberDiff line change
@@ -735,11 +735,40 @@ func (ctrl *csiSnapshotController) getVolumeFromVolumeSnapshot(snapshot *crdv1.V
735735
return nil, fmt.Errorf("failed to retrieve PV %s from the API server: %q", pvName, err)
736736
}
737737

738+
// Verify binding between PV/PVC is still valid
739+
bound := ctrl.IsVolumeBoundToClaim(pv, pvc)
740+
if bound == false {
741+
glog.Warningf("binding between PV %s and PVC %s is broken", pvName, pvc.Name)
742+
return nil, fmt.Errorf("claim in dataSource not bound or invalid")
743+
}
744+
745+
// Verify driver for PVC is the same as driver for VolumeSnapshot
746+
if pv.Spec.PersistentVolumeSource.CSI == nil || pv.Spec.PersistentVolumeSource.CSI.Driver != ctrl.snapshotterName {
747+
glog.Warningf("driver for PV %s is different from driver %s for snapshot %s", pvName, ctrl.snapshotterName, snapshot.Name)
748+
return nil, fmt.Errorf("claim in dataSource not bound or invalid")
749+
}
750+
738751
glog.V(5).Infof("getVolumeFromVolumeSnapshot: snapshot [%s] PV name [%s]", snapshot.Name, pvName)
739752

740753
return pv, nil
741754
}
742755

756+
// IsVolumeBoundToClaim returns true, if given volume is pre-bound or bound
757+
// to specific claim. Both claim.Name and claim.Namespace must be equal.
758+
// If claim.UID is present in volume.Spec.ClaimRef, it must be equal too.
759+
func (ctrl *csiSnapshotController) IsVolumeBoundToClaim(volume *v1.PersistentVolume, claim *v1.PersistentVolumeClaim) bool {
760+
if volume.Spec.ClaimRef == nil {
761+
return false
762+
}
763+
if claim.Name != volume.Spec.ClaimRef.Name || claim.Namespace != volume.Spec.ClaimRef.Namespace {
764+
return false
765+
}
766+
if volume.Spec.ClaimRef.UID != "" && claim.UID != volume.Spec.ClaimRef.UID {
767+
return false
768+
}
769+
return true
770+
}
771+
743772
func (ctrl *csiSnapshotController) getStorageClassFromVolumeSnapshot(snapshot *crdv1.VolumeSnapshot) (*storagev1.StorageClass, error) {
744773
// Get storage class from PVC or PV
745774
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)

0 commit comments

Comments
 (0)