From 49f7de70121deae6211cbe75ad3e8d96bbc67411 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 12 Sep 2019 16:23:15 -0700 Subject: [PATCH] Ignore non-migrated in-tree PVC when provisioning --- pkg/controller/controller.go | 10 +++ pkg/controller/controller_test.go | 123 +++++++++++++++++++++++------- 2 files changed, 105 insertions(+), 28 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index ce3931fe55..abf57178cb 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -114,6 +114,8 @@ const ( tokenPVNameKey = "pv.name" tokenPVCNameKey = "pvc.name" tokenPVCNameSpaceKey = "pvc.namespace" + + annStorageProvisioner = "volume.beta.kubernetes.io/storage-provisioner" ) var ( @@ -367,6 +369,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 diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 705267c16d..fe8f8abd3b 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -62,6 +62,8 @@ const ( var ( volumeModeFileSystem = v1.PersistentVolumeFilesystem volumeModeBlock = v1.PersistentVolumeBlock + + driverNameAnnotation = map[string]string{annStorageProvisioner: driverName} ) type csiConnection struct { @@ -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 @@ -874,7 +877,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, @@ -925,7 +929,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, @@ -976,7 +981,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, @@ -1027,7 +1033,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, @@ -1235,7 +1242,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, @@ -1293,7 +1301,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, @@ -1322,7 +1331,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, @@ -1345,7 +1355,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) } } @@ -1378,7 +1430,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) @@ -1419,7 +1471,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{ @@ -1466,7 +1518,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 == "" { @@ -1582,7 +1634,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, @@ -1626,7 +1679,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, @@ -1656,7 +1710,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, @@ -1685,7 +1740,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, @@ -1714,7 +1770,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, @@ -1743,7 +1800,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, @@ -2265,7 +2323,8 @@ func TestProvisionFromPVC(t *testing.T) { PVName: "test-name", PVC: &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - UID: "testid", + UID: "testid", + Annotations: driverNameAnnotation, }, Spec: v1.PersistentVolumeClaimSpec{ StorageClassName: &fakeSc1, @@ -2311,7 +2370,8 @@ func TestProvisionFromPVC(t *testing.T) { PVName: "test-name", PVC: &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - UID: "testid", + UID: "testid", + Annotations: map[string]string{annStorageProvisioner: driverName}, }, Spec: v1.PersistentVolumeClaimSpec{ StorageClassName: &fakeSc1, @@ -2343,7 +2403,8 @@ func TestProvisionFromPVC(t *testing.T) { PVName: "test-name", PVC: &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - UID: "testid", + UID: "testid", + Annotations: map[string]string{annStorageProvisioner: driverName}, }, Spec: v1.PersistentVolumeClaimSpec{ StorageClassName: &fakeSc2, @@ -2375,7 +2436,8 @@ func TestProvisionFromPVC(t *testing.T) { PVName: "test-name", PVC: &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - UID: "testid", + UID: "testid", + Annotations: map[string]string{annStorageProvisioner: driverName}, }, Spec: v1.PersistentVolumeClaimSpec{ StorageClassName: &fakeSc1, @@ -2408,7 +2470,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, @@ -2441,7 +2504,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, @@ -2473,7 +2537,8 @@ func TestProvisionFromPVC(t *testing.T) { PVName: "test-name", PVC: &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - UID: "testid", + UID: "testid", + Annotations: driverNameAnnotation, }, Spec: v1.PersistentVolumeClaimSpec{ Selector: nil, @@ -2505,7 +2570,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, @@ -2536,7 +2602,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,