From 275fdf088486746a30875369acb016676876662f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9iy=C3=AC=20Zhang?= Date: Sat, 18 Sep 2021 00:05:11 +0000 Subject: [PATCH] fixing unit test failures induced by turning on CSIMigrationGCE disable CSIMigrationGCE in some unit tests --- .../attachdetach/attach_detach_controller_test.go | 9 +++++---- .../volume/attachdetach/metrics/metrics_test.go | 5 ++++- .../desired_state_of_world_populator_test.go | 4 ++++ .../volume/persistentvolume/pv_controller_test.go | 14 ++++++++++++++ pkg/kubelet/kubelet_volumes_test.go | 13 +++++++++++++ pkg/kubelet/volumemanager/volume_manager_test.go | 9 +++++++++ 6 files changed, 49 insertions(+), 5 deletions(-) diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go index bcadae559dc..605ee535a72 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go @@ -430,11 +430,12 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) { fakeKubeClient := controllervolumetesting.CreateTestClient() informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, time.Second*1) var plugins []volume.VolumePlugin - if tc.csiMigration { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, tc.csiMigration)() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, tc.csiMigration)() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InTreePluginGCEUnregister, tc.csiMigration)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, tc.csiMigration)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, tc.csiMigration)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InTreePluginGCEUnregister, tc.csiMigration)() + + 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 { diff --git a/pkg/controller/volume/attachdetach/metrics/metrics_test.go b/pkg/controller/volume/attachdetach/metrics/metrics_test.go index 66723073112..ff038e62a1b 100644 --- a/pkg/controller/volume/attachdetach/metrics/metrics_test.go +++ b/pkg/controller/volume/attachdetach/metrics/metrics_test.go @@ -19,23 +19,26 @@ package metrics import ( "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" 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{} 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 a2baecb9801..ab772d6c6b8 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,15 +26,19 @@ 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{} diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 3da736cf6b0..1c694d9642b 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -567,6 +567,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations map[string]string expClaimAnnotations map[string]string migratedDriverGates []featuregate.Feature + disabledDriverGates []featuregate.Feature }{ { name: "migration on for GCE", @@ -575,6 +576,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: gcePlugin}, expClaimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: gcePlugin, pvutil.AnnMigratedTo: gceDriver}, migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + disabledDriverGates: []featuregate.Feature{}, }, { name: "migration on for GCE with Beta storage provisioner annontation", @@ -583,6 +585,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{pvutil.AnnBetaStorageProvisioner: gcePlugin}, expClaimAnnotations: map[string]string{pvutil.AnnBetaStorageProvisioner: gcePlugin, pvutil.AnnMigratedTo: gceDriver}, migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + disabledDriverGates: []featuregate.Feature{}, }, { name: "migration off for GCE", @@ -591,6 +594,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: gcePlugin}, expClaimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: gcePlugin}, migratedDriverGates: []featuregate.Feature{}, + disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { name: "migration off for GCE removes migrated to (rollback)", @@ -599,6 +603,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: gcePlugin, pvutil.AnnMigratedTo: gceDriver}, expClaimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: gcePlugin}, migratedDriverGates: []featuregate.Feature{}, + disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { name: "migration off for GCE removes migrated to (rollback) with Beta storage provisioner annontation", @@ -607,6 +612,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{pvutil.AnnBetaStorageProvisioner: gcePlugin, pvutil.AnnMigratedTo: gceDriver}, expClaimAnnotations: map[string]string{pvutil.AnnBetaStorageProvisioner: gcePlugin}, migratedDriverGates: []featuregate.Feature{}, + disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { name: "migration on for GCE other plugin not affected", @@ -615,6 +621,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: testPlugin}, expClaimAnnotations: map[string]string{pvutil.AnnStorageProvisioner: testPlugin}, migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + disabledDriverGates: []featuregate.Feature{}, }, { name: "not dynamically provisioned migration off for GCE", @@ -623,6 +630,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{}, expClaimAnnotations: map[string]string{}, migratedDriverGates: []featuregate.Feature{}, + disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { name: "not dynamically provisioned migration on for GCE", @@ -631,6 +639,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: map[string]string{}, expClaimAnnotations: map[string]string{}, migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + disabledDriverGates: []featuregate.Feature{}, }, { name: "nil annotations migration off for GCE", @@ -639,6 +648,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: nil, expClaimAnnotations: nil, migratedDriverGates: []featuregate.Feature{}, + disabledDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { name: "nil annotations migration on for GCE", @@ -647,6 +657,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) { claimAnnotations: nil, expClaimAnnotations: nil, migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, + disabledDriverGates: []featuregate.Feature{}, }, } @@ -658,6 +669,9 @@ func TestAnnealMigrationAnnotations(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 updateMigrationAnnotationsAndFinalizers(cmpm, translator, ann, nil, false) diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index 385c4ee6e52..24b3a71ca12 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -25,7 +25,10 @@ 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" @@ -92,6 +95,8 @@ func TestListVolumesForPod(t *testing.T) { } func TestPodVolumesExist(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -202,6 +207,8 @@ func TestPodVolumesExist(t *testing.T) { } func TestVolumeAttachAndMountControllerDisabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -257,6 +264,8 @@ func TestVolumeAttachAndMountControllerDisabled(t *testing.T) { } func TestVolumeUnmountAndDetachControllerDisabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -342,6 +351,8 @@ func TestVolumeUnmountAndDetachControllerDisabled(t *testing.T) { } func TestVolumeAttachAndMountControllerEnabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + testKubelet := newTestKubelet(t, true /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -422,6 +433,8 @@ func TestVolumeAttachAndMountControllerEnabled(t *testing.T) { } func TestVolumeUnmountAndDetachControllerEnabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + testKubelet := newTestKubelet(t, true /* controllerAttachDetachEnabled */) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet diff --git a/pkg/kubelet/volumemanager/volume_manager_test.go b/pkg/kubelet/volumemanager/volume_manager_test.go index cde1fe848f9..9ea2531924d 100644 --- a/pkg/kubelet/volumemanager/volume_manager_test.go +++ b/pkg/kubelet/volumemanager/volume_manager_test.go @@ -31,10 +31,13 @@ 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" @@ -53,6 +56,8 @@ const ( ) func TestGetMountedVolumesForPodAndGetVolumesInUse(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)() + tests := []struct { name string pvMode, podMode v1.PersistentVolumeMode @@ -140,6 +145,8 @@ 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) @@ -185,6 +192,8 @@ 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)