diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go index 2bc01b05b4c..52ff5348125 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go +++ b/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: 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{}) - newPod1 := controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1") - attachDetachRecoveryTestCase(t, []*v1.Pod{newPod1}, []*v1.Pod{}) - newPod1 = controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1") - attachDetachRecoveryTestCase(t, []*v1.Pod{}, []*v1.Pod{newPod1}) - newPod1 = controllervolumetesting.NewPodWithVolume("newpod-1", "volumeName2", "mynode-1") - newPod2 := controllervolumetesting.NewPodWithVolume("newpod-2", "volumeName3", "mynode-1") - attachDetachRecoveryTestCase(t, []*v1.Pod{newPod1}, []*v1.Pod{newPod2}) + testCases := []struct { + testName string + extraPods1 []*v1.Pod + extraPods2 []*v1.Pod + }{ + { + testName: "No extra pods", + }, + { + 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() + plugins := controllervolumetesting.CreateTestPlugin(true) 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() diff --git a/pkg/controller/volume/attachdetach/testing/plugin.go b/pkg/controller/volume/attachdetach/testing/plugin.go new file mode 100644 index 00000000000..3a224e90de1 --- /dev/null +++ b/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 +} diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index 262951d23a5..93809a23624 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/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", - FSType: "ext4", - ReadOnly: false, + PDName: "pdName", + FSType: "ext4", + // 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", - FSType: "ext4", - ReadOnly: false, + PDName: "pdName", + FSType: "ext4", + // 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 -} diff --git a/pkg/controller/volume/attachdetach/util/util_test.go b/pkg/controller/volume/attachdetach/util/util_test.go index b075189c48a..7d5b8c9e8db 100644 --- a/pkg/controller/volume/attachdetach/util/util_test.go +++ b/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) { diff --git a/pkg/volume/csimigration/plugin_manager_test.go b/pkg/volume/csimigration/plugin_manager_test.go index e9b18ead720..0eeb762eb65 100644 --- a/pkg/volume/csimigration/plugin_manager_test.go +++ b/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", - pluginName: "kubernetes.io/gce-pd", + name: "portworx-volume migration flag disabled and migration-complete flag disabled with CSI migration flag", + 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.InTreePluginGCEUnregister, + inTreePluginUnregister: features.InTreePluginPortworxUnregister, inTreePluginUnregisterEnabled: false, csiMigrationResult: true, csiMigrationCompleteResult: false, }, { - name: "gce-pd migration flag enabled and migration-complete flag enabled with CSI migration flag", - pluginName: "kubernetes.io/gce-pd", + name: "portworx-volume migration flag enabled and migration-complete flag enabled with CSI migration flag", + pluginName: "kubernetes.io/portworx-volume", + pluginFeature: features.CSIMigrationPortworx, pluginFeatureEnabled: true, - csiMigrationEnabled: true, - inTreePluginUnregister: features.InTreePluginGCEUnregister, + inTreePluginUnregister: features.InTreePluginPortworxUnregister, 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) }