Skip to content

Commit

Permalink
Merge pull request #341 from davidz627/fix/checkAnnotation
Browse files Browse the repository at this point in the history 
Ignore provisioning for in-tree volumes that have not been migrated
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Sep 13, 2019
2 parents 61b9e46 + 3184db6 commit a1a8f48
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 29 deletions.
10 changes: 10 additions & 0 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ const (
tokenPVCNameSpaceKey = "pvc.namespace"

deleteVolumeRetryCount = 5

annStorageProvisioner = "volume.beta.kubernetes.io/storage-provisioner"
)

var (
Expand Down Expand Up @@ -369,6 +371,14 @@ func (p *csiProvisioner) ProvisionExt(options controller.ProvisionOptions) (*v1.
return nil, controller.ProvisioningFinished, errors.New("storage class was nil")
}

if options.PVC.Annotations[annStorageProvisioner] != p.driverName {
return nil, controller.ProvisioningFinished, &controller.IgnoredError{
Reason: fmt.Sprintf("PVC annotated with external-provisioner name %s does not match provisioner driver name %s. This could mean the PVC is not migrated",
options.PVC.Annotations[annStorageProvisioner],
p.driverName),
}
}

migratedVolume := false
if p.supportsMigrationFromInTreePluginName != "" {
// NOTE: we cannot depend on PVC.Annotations[volume.beta.kubernetes.io/storage-provisioner] to get
Expand Down
126 changes: 97 additions & 29 deletions pkg/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ const (
var (
volumeModeFileSystem = v1.PersistentVolumeFilesystem
volumeModeBlock = v1.PersistentVolumeBlock

driverNameAnnotation = map[string]string{annStorageProvisioner: driverName}
)

type csiConnection struct {
Expand Down Expand Up @@ -472,8 +474,9 @@ func provisionFromPVCCapabilities() (connection.PluginCapabilitySet, connection.
func createFakePVC(requestBytes int64) *v1.PersistentVolumeClaim {
return &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Name: "fake-pvc",
UID: "testid",
Name: "fake-pvc",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil, // Provisioner doesn't support selector
Expand Down Expand Up @@ -876,7 +879,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -927,7 +931,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -978,7 +983,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1029,7 +1035,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1237,7 +1244,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1296,7 +1304,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1325,7 +1334,8 @@ func TestProvision(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1376,7 +1386,49 @@ func TestProvision(t *testing.T) {
}

for k, tc := range testcases {
runProvisionTest(t, k, tc, requestedBytes)
runProvisionTest(t, k, tc, requestedBytes, driverName, "" /* no migration */)
}
}

func TestProvisionWithMigration(t *testing.T) {
inTreePluginName := "kubernetes.io/gce-pd"
migrationDriverName := "pd.csi.storage.gke.io"
var requestBytes int64 = 100000

deletePolicy := v1.PersistentVolumeReclaimDelete
testcases := map[string]provisioningTestcase{
"should ignore in-tree with migration": {
volOpts: controller.ProvisionOptions{
StorageClass: &storagev1.StorageClass{
Provisioner: inTreePluginName,
Parameters: map[string]string{
"fstype": "ext3",
},
ReclaimPolicy: &deletePolicy,
},
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
Name: "fake-pvc",
Annotations: map[string]string{annStorageProvisioner: inTreePluginName},
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil, // Provisioner doesn't support selector
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(strconv.FormatInt(requestBytes, 10)),
},
},
},
},
},
expectErr: true,
},
}

for k, tc := range testcases {
runProvisionTest(t, k, tc, requestBytes, migrationDriverName, inTreePluginName)
}
}

Expand Down Expand Up @@ -1409,7 +1461,7 @@ func newSnapshot(name, className, boundToContent, snapshotUID, claimName string,
return &snapshot
}

func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64) {
func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requestedBytes int64, provisionDriverName, supportsMigrationFromInTreePluginName string) {
t.Logf("Running test: %v", k)

tmpdir := tempDir(t)
Expand Down Expand Up @@ -1450,7 +1502,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested
}

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

out := &csi.CreateVolumeResponse{
Volume: &csi.Volume{
Expand Down Expand Up @@ -1500,7 +1552,7 @@ func runProvisionTest(t *testing.T, k string, tc provisioningTestcase, requested
t.Errorf("test %q: Expected error, got none", k)
}
if !tc.expectErr && err != nil {
t.Errorf("test %q: got error: %v", k, err)
t.Fatalf("test %q: got error: %v", k, err)
}

if tc.expectState == "" {
Expand Down Expand Up @@ -1621,7 +1673,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1665,7 +1718,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1695,7 +1749,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1724,7 +1779,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1753,7 +1809,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1782,7 +1839,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -1811,7 +1869,8 @@ func TestProvisionFromSnapshot(t *testing.T) {
PVName: "test-name",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -2310,7 +2369,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2356,7 +2416,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2388,7 +2449,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc2,
Expand Down Expand Up @@ -2420,7 +2482,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: map[string]string{annStorageProvisioner: driverName},
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2453,7 +2516,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2486,7 +2550,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down Expand Up @@ -2518,7 +2583,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -2550,7 +2616,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: pvName,
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
Selector: nil,
Expand Down Expand Up @@ -2581,7 +2648,8 @@ func TestProvisionFromPVC(t *testing.T) {
PVName: "invalid-pv",
PVC: &v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
UID: "testid",
UID: "testid",
Annotations: driverNameAnnotation,
},
Spec: v1.PersistentVolumeClaimSpec{
StorageClassName: &fakeSc1,
Expand Down

0 comments on commit a1a8f48

Please sign in to comment.