diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go index e4a54d36171..31eb6939580 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go @@ -26,14 +26,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" kcache "k8s.io/client-go/tools/cache" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/csi" "k8s.io/kubernetes/pkg/volume/util" @@ -431,15 +428,9 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) { informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, time.Second*1) var plugins []volume.VolumePlugin - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, tc.csiMigration)() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InTreePluginGCEUnregister, tc.csiMigration)() + plugins = append(plugins, controllervolumetesting.CreateTestPlugin()...) + plugins = append(plugins, csi.ProbeVolumePlugins()...) - if tc.csiMigration { - // if InTreePluginGCEUnregister is enabled, only the CSI plugin is registered but not the in-tree one - plugins = append(plugins, csi.ProbeVolumePlugins()...) - } else { - plugins = controllervolumetesting.CreateTestPlugin() - } nodeInformer := informerFactory.Core().V1().Nodes().Informer() podInformer := informerFactory.Core().V1().Pods().Informer() pvInformer := informerFactory.Core().V1().PersistentVolumes().Informer() @@ -522,7 +513,14 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) { podInformer.GetIndexer().Add(newPod) } if tc.pvName != "" { - newPv := controllervolumetesting.NewPV(tc.pvName, tc.volName) + var newPv *v1.PersistentVolume + if tc.csiMigration { + // NewPV returns a GCEPersistentDisk volume, which is migrated. + newPv = controllervolumetesting.NewPV(tc.pvName, tc.volName) + } else { + // Otherwise use NFS, which is not subject to migration. + newPv = controllervolumetesting.NewNFSPV(tc.pvName, tc.volName) + } _, err = adc.kubeClient.CoreV1().PersistentVolumes().Create(context.TODO(), newPv, metav1.CreateOptions{}) if err != nil { t.Fatalf("Run failed with error. Failed to create a new pv: <%v>", err) diff --git a/pkg/controller/volume/attachdetach/metrics/metrics_test.go b/pkg/controller/volume/attachdetach/metrics/metrics_test.go index ff038e62a1b..4cfd4390682 100644 --- a/pkg/controller/volume/attachdetach/metrics/metrics_test.go +++ b/pkg/controller/volume/attachdetach/metrics/metrics_test.go @@ -26,19 +26,16 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" - featuregatetesting "k8s.io/component-base/featuregate/testing" csitrans "k8s.io/csi-translation-lib" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume/csimigration" volumetesting "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util/types" ) func TestVolumesInUseMetricCollection(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() fakeVolumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) fakeClient := &fake.Clientset{} @@ -101,6 +98,7 @@ func TestVolumesInUseMetricCollection(t *testing.T) { v1.ResourceName(v1.ResourceStorage): resource.MustParse("5G"), }, PersistentVolumeSource: v1.PersistentVolumeSource{ + // Note that as GCE CSI Migration is completed, this is handled by the PD CSI plugin. GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{}, }, AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce, v1.ReadOnlyMany}, @@ -128,7 +126,7 @@ func TestVolumesInUseMetricCollection(t *testing.T) { t.Errorf("Expected one volume in use got %d", len(nodeUseMap)) } testNodeMetric := nodeUseMap["metric-test-host"] - pluginUseCount, ok := testNodeMetric["fake-plugin"] + pluginUseCount, ok := testNodeMetric["fake-plugin:pd.csi.storage.gke.io"] if !ok { t.Errorf("Expected fake plugin pvc got nothing") } diff --git a/pkg/controller/volume/attachdetach/populator/desired_state_of_world_populator_test.go b/pkg/controller/volume/attachdetach/populator/desired_state_of_world_populator_test.go index ab772d6c6b8..5d968dfe3bb 100644 --- a/pkg/controller/volume/attachdetach/populator/desired_state_of_world_populator_test.go +++ b/pkg/controller/volume/attachdetach/populator/desired_state_of_world_populator_test.go @@ -26,19 +26,15 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" - featuregatetesting "k8s.io/component-base/featuregate/testing" csitrans "k8s.io/csi-translation-lib" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume/csimigration" volumetesting "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util" ) func TestFindAndAddActivePods_FindAndRemoveDeletedPods(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - fakeVolumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) fakeClient := &fake.Clientset{} @@ -59,8 +55,8 @@ func TestFindAndAddActivePods_FindAndRemoveDeletedPods(t *testing.T) { { Name: "dswp-test-volume-name", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "dswp-test-fake-device", + RBD: &v1.RBDVolumeSource{ + RBDImage: "dswp-test-fake-device", }, }, }, @@ -75,7 +71,7 @@ func TestFindAndAddActivePods_FindAndRemoveDeletedPods(t *testing.T) { podName := util.GetUniquePodName(pod) - generatedVolumeName := "fake-plugin/" + pod.Spec.Volumes[0].GCEPersistentDisk.PDName + generatedVolumeName := "fake-plugin/" + pod.Spec.Volumes[0].RBD.RBDImage pvcLister := fakeInformerFactory.Core().V1().PersistentVolumeClaims().Lister() pvLister := fakeInformerFactory.Core().V1().PersistentVolumes().Lister() diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index 3ff5f3af9ab..c9586b75dd7 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -296,6 +296,23 @@ func NewPV(pvName, volumeName string) *v1.PersistentVolume { } } +// Returns an NFS PV. This can be used for an in-tree volume that is not migrated (unlike NewPV, which uses the GCE persistent disk). +func NewNFSPV(pvName, volumeName string) *v1.PersistentVolume { + return &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(pvName), + Name: pvName, + }, + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{ + NFS: &v1.NFSVolumeSource{ + Server: volumeName, + }, + }, + }, + } +} + func attachVolumeToNode(nodes *v1.NodeList, volumeName, nodeName string) { // if nodeName exists, get the object.. if not create node object var node *v1.Node @@ -366,7 +383,14 @@ func (plugin *TestPlugin) GetVolumeName(spec *volume.Spec) (string, error) { if spec.Volume != nil { return spec.Name(), nil } else if spec.PersistentVolume != nil { - return spec.PersistentVolume.Spec.PersistentVolumeSource.GCEPersistentDisk.PDName, nil + if spec.PersistentVolume.Spec.PersistentVolumeSource.GCEPersistentDisk != nil { + return spec.PersistentVolume.Spec.PersistentVolumeSource.GCEPersistentDisk.PDName, nil + } else if spec.PersistentVolume.Spec.PersistentVolumeSource.NFS != nil { + return spec.PersistentVolume.Spec.PersistentVolumeSource.NFS.Server, nil + } else if spec.PersistentVolume.Spec.PersistentVolumeSource.RBD != nil { + return spec.PersistentVolume.Spec.PersistentVolumeSource.RBD.RBDImage, nil + } + return "", fmt.Errorf("GetVolumeName called with unexpected PersistentVolume: %v", spec) } else { return "", nil } diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index ae68d961d9d..661c32c4148 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -34,7 +34,6 @@ import ( storagelisters "k8s.io/client-go/listers/storage/v1" core "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - "k8s.io/component-base/featuregate" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-helpers/storage/volume" csitrans "k8s.io/csi-translation-lib" @@ -471,107 +470,82 @@ func makeStorageClass(scName string, mode *storagev1.VolumeBindingMode) *storage } func TestAnnealMigrationAnnotations(t *testing.T) { + // The gce-pd plugin is used to test a migrated plugin (as the feature is + // locked as of 1.25), and rbd is used as a non-migrated plugin (still alpha + // as of 1.25). As plugins are migrated, rbd should be changed to a non- + // migrated plugin. If there are no other non-migrated plugins, then those + // test cases are moot and they can be removed (keeping only the test cases + // with gce-pd). const testPlugin = "non-migrated-plugin" - const gcePlugin = "kubernetes.io/gce-pd" - const gceDriver = "pd.csi.storage.gke.io" + const migratedPlugin = "kubernetes.io/gce-pd" + const migratedDriver = "pd.csi.storage.gke.io" + const nonmigratedPlugin = "kubernetes.io/rbd" + const nonmigratedDriver = "rbd.csi.ceph.com" tests := []struct { name string volumeAnnotations map[string]string expVolumeAnnotations map[string]string claimAnnotations map[string]string expClaimAnnotations map[string]string - migratedDriverGates []featuregate.Feature - disabledDriverGates []featuregate.Feature + testMigration bool }{ { - name: "migration on for GCE", - volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin}, - expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver}, - claimAnnotations: map[string]string{volume.AnnStorageProvisioner: gcePlugin}, - expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: gcePlugin, volume.AnnMigratedTo: gceDriver}, - migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, - disabledDriverGates: []featuregate.Feature{}, + name: "migration on", + volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: migratedPlugin}, + expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: migratedPlugin, volume.AnnMigratedTo: migratedDriver}, + claimAnnotations: map[string]string{volume.AnnStorageProvisioner: migratedPlugin}, + expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: migratedPlugin, volume.AnnMigratedTo: migratedDriver}, }, { - name: "migration on for GCE with Beta storage provisioner annontation", - volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin}, - expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver}, - claimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: gcePlugin}, - expClaimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: gcePlugin, volume.AnnMigratedTo: gceDriver}, - migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, - disabledDriverGates: []featuregate.Feature{}, + name: "migration on with Beta storage provisioner annontation", + volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: migratedPlugin}, + expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: migratedPlugin, volume.AnnMigratedTo: migratedDriver}, + claimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: migratedPlugin}, + expClaimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: migratedPlugin, volume.AnnMigratedTo: migratedDriver}, }, { - name: "migration off for GCE", - volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin}, - expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin}, - claimAnnotations: map[string]string{volume.AnnStorageProvisioner: gcePlugin}, - expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: gcePlugin}, - migratedDriverGates: []featuregate.Feature{}, - disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + name: "migration off", + volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: nonmigratedPlugin}, + expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: nonmigratedPlugin}, + claimAnnotations: map[string]string{volume.AnnStorageProvisioner: nonmigratedPlugin}, + expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: nonmigratedPlugin}, }, { - name: "migration off for GCE removes migrated to (rollback)", - volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver}, - expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin}, - claimAnnotations: map[string]string{volume.AnnStorageProvisioner: gcePlugin, volume.AnnMigratedTo: gceDriver}, - expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: gcePlugin}, - migratedDriverGates: []featuregate.Feature{}, - disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + name: "migration off removes migrated to (rollback)", + volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: nonmigratedPlugin, volume.AnnMigratedTo: nonmigratedDriver}, + expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: nonmigratedPlugin}, + claimAnnotations: map[string]string{volume.AnnStorageProvisioner: nonmigratedPlugin, volume.AnnMigratedTo: nonmigratedDriver}, + expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: nonmigratedPlugin}, }, { - name: "migration off for GCE removes migrated to (rollback) with Beta storage provisioner annontation", - volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver}, - expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin}, - claimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: gcePlugin, volume.AnnMigratedTo: gceDriver}, - expClaimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: gcePlugin}, - migratedDriverGates: []featuregate.Feature{}, - disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + name: "migration off removes migrated to (rollback) with Beta storage provisioner annontation", + volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: nonmigratedPlugin, volume.AnnMigratedTo: nonmigratedDriver}, + expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: nonmigratedPlugin}, + claimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: nonmigratedPlugin, volume.AnnMigratedTo: nonmigratedDriver}, + expClaimAnnotations: map[string]string{volume.AnnBetaStorageProvisioner: nonmigratedPlugin}, }, { - name: "migration on for GCE other plugin not affected", + name: "migration on, other plugin not affected", volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: testPlugin}, expVolumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: testPlugin}, claimAnnotations: map[string]string{volume.AnnStorageProvisioner: testPlugin}, expClaimAnnotations: map[string]string{volume.AnnStorageProvisioner: testPlugin}, - migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, - disabledDriverGates: []featuregate.Feature{}, }, { - name: "not dynamically provisioned migration off for GCE", + name: "not dynamically provisioned", volumeAnnotations: map[string]string{}, expVolumeAnnotations: map[string]string{}, claimAnnotations: map[string]string{}, expClaimAnnotations: map[string]string{}, - migratedDriverGates: []featuregate.Feature{}, - disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + testMigration: false, }, { - name: "not dynamically provisioned migration on for GCE", - volumeAnnotations: map[string]string{}, - expVolumeAnnotations: map[string]string{}, - claimAnnotations: map[string]string{}, - expClaimAnnotations: map[string]string{}, - migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, - disabledDriverGates: []featuregate.Feature{}, - }, - { - name: "nil annotations migration off for GCE", + name: "nil annotations", volumeAnnotations: nil, expVolumeAnnotations: nil, claimAnnotations: nil, expClaimAnnotations: nil, - migratedDriverGates: []featuregate.Feature{}, - disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, - }, - { - name: "nil annotations migration on for GCE", - volumeAnnotations: nil, - expVolumeAnnotations: nil, - claimAnnotations: nil, - expClaimAnnotations: nil, - migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, - disabledDriverGates: []featuregate.Feature{}, + testMigration: false, }, } @@ -580,12 +554,6 @@ func TestAnnealMigrationAnnotations(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - for _, f := range tc.migratedDriverGates { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, true)() - } - for _, f := range tc.disabledDriverGates { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, false)() - } if tc.volumeAnnotations != nil { ann := tc.volumeAnnotations updateMigrationAnnotations(cmpm, translator, ann, false) @@ -607,11 +575,14 @@ func TestAnnealMigrationAnnotations(t *testing.T) { func TestModifyDeletionFinalizers(t *testing.T) { // This set of tests ensures that protection finalizer is removed when CSI migration is disabled - // and PV controller needs to remove finalizers added by the external-provisioner. - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + // and PV controller needs to remove finalizers added by the external-provisioner. The rbd + // in-tree plugin is used as migration is disabled. When that plugin is migrated, a different + // non-migrated one should be used. If all plugins are migrated this test can be removed. The + // gce in-tree plugin is used for a migrated driver as it is feature-locked as of 1.25. defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HonorPVReclaimPolicy, true)() - const gcePlugin = "kubernetes.io/gce-pd" - const gceDriver = "pd.csi.storage.gke.io" + const nonmigratedDriver = "rbd.csi.ceph.com" + const migratedPlugin = "kubernetes.io/gce-pd" + const migratedDriver = "pd.csi.storage.gke.io" const customFinalizer = "test.volume.kubernetes.io/finalizer" tests := []struct { name string @@ -619,24 +590,21 @@ func TestModifyDeletionFinalizers(t *testing.T) { volumeAnnotations map[string]string expVolumeFinalizers []string expModified bool - migratedDriverGates []featuregate.Feature }{ { // Represents a CSI volume provisioned through external-provisioner, no CSI migration enabled. name: "13-1 migration was never enabled, volume has the finalizer", - initialVolume: newExternalProvisionedVolume("volume-13-1", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, gceDriver, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), + initialVolume: newExternalProvisionedVolume("volume-13-1", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, nonmigratedDriver, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer}, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, { // Represents a volume provisioned through external-provisioner but the external-provisioner has // yet to sync the volume to add the new finalizer name: "13-2 migration was never enabled, volume does not have the finalizer", - initialVolume: newExternalProvisionedVolume("volume-13-2", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, gceDriver, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), + initialVolume: newExternalProvisionedVolume("volume-13-2", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, nonmigratedDriver, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: nil, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, { // Represents an in-tree volume that has the migrated-to annotation but the external-provisioner is @@ -648,14 +616,12 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-3", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{customFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{customFinalizer, volume.PVDeletionInTreeProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { name: "13-4 migration was disabled, volume has no finalizers", initialVolume: newVolumeWithFinalizers("volume-13-4", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { // Represents roll back scenario where the external-provisioner has added the pv deletion protection @@ -665,7 +631,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-5", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { // Represents roll-back of csi-migration as 13-5, here there are multiple finalizers, only the pv deletion @@ -675,17 +640,15 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-6", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer, customFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{customFinalizer, volume.PVDeletionInTreeProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { // csi migration is enabled, the pv controller should not delete the finalizer added by the // external-provisioner and the in-tree finalizer should be deleted. name: "13-7 migration is enabled, volume has both the in-tree and external PV deletion protection finalizer", initialVolume: newVolumeWithFinalizers("volume-13-7", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer, volume.PVDeletionInTreeProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), - volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver}, + volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: migratedPlugin, volume.AnnMigratedTo: migratedDriver}, expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { // csi-migration is not completely enabled as the specific plugin feature is not present. This is equivalent @@ -694,7 +657,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-8", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { // same as 13-8 but multiple finalizers exists, only the pv deletion protection finalizer needs to be @@ -703,7 +665,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-9", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer, customFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: []string{customFinalizer, volume.PVDeletionInTreeProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { // corner error case. @@ -711,14 +672,12 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-10", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer}), expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer}, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, { name: "13-11 missing annotations and finalizers", initialVolume: newVolumeWithFinalizers("volume-13-11", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, nil), expVolumeFinalizers: nil, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, { // When ReclaimPolicy is Retain ensure that in-tree pv deletion protection finalizer is not added. @@ -726,7 +685,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-12", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classCopper, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: nil, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, { // When ReclaimPolicy is Recycle ensure that in-tree pv deletion protection finalizer is not added. @@ -734,7 +692,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-13", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimRecycle, classCopper, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: nil, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, { // When ReclaimPolicy is Retain ensure that in-tree pv deletion protection finalizer present is removed. @@ -742,7 +699,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-14", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classCopper, []string{volume.PVDeletionInTreeProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController), expVolumeFinalizers: nil, expModified: true, - migratedDriverGates: []featuregate.Feature{}, }, { // Statically provisioned volumes should not have the in-tree pv deletion protection finalizer @@ -750,7 +706,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { initialVolume: newVolumeWithFinalizers("volume-13-14", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper, nil), expVolumeFinalizers: nil, expModified: false, - migratedDriverGates: []featuregate.Feature{}, }, } @@ -759,9 +714,6 @@ func TestModifyDeletionFinalizers(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - for _, f := range tc.migratedDriverGates { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, true)() - } if tc.volumeAnnotations != nil { tc.initialVolume.SetAnnotations(tc.volumeAnnotations) } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 22708622451..b70a830bf52 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -134,6 +134,7 @@ const ( // owner: @davidz627 // alpha: v1.14 // beta: v1.17 + // GA: 1.25 // // Enables the GCE PD in-tree driver to GCE CSI Driver migration feature. CSIMigrationGCE featuregate.Feature = "CSIMigrationGCE" @@ -880,7 +881,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CSIMigrationAzureFile: {Default: true, PreRelease: featuregate.Beta}, // On by default in 1.24 (requires Azure File CSI driver) - CSIMigrationGCE: {Default: true, PreRelease: featuregate.Beta}, // On by default in 1.23 (requires GCE PD CSI Driver) + CSIMigrationGCE: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.25 (requires GCE PD CSI Driver) CSIMigrationOpenStack: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26 diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index 70290abb897..f36d9aaf3ca 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -25,10 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" core "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util" @@ -103,8 +100,6 @@ func TestPodVolumesExist(t *testing.T) { t.Skip("skipping test in short mode.") } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -131,8 +126,8 @@ func TestPodVolumesExist(t *testing.T) { { Name: "vol1", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device1", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake1", }, }, }, @@ -160,8 +155,8 @@ func TestPodVolumesExist(t *testing.T) { { Name: "vol2", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device2", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake2", }, }, }, @@ -189,8 +184,8 @@ func TestPodVolumesExist(t *testing.T) { { Name: "vol3", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device3", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake3", }, }, }, @@ -219,8 +214,6 @@ func TestVolumeAttachAndMountControllerDisabled(t *testing.T) { t.Skip("skipping test in short mode.") } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -241,8 +234,8 @@ func TestVolumeAttachAndMountControllerDisabled(t *testing.T) { { Name: "vol1", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake", }, }, }, @@ -280,8 +273,6 @@ func TestVolumeUnmountAndDetachControllerDisabled(t *testing.T) { t.Skip("skipping test in short mode.") } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -302,8 +293,8 @@ func TestVolumeUnmountAndDetachControllerDisabled(t *testing.T) { { Name: "vol1", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake-device", }, }, }, @@ -371,8 +362,6 @@ func TestVolumeAttachAndMountControllerEnabled(t *testing.T) { t.Skip("skipping test in short mode.") } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - testKubelet := newTestKubelet(t, true /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -410,8 +399,8 @@ func TestVolumeAttachAndMountControllerEnabled(t *testing.T) { { Name: "vol1", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake-device", }, }, }, @@ -457,8 +446,6 @@ func TestVolumeUnmountAndDetachControllerEnabled(t *testing.T) { t.Skip("skipping test in short mode.") } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - testKubelet := newTestKubelet(t, true /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -496,8 +483,8 @@ func TestVolumeUnmountAndDetachControllerEnabled(t *testing.T) { { Name: "vol1", VolumeSource: v1.VolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device", + RBD: &v1.RBDVolumeSource{ + RBDImage: "fake-device", }, }, }, diff --git a/pkg/kubelet/volumemanager/volume_manager_test.go b/pkg/kubelet/volumemanager/volume_manager_test.go index 8d23092b535..46e4a75b51a 100644 --- a/pkg/kubelet/volumemanager/volume_manager_test.go +++ b/pkg/kubelet/volumemanager/volume_manager_test.go @@ -31,13 +31,10 @@ import ( kubetypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" utiltesting "k8s.io/client-go/util/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/config" "k8s.io/kubernetes/pkg/kubelet/configmap" containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" @@ -56,8 +53,6 @@ const ( ) func TestGetMountedVolumesForPodAndGetVolumesInUse(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - tests := []struct { name string pvMode, podMode v1.PersistentVolumeMode @@ -145,8 +140,6 @@ func TestGetMountedVolumesForPodAndGetVolumesInUse(t *testing.T) { } func TestInitialPendingVolumesForPodAndGetVolumesInUse(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - tmpDir, err := utiltesting.MkTmpdir("volumeManagerTest") if err != nil { t.Fatalf("can't make a temp dir: %v", err) @@ -192,8 +185,6 @@ func TestInitialPendingVolumesForPodAndGetVolumesInUse(t *testing.T) { } func TestGetExtraSupplementalGroupsForPod(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() - tmpDir, err := utiltesting.MkTmpdir("volumeManagerTest") if err != nil { t.Fatalf("can't make a temp dir: %v", err) @@ -238,8 +229,8 @@ func TestGetExtraSupplementalGroupsForPod(t *testing.T) { }, Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device", + RBD: &v1.RBDPersistentVolumeSource{ + RBDImage: "fake-device", }, }, ClaimRef: &v1.ObjectReference{ @@ -385,8 +376,8 @@ func createObjects(pvMode, podMode v1.PersistentVolumeMode) (*v1.Node, *v1.Pod, }, Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "fake-device", + RBD: &v1.RBDPersistentVolumeSource{ + RBDImage: "fake-device", }, }, ClaimRef: &v1.ObjectReference{ diff --git a/pkg/scheduler/framework/plugins/volumebinding/binder_test.go b/pkg/scheduler/framework/plugins/volumebinding/binder_test.go index 94e93dd6291..8b71a5d20ba 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/binder_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/binder_test.go @@ -33,19 +33,16 @@ import ( "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" coreinformers "k8s.io/client-go/informers/core/v1" storageinformers "k8s.io/client-go/informers/storage/v1" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-helpers/storage/volume" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/controller" pvtesting "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/testing" - "k8s.io/kubernetes/pkg/features" ) var ( @@ -94,8 +91,9 @@ var ( pvBoundImmediateNode2 = makeTestPV("pv-bound-immediate", "node2", "1G", "1", immediateBoundPVC, immediateClass) // PVs for CSI migration - migrationPVBound = makeTestPVForCSIMigration(zone1Labels, boundMigrationPVC) - migrationPVBoundToUnbound = makeTestPVForCSIMigration(zone1Labels, unboundPVC) + migrationPVBound = makeTestPVForCSIMigration(zone1Labels, boundMigrationPVC, true) + migrationPVBoundToUnbound = makeTestPVForCSIMigration(zone1Labels, unboundPVC, true) + nonmigrationPVBoundToUnbound = makeTestPVForCSIMigration(zone1Labels, unboundPVC, false) // storage class names waitClass = "waitClass" @@ -693,17 +691,29 @@ func makeTestPV(name, node, capacity, version string, boundToPVC *v1.PersistentV return pv } -func makeTestPVForCSIMigration(labels map[string]string, pvc *v1.PersistentVolumeClaim) *v1.PersistentVolume { +func makeTestPVForCSIMigration(labels map[string]string, pvc *v1.PersistentVolumeClaim, migrationEnabled bool) *v1.PersistentVolume { pv := makeTestPV("pv-migration-bound", "node1", "1G", "1", pvc, waitClass) pv.Spec.NodeAffinity = nil // Will be written by the CSI translation lib pv.ObjectMeta.Labels = labels - pv.Spec.PersistentVolumeSource = v1.PersistentVolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "test-disk", - FSType: "ext4", - Partition: 0, - ReadOnly: false, - }, + // GCEPersistentDisk is used when migration is enabled, as its featuregate is locked to GA. + // RBD is used for the nonmigrated case, as its featuregate is still alpha. When RBD migration goes GA, + // a different nonmigrated plugin should be used instead. If there are no other plugins, then the + // nonmigrated test case is no longer relevant and can be removed. + if migrationEnabled { + pv.Spec.PersistentVolumeSource = v1.PersistentVolumeSource{ + GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ + PDName: "test-disk", + FSType: "ext4", + Partition: 0, + ReadOnly: false, + }, + } + } else { + pv.Spec.PersistentVolumeSource = v1.PersistentVolumeSource{ + RBD: &v1.RBDPersistentVolumeSource{ + RBDImage: "test-disk", + }, + } } return pv } @@ -1217,8 +1227,6 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, true)() - // Setup testEnv := newTestBinder(t, ctx.Done()) testEnv.initVolumes(scenario.pvs, scenario.pvs) @@ -1775,9 +1783,8 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { apiPVCs []*v1.PersistentVolumeClaim // Expected return values - shouldFail bool - expectedBound bool - migrationEnabled bool + shouldFail bool + expectedBound bool } scenarios := map[string]scenarioType{ "provisioning-pvc-bound": { @@ -1791,50 +1798,45 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { expectedBound: true, }, "binding-node-pv-same-zone": { - bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, - provisionedPVCs: []*v1.PersistentVolumeClaim{}, - initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, - initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - initNodes: []*v1.Node{node1Zone1}, - initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, - migrationEnabled: true, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + provisionedPVCs: []*v1.PersistentVolumeClaim{}, + initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, + initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, + initNodes: []*v1.Node{node1Zone1}, + initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, }, "binding-without-csinode": { - bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, - provisionedPVCs: []*v1.PersistentVolumeClaim{}, - initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, - initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - initNodes: []*v1.Node{node1Zone1}, - initCSINodes: []*storagev1.CSINode{}, - migrationEnabled: true, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + provisionedPVCs: []*v1.PersistentVolumeClaim{}, + initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, + initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, + initNodes: []*v1.Node{node1Zone1}, + initCSINodes: []*storagev1.CSINode{}, }, "binding-non-migrated-plugin": { - bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, - provisionedPVCs: []*v1.PersistentVolumeClaim{}, - initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, - initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - initNodes: []*v1.Node{node1Zone1}, - initCSINodes: []*storagev1.CSINode{csiNode1NotMigrated}, - migrationEnabled: true, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + provisionedPVCs: []*v1.PersistentVolumeClaim{}, + initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, + initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, + initNodes: []*v1.Node{node1Zone1}, + initCSINodes: []*storagev1.CSINode{csiNode1NotMigrated}, }, "binding-node-pv-in-different-zones": { - bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, - provisionedPVCs: []*v1.PersistentVolumeClaim{}, - initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, - initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - initNodes: []*v1.Node{node1Zone2}, - initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, - migrationEnabled: true, - shouldFail: true, + bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, + provisionedPVCs: []*v1.PersistentVolumeClaim{}, + initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, + initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, + initNodes: []*v1.Node{node1Zone2}, + initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, + shouldFail: true, }, "binding-node-pv-different-zones-migration-off": { - bindings: []*BindingInfo{makeBinding(unboundPVC, migrationPVBoundToUnbound)}, - provisionedPVCs: []*v1.PersistentVolumeClaim{}, - initPVs: []*v1.PersistentVolume{migrationPVBoundToUnbound}, - initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, - initNodes: []*v1.Node{node1Zone2}, - initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, - migrationEnabled: false, + bindings: []*BindingInfo{makeBinding(unboundPVC, nonmigrationPVBoundToUnbound)}, + provisionedPVCs: []*v1.PersistentVolumeClaim{}, + initPVs: []*v1.PersistentVolume{nonmigrationPVBoundToUnbound}, + initPVCs: []*v1.PersistentVolumeClaim{unboundPVC}, + initNodes: []*v1.Node{node1Zone2}, + initCSINodes: []*storagev1.CSINode{csiNode1Migrated}, }, } @@ -1842,8 +1844,6 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, scenario.migrationEnabled)() - // Setup pod := makePod("test-pod"). withNamespace("testns"). diff --git a/pkg/volume/csimigration/plugin_manager_test.go b/pkg/volume/csimigration/plugin_manager_test.go index 9467a497b2c..ae23bd2c9fd 100644 --- a/pkg/volume/csimigration/plugin_manager_test.go +++ b/pkg/volume/csimigration/plugin_manager_test.go @@ -39,8 +39,8 @@ func TestIsMigratable(t *testing.T) { spec *volume.Spec }{ { - name: "GCE PD PV source with CSIMigrationGCE enabled", - pluginFeature: features.CSIMigrationGCE, + name: "RBD PV source with CSIMigrationGCE enabled", + pluginFeature: features.CSIMigrationRBD, pluginFeatureEnabled: true, isMigratable: true, csiMigrationEnabled: true, @@ -48,11 +48,8 @@ func TestIsMigratable(t *testing.T) { PersistentVolume: &v1.PersistentVolume{ Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "test-disk", - FSType: "ext4", - Partition: 0, - ReadOnly: false, + RBD: &v1.RBDPersistentVolumeSource{ + RBDImage: "test-disk", }, }, }, @@ -60,8 +57,8 @@ func TestIsMigratable(t *testing.T) { }, }, { - name: "GCE PD PV Source with CSIMigrationGCE disabled", - pluginFeature: features.CSIMigrationGCE, + name: "RBD PD PV Source with CSIMigrationGCE disabled", + pluginFeature: features.CSIMigrationRBD, pluginFeatureEnabled: false, isMigratable: false, csiMigrationEnabled: true, @@ -69,11 +66,8 @@ func TestIsMigratable(t *testing.T) { PersistentVolume: &v1.PersistentVolume{ Spec: v1.PersistentVolumeSpec{ PersistentVolumeSource: v1.PersistentVolumeSource{ - GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{ - PDName: "test-disk", - FSType: "ext4", - Partition: 0, - ReadOnly: false, + RBD: &v1.RBDPersistentVolumeSource{ + RBDImage: "test-disk", }, }, }, @@ -186,28 +180,6 @@ func TestMigrationFeatureFlagStatus(t *testing.T) { csiMigrationResult bool csiMigrationCompleteResult bool }{ - { - name: "gce-pd migration flag disabled and migration-complete flag disabled with CSI migration flag disabled", - pluginName: "kubernetes.io/gce-pd", - pluginFeature: features.CSIMigrationGCE, - pluginFeatureEnabled: false, - csiMigrationEnabled: false, - inTreePluginUnregister: features.InTreePluginGCEUnregister, - inTreePluginUnregisterEnabled: false, - csiMigrationResult: false, - csiMigrationCompleteResult: false, - }, - { - name: "gce-pd migration flag disabled and migration-complete flag disabled with CSI migration flag enabled", - pluginName: "kubernetes.io/gce-pd", - pluginFeature: features.CSIMigrationGCE, - pluginFeatureEnabled: false, - csiMigrationEnabled: true, - inTreePluginUnregister: features.InTreePluginGCEUnregister, - inTreePluginUnregisterEnabled: false, - csiMigrationResult: false, - csiMigrationCompleteResult: false, - }, { name: "gce-pd migration flag enabled and migration-complete flag disabled with CSI migration flag enabled", pluginName: "kubernetes.io/gce-pd", @@ -257,7 +229,12 @@ func TestMigrationFeatureFlagStatus(t *testing.T) { for _, test := range testCases { pm := NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate) t.Run(fmt.Sprintf("Testing %v", test.name), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() + // CSIMigrationGCE is locked to on, so it cannot be enabled or disabled. There are a couple + // of test cases that check correct behavior when CSIMigrationGCE is enabled, but there are + // no longer any tests cases for CSIMigrationGCE being disabled as that is not possible. + if test.pluginFeature != features.CSIMigrationGCE { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() + } defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.inTreePluginUnregister, test.inTreePluginUnregisterEnabled)() csiMigrationResult := pm.IsMigrationEnabledForPlugin(test.pluginName) diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 12882a16f0d..a747d755725 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -250,9 +250,14 @@ func (plugin *FakeVolumePlugin) GetVolumeName(spec *volume.Spec) (string, error) var volumeName string if spec.Volume != nil && spec.Volume.GCEPersistentDisk != nil { volumeName = spec.Volume.GCEPersistentDisk.PDName + } else if spec.Volume != nil && spec.Volume.RBD != nil { + volumeName = spec.Volume.RBD.RBDImage } else if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.GCEPersistentDisk != nil { volumeName = spec.PersistentVolume.Spec.GCEPersistentDisk.PDName + } else if spec.PersistentVolume != nil && + spec.PersistentVolume.Spec.RBD != nil { + volumeName = spec.PersistentVolume.Spec.RBD.RBDImage } else if spec.Volume != nil && spec.Volume.CSI != nil { volumeName = spec.Volume.CSI.Driver } @@ -671,9 +676,14 @@ func getUniqueVolumeName(spec *volume.Spec) (string, error) { var volumeName string if spec.Volume != nil && spec.Volume.GCEPersistentDisk != nil { volumeName = spec.Volume.GCEPersistentDisk.PDName + } else if spec.Volume != nil && spec.Volume.RBD != nil { + volumeName = spec.Volume.RBD.RBDImage } else if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.GCEPersistentDisk != nil { volumeName = spec.PersistentVolume.Spec.GCEPersistentDisk.PDName + } else if spec.PersistentVolume != nil && + spec.PersistentVolume.Spec.RBD != nil { + volumeName = spec.PersistentVolume.Spec.RBD.RBDImage } if volumeName == "" { volumeName = spec.Name()