Merge pull request #124108 from carlory/update-test-InTreePluginXXXUnregister

update unit test for adc to test volume migration

pkg/controller/volume/attachdetach/attach_detach_controller_test.go

@@ -87,8 +87,12 @@ func Test_AttachDetachControllerStateOfWorldPopulators_Positive(t *testing.T) {
 	fakeKubeClient := controllervolumetesting.CreateTestClient()
 	informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, controller.NoResyncPeriodFunc())
 
+	var plugins []volume.VolumePlugin
+	plugins = append(plugins, controllervolumetesting.CreateTestPlugin(false)...)
+	plugins = append(plugins, csi.ProbeVolumePlugins()...)
+
 	logger, tCtx := ktesting.NewTestContext(t)
-	adc := createADC(t, tCtx, fakeKubeClient, informerFactory, controllervolumetesting.CreateTestPlugin())
+	adc := createADC(t, tCtx, fakeKubeClient, informerFactory, plugins)
 
 	// Act
 	informerFactory.Start(tCtx.Done())
@@ -136,9 +140,11 @@ func Test_AttachDetachControllerStateOfWorldPopulators_Positive(t *testing.T) {
 		t.Fatalf("Run failed with error. Expected: <no error> Actual: %v", err)
 	}
 	for _, pod := range pods {
-		uniqueName := fmt.Sprintf("%s/%s", controllervolumetesting.TestPluginName, pod.Spec.Volumes[0].Name)
+		// All pods in fakeKubeClient have only one volume and share the same volume.
+		// pdName is the part of the translated volume name after csi migration.
+		volumeName := v1.UniqueVolumeName(csiPDUniqueNamePrefix + "pdName")
 		nodeName := types.NodeName(pod.Spec.NodeName)
-		found := adc.desiredStateOfWorld.VolumeExists(v1.UniqueVolumeName(uniqueName), nodeName)
+		found := adc.desiredStateOfWorld.VolumeExists(volumeName, nodeName)
 		if !found {
 			t.Fatalf("Run failed with error. Volume %s, node %s not found in DesiredStateOfWorld",
 				pod.Spec.Volumes[0].Name,
@@ -238,21 +244,43 @@ func BenchmarkNodeUpdate(b *testing.B) {
 }
 
 func Test_AttachDetachControllerRecovery(t *testing.T) {
-	attachDetachRecoveryTestCase(t, []*v1.Pod{}, []*v1.Pod{})
+	testCases := []struct {
-	newPod1 := controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1")
+		testName   string
-	attachDetachRecoveryTestCase(t, []*v1.Pod{newPod1}, []*v1.Pod{})
+		extraPods1 []*v1.Pod
-	newPod1 = controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1")
+		extraPods2 []*v1.Pod
-	attachDetachRecoveryTestCase(t, []*v1.Pod{}, []*v1.Pod{newPod1})
+	}{
-	newPod1 = controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1")
+		{
-	newPod2 := controllervolumetesting.NewPodWithVolume("newpod-2", "volumeName3", "mynode-1")
+			testName: "No extra pods",
-	attachDetachRecoveryTestCase(t, []*v1.Pod{newPod1}, []*v1.Pod{newPod2})
+		},
+		{
+			testName:   "Add a new pod between ASW and DSW ppoulators",
+			extraPods1: []*v1.Pod{controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1")},
+		},
+		{
+			testName:   "Add a new pod between DSW ppoulator and reconciler run",
+			extraPods2: []*v1.Pod{controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1")},
+		},
+		{
+			testName: "Add a new pod between ASW and DSW ppoulators and another between DSW ppoulator and reconciler run",
+			// All the pods share the same underlying volume, including pre-existing pods. It means that the volume
+			// will be translated to the same persistent volume name. But each "extra pods" is running on a different node.
+			// So the expected attached volumes should be 1+extraPodsNum.
+			extraPods1: []*v1.Pod{controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1")},
+			extraPods2: []*v1.Pod{controllervolumetesting.NewPodWithVolume("newpod-2", "volumeName3", "mynode-2")},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.testName, func(t *testing.T) {
+			t.Parallel()
+			attachDetachRecoveryTestCase(t, tc.extraPods1, tc.extraPods2)
+		})
+	}
 }
 
 func attachDetachRecoveryTestCase(t *testing.T, extraPods1 []*v1.Pod, extraPods2 []*v1.Pod) {
 	fakeKubeClient := controllervolumetesting.CreateTestClient()
 	informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, time.Second*1)
-	//informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, time.Second*1)
+	plugins := controllervolumetesting.CreateTestPlugin(true)
-	plugins := controllervolumetesting.CreateTestPlugin()
 	var prober volume.DynamicPluginProber = nil // TODO (#51147) inject mock
 	nodeInformer := informerFactory.Core().V1().Nodes().Informer()
 	csiNodeInformer := informerFactory.Storage().V1().CSINodes().Informer()
@@ -449,18 +477,6 @@ type vaTest struct {
 
 func Test_ADC_VolumeAttachmentRecovery(t *testing.T) {
 	for _, tc := range []vaTest{
-		{ // pod is scheduled
-			testName:          "Scheduled pod",
-			volName:           "vol1",
-			podName:           "pod1",
-			podNodeName:       "mynode-1",
-			pvName:            "pv1",
-			vaName:            "va1",
-			vaNodeName:        "mynode-1",
-			vaAttachStatus:    false,
-			expected_attaches: map[string][]string{"mynode-1": {"vol1"}},
-			expected_detaches: map[string][]string{},
-		},
 		{ // pod is deleted, attach status:true, verify dangling volume is detached
 			testName:          "VA status is attached",
 			volName:           "vol1",
@@ -514,7 +530,7 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) {
 	informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, time.Second*1)
 	var plugins []volume.VolumePlugin
 
-	plugins = append(plugins, controllervolumetesting.CreateTestPlugin()...)
+	plugins = append(plugins, controllervolumetesting.CreateTestPlugin(false)...)
 	plugins = append(plugins, csi.ProbeVolumePlugins()...)
 
 	nodeInformer := informerFactory.Core().V1().Nodes().Informer()

pkg/controller/volume/attachdetach/testing/plugin.go

@@ -0,0 +1,281 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package testing
+
+import (
+	"fmt"
+	"sync"
+	"time"
+
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/volume"
+)
+
+const TestPluginName = "kubernetes.io/testPlugin"
+
+type TestPlugin struct {
+	// SupportCSIVolume allows the plugin to support CSI volumes.
+	// It does not mock the actual CSI volume operations.
+	SupportCSIVolume  bool
+	ErrorEncountered  bool
+	attachedVolumeMap map[string][]string
+	detachedVolumeMap map[string][]string
+	pluginLock        *sync.RWMutex
+}
+
+func (plugin *TestPlugin) Init(host volume.VolumeHost) error {
+	return nil
+}
+
+func (plugin *TestPlugin) GetPluginName() string {
+	return TestPluginName
+}
+
+func (plugin *TestPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
+	plugin.pluginLock.Lock()
+	defer plugin.pluginLock.Unlock()
+	if spec == nil {
+		plugin.ErrorEncountered = true
+		return "", fmt.Errorf("GetVolumeName called with nil volume spec")
+	}
+	if spec.Volume != nil {
+		return spec.Name(), nil
+	} else if spec.PersistentVolume != 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
+		} else if spec.PersistentVolume.Spec.PersistentVolumeSource.CSI != nil {
+			csi := spec.PersistentVolume.Spec.PersistentVolumeSource.CSI
+			return fmt.Sprintf("%s^%s", csi.Driver, csi.VolumeHandle), nil
+		}
+		return "", fmt.Errorf("GetVolumeName called with unexpected PersistentVolume: %v", spec)
+	} else {
+		return "", nil
+	}
+}
+
+func (plugin *TestPlugin) CanSupport(spec *volume.Spec) bool {
+	plugin.pluginLock.Lock()
+	defer plugin.pluginLock.Unlock()
+	if spec == nil {
+		plugin.ErrorEncountered = true
+	} else {
+		if spec.Volume != nil && spec.Volume.CSI != nil {
+			return plugin.SupportCSIVolume
+		}
+		if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.CSI != nil {
+			return plugin.SupportCSIVolume
+		}
+	}
+	return true
+}
+
+func (plugin *TestPlugin) RequiresRemount(spec *volume.Spec) bool {
+	return false
+}
+
+func (plugin *TestPlugin) NewMounter(spec *volume.Spec, podRef *v1.Pod, opts volume.VolumeOptions) (volume.Mounter, error) {
+	plugin.pluginLock.Lock()
+	defer plugin.pluginLock.Unlock()
+	if spec == nil {
+		plugin.ErrorEncountered = true
+	}
+	return nil, nil
+}
+
+func (plugin *TestPlugin) NewUnmounter(name string, podUID types.UID) (volume.Unmounter, error) {
+	return nil, nil
+}
+
+func (plugin *TestPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) {
+	fakeVolume := &v1.Volume{
+		Name: volumeName,
+		VolumeSource: v1.VolumeSource{
+			GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
+				PDName:   "pdName",
+				FSType:   "ext4",
+				ReadOnly: false,
+			},
+		},
+	}
+	return volume.ReconstructedVolume{
+		Spec: volume.NewSpecFromVolume(fakeVolume),
+	}, nil
+}
+
+func (plugin *TestPlugin) NewAttacher() (volume.Attacher, error) {
+	attacher := testPluginAttacher{
+		ErrorEncountered:  &plugin.ErrorEncountered,
+		attachedVolumeMap: plugin.attachedVolumeMap,
+		pluginLock:        plugin.pluginLock,
+	}
+	return &attacher, nil
+}
+
+func (plugin *TestPlugin) NewDeviceMounter() (volume.DeviceMounter, error) {
+	return plugin.NewAttacher()
+}
+
+func (plugin *TestPlugin) NewDetacher() (volume.Detacher, error) {
+	detacher := testPluginDetacher{
+		detachedVolumeMap: plugin.detachedVolumeMap,
+		pluginLock:        plugin.pluginLock,
+	}
+	return &detacher, nil
+}
+
+func (plugin *TestPlugin) CanAttach(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
+func (plugin *TestPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
+	return true, nil
+}
+
+func (plugin *TestPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) {
+	return plugin.NewDetacher()
+}
+
+func (plugin *TestPlugin) GetDeviceMountRefs(deviceMountPath string) ([]string, error) {
+	return []string{}, nil
+}
+
+func (plugin *TestPlugin) SupportsMountOption() bool {
+	return false
+}
+
+func (plugin *TestPlugin) SupportsBulkVolumeVerification() bool {
+	return false
+}
+
+func (plugin *TestPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
+	return false, nil
+}
+
+func (plugin *TestPlugin) GetErrorEncountered() bool {
+	plugin.pluginLock.RLock()
+	defer plugin.pluginLock.RUnlock()
+	return plugin.ErrorEncountered
+}
+
+func (plugin *TestPlugin) GetAttachedVolumes() map[string][]string {
+	plugin.pluginLock.RLock()
+	defer plugin.pluginLock.RUnlock()
+	ret := make(map[string][]string)
+	for nodeName, volumeList := range plugin.attachedVolumeMap {
+		ret[nodeName] = make([]string, len(volumeList))
+		copy(ret[nodeName], volumeList)
+	}
+	return ret
+}
+
+func (plugin *TestPlugin) GetDetachedVolumes() map[string][]string {
+	plugin.pluginLock.RLock()
+	defer plugin.pluginLock.RUnlock()
+	ret := make(map[string][]string)
+	for nodeName, volumeList := range plugin.detachedVolumeMap {
+		ret[nodeName] = make([]string, len(volumeList))
+		copy(ret[nodeName], volumeList)
+	}
+	return ret
+}
+
+func CreateTestPlugin(supportCSIVolume bool) []volume.VolumePlugin {
+	attachedVolumes := make(map[string][]string)
+	detachedVolumes := make(map[string][]string)
+	return []volume.VolumePlugin{&TestPlugin{
+		SupportCSIVolume:  supportCSIVolume,
+		ErrorEncountered:  false,
+		attachedVolumeMap: attachedVolumes,
+		detachedVolumeMap: detachedVolumes,
+		pluginLock:        &sync.RWMutex{},
+	}}
+}
+
+// Attacher
+type testPluginAttacher struct {
+	ErrorEncountered  *bool
+	attachedVolumeMap map[string][]string
+	pluginLock        *sync.RWMutex
+}
+
+func (attacher *testPluginAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) (string, error) {
+	attacher.pluginLock.Lock()
+	defer attacher.pluginLock.Unlock()
+	if spec == nil {
+		*attacher.ErrorEncountered = true
+		return "", fmt.Errorf("Attach called with nil volume spec")
+	}
+	attacher.attachedVolumeMap[string(nodeName)] = append(attacher.attachedVolumeMap[string(nodeName)], spec.Name())
+	return spec.Name(), nil
+}
+
+func (attacher *testPluginAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.NodeName) (map[*volume.Spec]bool, error) {
+	return nil, nil
+}
+
+func (attacher *testPluginAttacher) WaitForAttach(spec *volume.Spec, devicePath string, pod *v1.Pod, timeout time.Duration) (string, error) {
+	attacher.pluginLock.Lock()
+	defer attacher.pluginLock.Unlock()
+	if spec == nil {
+		*attacher.ErrorEncountered = true
+		return "", fmt.Errorf("WaitForAttach called with nil volume spec")
+	}
+	fakePath := fmt.Sprintf("%s/%s", devicePath, spec.Name())
+	return fakePath, nil
+}
+
+func (attacher *testPluginAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error) {
+	attacher.pluginLock.Lock()
+	defer attacher.pluginLock.Unlock()
+	if spec == nil {
+		*attacher.ErrorEncountered = true
+		return "", fmt.Errorf("GetDeviceMountPath called with nil volume spec")
+	}
+	return "", nil
+}
+
+func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string, _ volume.DeviceMounterArgs) error {
+	attacher.pluginLock.Lock()
+	defer attacher.pluginLock.Unlock()
+	if spec == nil {
+		*attacher.ErrorEncountered = true
+		return fmt.Errorf("MountDevice called with nil volume spec")
+	}
+	return nil
+}
+
+// Detacher
+type testPluginDetacher struct {
+	detachedVolumeMap map[string][]string
+	pluginLock        *sync.RWMutex
+}
+
+func (detacher *testPluginDetacher) Detach(volumeName string, nodeName types.NodeName) error {
+	detacher.pluginLock.Lock()
+	defer detacher.pluginLock.Unlock()
+	detacher.detachedVolumeMap[string(nodeName)] = append(detacher.detachedVolumeMap[string(nodeName)], volumeName)
+	return nil
+}
+
+func (detacher *testPluginDetacher) UnmountDevice(deviceMountPath string) error {
+	return nil
+}

pkg/controller/volume/attachdetach/testing/testvolumespec.go

@@ -18,8 +18,6 @@ package testing
 
 import (
 	"fmt"
-	"sync"
-	"time"
 
 	v1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
@@ -33,8 +31,6 @@ import (
 	"k8s.io/kubernetes/pkg/volume/util"
 )
 
-const TestPluginName = "kubernetes.io/testPlugin"
-
 // GetTestVolumeSpec returns a test volume spec
 func GetTestVolumeSpec(volumeName string, diskName v1.UniqueVolumeName) *volume.Spec {
 	return &volume.Spec{
@@ -93,7 +89,7 @@ func CreateTestClient() *fake.Clientset {
 							VolumeMounts: []v1.VolumeMount{
 								{
 									Name:      "volumeMountName",
-									ReadOnly:  false,
+									ReadOnly:  true,
 									MountPath: "/mnt",
 								},
 							},
@@ -104,9 +100,10 @@ func CreateTestClient() *fake.Clientset {
 							Name: "volumeName",
 							VolumeSource: v1.VolumeSource{
 								GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
-									PDName:   "pdName",
+									PDName: "pdName",
-									FSType:   "ext4",
+									FSType: "ext4",
-									ReadOnly: false,
+									// Make the translated volume allow Multi-Attach.
+									ReadOnly: true,
 								},
 							},
 						},
@@ -125,26 +122,6 @@ func CreateTestClient() *fake.Clientset {
 		extraPods.Items = append(extraPods.Items, *pod)
 		return true, createAction.GetObject(), nil
 	})
-	fakeClient.AddReactor("list", "csinodes", func(action core.Action) (handled bool, ret runtime.Object, err error) {
-		obj := &storagev1.CSINodeList{}
-		nodeNamePrefix := "mynode"
-		for i := 0; i < 5; i++ {
-			var nodeName string
-			if i != 0 {
-				nodeName = fmt.Sprintf("%s-%d", nodeNamePrefix, i)
-			} else {
-				// We want also the "mynode" node since all the testing pods live there
-				nodeName = nodeNamePrefix
-			}
-			csiNode := storagev1.CSINode{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: nodeName,
-				},
-			}
-			obj.Items = append(obj.Items, csiNode)
-		}
-		return true, obj, nil
-	})
 	nodes = &v1.NodeList{}
 	nodeNamePrefix := "mynode"
 	for i := 0; i < 5; i++ {
@@ -173,6 +150,23 @@ func CreateTestClient() *fake.Clientset {
 		obj.Items = append(obj.Items, nodes.Items...)
 		return true, obj, nil
 	})
+	fakeClient.AddReactor("list", "csinodes", func(action core.Action) (handled bool, ret runtime.Object, err error) {
+		obj := &storagev1.CSINodeList{}
+		for _, node := range nodes.Items {
+			csiNode := storagev1.CSINode{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						// All the in-tree plugins have been migrated to CSI since v1.27.
+						// So hardcoding the migrated plugins here.
+						"storage.alpha.kubernetes.io/migrated-plugins": "kubernetes.io/aws-ebs,kubernetes.io/azure-disk,kubernetes.io/azure-file,kubernetes.io/cinder,kubernetes.io/gce-pd,kubernetes.io/vsphere-volume",
+					},
+					Name: node.Name,
+				},
+			}
+			obj.Items = append(obj.Items, csiNode)
+		}
+		return true, obj, nil
+	})
 	volumeAttachments = &storagev1.VolumeAttachmentList{}
 	fakeClient.AddReactor("list", "volumeattachments", func(action core.Action) (handled bool, ret runtime.Object, err error) {
 		obj := &storagev1.VolumeAttachmentList{}
@@ -235,7 +229,7 @@ func NewPodWithVolume(podName, volumeName, nodeName string) *v1.Pod {
 					VolumeMounts: []v1.VolumeMount{
 						{
 							Name:      "volumeMountName",
-							ReadOnly:  false,
+							ReadOnly:  true,
 							MountPath: "/mnt",
 						},
 					},
@@ -246,9 +240,10 @@ func NewPodWithVolume(podName, volumeName, nodeName string) *v1.Pod {
 					Name: volumeName,
 					VolumeSource: v1.VolumeSource{
 						GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
-							PDName:   "pdName",
+							PDName: "pdName",
-							FSType:   "ext4",
+							FSType: "ext4",
-							ReadOnly: false,
+							// Make the translated volume allow Multi-Attach.
+							ReadOnly: true,
 						},
 					},
 				},
@@ -348,241 +343,3 @@ func attachVolumeToNode(nodes *v1.NodeList, volumeName, nodeName string, inUse b
 		node.Status.VolumesInUse = append(node.Status.VolumesInUse, uniqueVolumeName)
 	}
 }
-
-type TestPlugin struct {
-	ErrorEncountered  bool
-	attachedVolumeMap map[string][]string
-	detachedVolumeMap map[string][]string
-	pluginLock        *sync.RWMutex
-}
-
-func (plugin *TestPlugin) Init(host volume.VolumeHost) error {
-	return nil
-}
-
-func (plugin *TestPlugin) GetPluginName() string {
-	return TestPluginName
-}
-
-func (plugin *TestPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
-	plugin.pluginLock.Lock()
-	defer plugin.pluginLock.Unlock()
-	if spec == nil {
-		plugin.ErrorEncountered = true
-		return "", fmt.Errorf("GetVolumeName called with nil volume spec")
-	}
-	if spec.Volume != nil {
-		return spec.Name(), nil
-	} else if spec.PersistentVolume != 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
-	}
-}
-
-func (plugin *TestPlugin) CanSupport(spec *volume.Spec) bool {
-	plugin.pluginLock.Lock()
-	defer plugin.pluginLock.Unlock()
-	if spec == nil {
-		plugin.ErrorEncountered = true
-	}
-	return true
-}
-
-func (plugin *TestPlugin) RequiresRemount(spec *volume.Spec) bool {
-	return false
-}
-
-func (plugin *TestPlugin) NewMounter(spec *volume.Spec, podRef *v1.Pod, opts volume.VolumeOptions) (volume.Mounter, error) {
-	plugin.pluginLock.Lock()
-	defer plugin.pluginLock.Unlock()
-	if spec == nil {
-		plugin.ErrorEncountered = true
-	}
-	return nil, nil
-}
-
-func (plugin *TestPlugin) NewUnmounter(name string, podUID types.UID) (volume.Unmounter, error) {
-	return nil, nil
-}
-
-func (plugin *TestPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) {
-	fakeVolume := &v1.Volume{
-		Name: volumeName,
-		VolumeSource: v1.VolumeSource{
-			GCEPersistentDisk: &v1.GCEPersistentDiskVolumeSource{
-				PDName:   "pdName",
-				FSType:   "ext4",
-				ReadOnly: false,
-			},
-		},
-	}
-	return volume.ReconstructedVolume{
-		Spec: volume.NewSpecFromVolume(fakeVolume),
-	}, nil
-}
-
-func (plugin *TestPlugin) NewAttacher() (volume.Attacher, error) {
-	attacher := testPluginAttacher{
-		ErrorEncountered:  &plugin.ErrorEncountered,
-		attachedVolumeMap: plugin.attachedVolumeMap,
-		pluginLock:        plugin.pluginLock,
-	}
-	return &attacher, nil
-}
-
-func (plugin *TestPlugin) NewDeviceMounter() (volume.DeviceMounter, error) {
-	return plugin.NewAttacher()
-}
-
-func (plugin *TestPlugin) NewDetacher() (volume.Detacher, error) {
-	detacher := testPluginDetacher{
-		detachedVolumeMap: plugin.detachedVolumeMap,
-		pluginLock:        plugin.pluginLock,
-	}
-	return &detacher, nil
-}
-
-func (plugin *TestPlugin) CanAttach(spec *volume.Spec) (bool, error) {
-	return true, nil
-}
-
-func (plugin *TestPlugin) CanDeviceMount(spec *volume.Spec) (bool, error) {
-	return true, nil
-}
-
-func (plugin *TestPlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, error) {
-	return plugin.NewDetacher()
-}
-
-func (plugin *TestPlugin) GetDeviceMountRefs(deviceMountPath string) ([]string, error) {
-	return []string{}, nil
-}
-
-func (plugin *TestPlugin) SupportsMountOption() bool {
-	return false
-}
-
-func (plugin *TestPlugin) SupportsBulkVolumeVerification() bool {
-	return false
-}
-
-func (plugin *TestPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
-	return false, nil
-}
-
-func (plugin *TestPlugin) GetErrorEncountered() bool {
-	plugin.pluginLock.RLock()
-	defer plugin.pluginLock.RUnlock()
-	return plugin.ErrorEncountered
-}
-
-func (plugin *TestPlugin) GetAttachedVolumes() map[string][]string {
-	plugin.pluginLock.RLock()
-	defer plugin.pluginLock.RUnlock()
-	ret := make(map[string][]string)
-	for nodeName, volumeList := range plugin.attachedVolumeMap {
-		ret[nodeName] = make([]string, len(volumeList))
-		copy(ret[nodeName], volumeList)
-	}
-	return ret
-}
-
-func (plugin *TestPlugin) GetDetachedVolumes() map[string][]string {
-	plugin.pluginLock.RLock()
-	defer plugin.pluginLock.RUnlock()
-	ret := make(map[string][]string)
-	for nodeName, volumeList := range plugin.detachedVolumeMap {
-		ret[nodeName] = make([]string, len(volumeList))
-		copy(ret[nodeName], volumeList)
-	}
-	return ret
-}
-
-func CreateTestPlugin() []volume.VolumePlugin {
-	attachedVolumes := make(map[string][]string)
-	detachedVolumes := make(map[string][]string)
-	return []volume.VolumePlugin{&TestPlugin{
-		ErrorEncountered:  false,
-		attachedVolumeMap: attachedVolumes,
-		detachedVolumeMap: detachedVolumes,
-		pluginLock:        &sync.RWMutex{},
-	}}
-}
-
-// Attacher
-type testPluginAttacher struct {
-	ErrorEncountered  *bool
-	attachedVolumeMap map[string][]string
-	pluginLock        *sync.RWMutex
-}
-
-func (attacher *testPluginAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) (string, error) {
-	attacher.pluginLock.Lock()
-	defer attacher.pluginLock.Unlock()
-	if spec == nil {
-		*attacher.ErrorEncountered = true
-		return "", fmt.Errorf("Attach called with nil volume spec")
-	}
-	attacher.attachedVolumeMap[string(nodeName)] = append(attacher.attachedVolumeMap[string(nodeName)], spec.Name())
-	return spec.Name(), nil
-}
-
-func (attacher *testPluginAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.NodeName) (map[*volume.Spec]bool, error) {
-	return nil, nil
-}
-
-func (attacher *testPluginAttacher) WaitForAttach(spec *volume.Spec, devicePath string, pod *v1.Pod, timeout time.Duration) (string, error) {
-	attacher.pluginLock.Lock()
-	defer attacher.pluginLock.Unlock()
-	if spec == nil {
-		*attacher.ErrorEncountered = true
-		return "", fmt.Errorf("WaitForAttach called with nil volume spec")
-	}
-	fakePath := fmt.Sprintf("%s/%s", devicePath, spec.Name())
-	return fakePath, nil
-}
-
-func (attacher *testPluginAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error) {
-	attacher.pluginLock.Lock()
-	defer attacher.pluginLock.Unlock()
-	if spec == nil {
-		*attacher.ErrorEncountered = true
-		return "", fmt.Errorf("GetDeviceMountPath called with nil volume spec")
-	}
-	return "", nil
-}
-
-func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string, _ volume.DeviceMounterArgs) error {
-	attacher.pluginLock.Lock()
-	defer attacher.pluginLock.Unlock()
-	if spec == nil {
-		*attacher.ErrorEncountered = true
-		return fmt.Errorf("MountDevice called with nil volume spec")
-	}
-	return nil
-}
-
-// Detacher
-type testPluginDetacher struct {
-	detachedVolumeMap map[string][]string
-	pluginLock        *sync.RWMutex
-}
-
-func (detacher *testPluginDetacher) Detach(volumeName string, nodeName types.NodeName) error {
-	detacher.pluginLock.Lock()
-	defer detacher.pluginLock.Unlock()
-	detacher.detachedVolumeMap[string(nodeName)] = append(detacher.detachedVolumeMap[string(nodeName)], volumeName)
-	return nil
-}
-
-func (detacher *testPluginDetacher) UnmountDevice(deviceMountPath string) error {
-	return nil
-}

pkg/controller/volume/attachdetach/util/util_test.go

@@ -217,7 +217,7 @@ func Test_CreateVolumeSpec(t *testing.T) {
 			},
 		},
 		{
-			desc:           "CSINode not found for a volume type that supports csi migration",
+			desc:           "CSINode not found for a volume type that completes csi migration",
 			createNodeName: kubetypes.NodeName("another-node"),
 			pod: &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{
@@ -238,7 +238,6 @@ func Test_CreateVolumeSpec(t *testing.T) {
 					},
 				},
 			},
-			wantErrorMessage: "csiNode \"another-node\" not found",
 		},
 	} {
 		t.Run(test.desc, func(t *testing.T) {

pkg/volume/csimigration/plugin_manager_test.go

@@ -95,7 +95,6 @@ func TestMigrationFeatureFlagStatus(t *testing.T) {
 	testCases := []struct {
 		name                          string
 		pluginName                    string
-		csiMigrationEnabled           bool
 		pluginFeature                 featuregate.Feature
 		pluginFeatureEnabled          bool
 		inTreePluginUnregister        featuregate.Feature
@@ -104,21 +103,41 @@ func TestMigrationFeatureFlagStatus(t *testing.T) {
 		csiMigrationCompleteResult    bool
 	}{
 		{
-			name:                          "gce-pd migration flag enabled and migration-complete flag disabled with CSI migration flag",
+			name:                          "portworx-volume migration flag disabled and migration-complete flag disabled with CSI migration flag",
-			pluginName:                    "kubernetes.io/gce-pd",
+			pluginName:                    "kubernetes.io/portworx-volume",
+			pluginFeature:                 features.CSIMigrationPortworx,
+			pluginFeatureEnabled:          false,
+			inTreePluginUnregister:        features.InTreePluginPortworxUnregister,
+			inTreePluginUnregisterEnabled: false,
+			csiMigrationResult:            false,
+			csiMigrationCompleteResult:    false,
+		},
+		{
+			name:                          "portworx-volume migration flag disabled and migration-complete flag enabled with CSI migration flag",
+			pluginName:                    "kubernetes.io/portworx-volume",
+			pluginFeature:                 features.CSIMigrationPortworx,
+			pluginFeatureEnabled:          false,
+			inTreePluginUnregister:        features.InTreePluginPortworxUnregister,
+			inTreePluginUnregisterEnabled: true,
+			csiMigrationResult:            false,
+			csiMigrationCompleteResult:    false,
+		},
+		{
+			name:                          "portworx-volume migration flag enabled and migration-complete flag disabled with CSI migration flag",
+			pluginName:                    "kubernetes.io/portworx-volume",
+			pluginFeature:                 features.CSIMigrationPortworx,
 			pluginFeatureEnabled:          true,
-			csiMigrationEnabled:           true,
+			inTreePluginUnregister:        features.InTreePluginPortworxUnregister,
-			inTreePluginUnregister:        features.InTreePluginGCEUnregister,
 			inTreePluginUnregisterEnabled: false,
 			csiMigrationResult:            true,
 			csiMigrationCompleteResult:    false,
 		},
 		{
-			name:                          "gce-pd migration flag enabled and migration-complete flag enabled with CSI migration flag",
+			name:                          "portworx-volume migration flag enabled and migration-complete flag enabled with CSI migration flag",
-			pluginName:                    "kubernetes.io/gce-pd",
+			pluginName:                    "kubernetes.io/portworx-volume",
+			pluginFeature:                 features.CSIMigrationPortworx,
 			pluginFeatureEnabled:          true,
-			csiMigrationEnabled:           true,
+			inTreePluginUnregister:        features.InTreePluginPortworxUnregister,
-			inTreePluginUnregister:        features.InTreePluginGCEUnregister,
 			inTreePluginUnregisterEnabled: true,
 			csiMigrationResult:            true,
 			csiMigrationCompleteResult:    true,
@@ -128,9 +147,6 @@ 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) {
-			// 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 len(test.pluginFeature) > 0 {
 				featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)
 			}