From 96b7de3b9b4c932df2d3d1decbdb26dd7eba6ce9 Mon Sep 17 00:00:00 2001 From: mlmhl Date: Sun, 17 Dec 2017 20:58:28 +0800 Subject: [PATCH] remove dependence of VolumePluginMgr from DesiredStateOfWorld and ActualStateOfWorld --- .../app/controllermanager.go | 21 +-- cmd/kube-controller-manager/app/core.go | 1 + .../attachdetach/attach_detach_controller.go | 9 +- .../attach_detach_controller_test.go | 12 +- .../desired_state_of_world_populator_test.go | 2 +- .../reconciler/reconciler_test.go | 77 +++++++---- .../volume/cache/actual_state_of_world.go | 36 +---- .../cache/actual_state_of_world_test.go | 125 ++++++++---------- .../volume/cache/desired_state_of_world.go | 32 +---- .../cache/desired_state_of_world_test.go | 123 +++++++---------- pkg/controller/volume/util/util.go | 25 ++-- .../operationexecutor/operation_generator.go | 15 ++- pkg/volume/util/volumehelper/volumehelper.go | 33 +++++ 13 files changed, 251 insertions(+), 260 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 8697214e09c3d..dda330f6f4e78 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -56,6 +56,7 @@ import ( "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/controller" serviceaccountcontroller "k8s.io/kubernetes/pkg/controller/serviceaccount" + "k8s.io/kubernetes/pkg/controller/volume/cache" "k8s.io/kubernetes/pkg/serviceaccount" "k8s.io/kubernetes/pkg/util/configz" "k8s.io/kubernetes/pkg/version" @@ -280,6 +281,9 @@ type ControllerContext struct { // InformersStarted is closed after all of the controllers have been initialized and are running. After this point it is safe, // for an individual controller to start the shared informers. Before it is closed, they should not. InformersStarted chan struct{} + + // DesiredStateOfWorld is the desired state of volumes used by AttachDetachController and ExpandController. + DesiredStateOfWorld cache.DesiredStateOfWorld } func (c ControllerContext) IsControllerEnabled(name string) bool { @@ -473,14 +477,15 @@ func CreateControllerContext(s *options.CMServer, rootClientBuilder, clientBuild } ctx := ControllerContext{ - ClientBuilder: clientBuilder, - InformerFactory: sharedInformers, - Options: *s, - AvailableResources: availableResources, - Cloud: cloud, - LoopMode: loopMode, - Stop: stop, - InformersStarted: make(chan struct{}), + ClientBuilder: clientBuilder, + InformerFactory: sharedInformers, + Options: *s, + AvailableResources: availableResources, + Cloud: cloud, + LoopMode: loopMode, + Stop: stop, + InformersStarted: make(chan struct{}), + DesiredStateOfWorld: cache.NewDesiredStateOfWorld(), } return ctx, nil } diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index 819d18e0f013b..010df32350578 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -187,6 +187,7 @@ func startAttachDetachController(ctx ControllerContext) (bool, error) { ctx.Options.DisableAttachDetachReconcilerSync, ctx.Options.ReconcilerSyncLoopPeriod.Duration, attachdetach.DefaultTimerConfig, + ctx.DesiredStateOfWorld, ) if attachDetachControllerErr != nil { return true, fmt.Errorf("failed to start attach/detach controller: %v", attachDetachControllerErr) diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index 7f255030fa115..c0249e5ce9922 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -37,10 +37,10 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/controller/volume/cache" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/populator" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/reconciler" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/statusupdater" + "k8s.io/kubernetes/pkg/controller/volume/cache" "k8s.io/kubernetes/pkg/controller/volume/util" "k8s.io/kubernetes/pkg/util/io" "k8s.io/kubernetes/pkg/util/mount" @@ -101,7 +101,8 @@ func NewAttachDetachController( prober volume.DynamicPluginProber, disableReconciliationSync bool, reconcilerSyncDuration time.Duration, - timerConfig TimerConfig) (AttachDetachController, error) { + timerConfig TimerConfig, + desiredStateOfWorld cache.DesiredStateOfWorld) (AttachDetachController, error) { // TODO: The default resyncPeriod for shared informers is 12 hours, this is // unacceptable for the attach/detach controller. For example, if a pod is // skipped because the node it is scheduled to didn't set its annotation in @@ -139,8 +140,8 @@ func NewAttachDetachController( recorder := eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "attachdetach-controller"}) blkutil := volumeutil.NewBlockVolumePathHandler() - adc.desiredStateOfWorld = cache.NewDesiredStateOfWorld(&adc.volumePluginMgr) - adc.actualStateOfWorld = cache.NewActualStateOfWorld(&adc.volumePluginMgr) + adc.desiredStateOfWorld = desiredStateOfWorld + adc.actualStateOfWorld = cache.NewActualStateOfWorld() adc.attacherDetacher = operationexecutor.NewOperationExecutor(operationexecutor.NewOperationGenerator( kubeClient, diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go index 03f1a16c35b9c..9a06c0fe75331 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go @@ -27,8 +27,8 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/informers" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/controller/volume/cache" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" + "k8s.io/kubernetes/pkg/controller/volume/cache" "k8s.io/kubernetes/pkg/volume" ) @@ -49,7 +49,8 @@ func Test_NewAttachDetachController_Positive(t *testing.T) { nil, /* prober */ false, 5*time.Second, - DefaultTimerConfig) + DefaultTimerConfig, + cache.NewDesiredStateOfWorld()) // Assert if err != nil { @@ -87,8 +88,8 @@ func Test_AttachDetachControllerStateOfWolrdPopulators_Positive(t *testing.T) { t.Fatalf("Could not initialize volume plugins for Attach/Detach Controller: %+v", err) } - adc.actualStateOfWorld = cache.NewActualStateOfWorld(&adc.volumePluginMgr) - adc.desiredStateOfWorld = cache.NewDesiredStateOfWorld(&adc.volumePluginMgr) + adc.actualStateOfWorld = cache.NewActualStateOfWorld() + adc.desiredStateOfWorld = cache.NewDesiredStateOfWorld() err := adc.populateActualStateOfWorld() if err != nil { @@ -223,7 +224,8 @@ func attachDetachRecoveryTestCase(t *testing.T, extraPods1 []*v1.Pod, extraPods2 prober, false, 1*time.Second, - DefaultTimerConfig) + DefaultTimerConfig, + cache.NewDesiredStateOfWorld()) if err != nil { t.Fatalf("Run failed with error. Expected: Actual: <%v>", err) 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 753be454977ea..708c0327491b7 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 @@ -38,7 +38,7 @@ func TestFindAndAddActivePods_FindAndRemoveDeletedPods(t *testing.T) { fakeInformerFactory := informers.NewSharedInformerFactory(fakeClient, controller.NoResyncPeriodFunc()) fakePodInformer := fakeInformerFactory.Core().V1().Pods() - fakesDSW := cache.NewDesiredStateOfWorld(fakeVolumePluginMgr) + fakesDSW := cache.NewDesiredStateOfWorld() pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go b/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go index a76f1ff9a510f..0e5de8c4d9b2e 100644 --- a/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go +++ b/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go @@ -26,12 +26,13 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/controller/volume/cache" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/statusupdater" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" + "k8s.io/kubernetes/pkg/controller/volume/cache" volumetesting "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util/operationexecutor" "k8s.io/kubernetes/pkg/volume/util/types" + "k8s.io/kubernetes/pkg/volume/util/volumehelper" ) const ( @@ -46,8 +47,8 @@ const ( func Test_Run_Positive_DoNothing(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -82,8 +83,8 @@ func Test_Run_Positive_DoNothing(t *testing.T) { func Test_Run_Positive_OneDesiredVolumeAttach(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -109,7 +110,12 @@ func Test_Run_Positive_OneDesiredVolumeAttach(t *testing.T) { nodeName) } - _, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + volumeNameFromSpec, getVolumeNameErr := volumehelper.GetUniqueVolumeNameFromSpec(fakePlugin, volumeSpec) + if getVolumeNameErr != nil { + t.Fatalf("GetUniqueVolumeNameFromSpec. Expected: Actual: <%v>", getVolumeNameErr) + } + + _, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeNameFromSpec) if podErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podErr) } @@ -134,8 +140,8 @@ func Test_Run_Positive_OneDesiredVolumeAttach(t *testing.T) { func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithUnmountedVolume(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -161,7 +167,12 @@ func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithUnmountedVolume(t *te nodeName) } - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + volumeNameFromSpec, getVolumeNameErr := volumehelper.GetUniqueVolumeNameFromSpec(fakePlugin, volumeSpec) + if getVolumeNameErr != nil { + t.Fatalf("GetUniqueVolumeNameFromSpec. Expected: Actual: <%v>", getVolumeNameErr) + } + + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } @@ -207,8 +218,8 @@ func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithUnmountedVolume(t *te func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithMountedVolume(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -234,7 +245,12 @@ func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithMountedVolume(t *test nodeName) } - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + volumeNameFromSpec, getVolumeNameErr := volumehelper.GetUniqueVolumeNameFromSpec(fakePlugin, volumeSpec) + if getVolumeNameErr != nil { + t.Fatalf("GetUniqueVolumeNameFromSpec. Expected: Actual: <%v>", getVolumeNameErr) + } + + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } @@ -280,8 +296,8 @@ func Test_Run_Positive_OneDesiredVolumeAttachThenDetachWithMountedVolume(t *test func Test_Run_Negative_OneDesiredVolumeAttachThenDetachWithUnmountedVolumeUpdateStatusFail(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -307,7 +323,12 @@ func Test_Run_Negative_OneDesiredVolumeAttachThenDetachWithUnmountedVolumeUpdate nodeName) } - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + volumeNameFromSpec, getVolumeNameErr := volumehelper.GetUniqueVolumeNameFromSpec(fakePlugin, volumeSpec) + if getVolumeNameErr != nil { + t.Fatalf("GetUniqueVolumeNameFromSpec. Expected: Actual: <%v>", getVolumeNameErr) + } + + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } @@ -356,8 +377,8 @@ func Test_Run_Negative_OneDesiredVolumeAttachThenDetachWithUnmountedVolumeUpdate func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteMany(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -380,11 +401,16 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteMany(t *testing. dsw.AddNode(nodeName1, false /*keepTerminatedPodVolumes*/) dsw.AddNode(nodeName2, false /*keepTerminatedPodVolumes*/) - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName1), controllervolumetesting.NewPod(podName1, podName1), volumeSpec, nodeName1) + volumeNameFromSpec, getVolumeNameErr := volumehelper.GetUniqueVolumeNameFromSpec(fakePlugin, volumeSpec) + if getVolumeNameErr != nil { + t.Fatalf("GetUniqueVolumeNameFromSpec. Expected: Actual: <%v>", getVolumeNameErr) + } + + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName1), controllervolumetesting.NewPod(podName1, podName1), volumeSpec, nodeName1, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } - _, podAddErr = dsw.AddPod(types.UniquePodName(podName2), controllervolumetesting.NewPod(podName2, podName2), volumeSpec, nodeName2) + _, podAddErr = dsw.AddPod(types.UniquePodName(podName2), controllervolumetesting.NewPod(podName2, podName2), volumeSpec, nodeName2, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } @@ -448,8 +474,8 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteMany(t *testing. func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteOnce(t *testing.T) { // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) - dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) - asw := cache.NewActualStateOfWorld(volumePluginMgr) + dsw := cache.NewDesiredStateOfWorld() + asw := cache.NewActualStateOfWorld() fakeKubeClient := controllervolumetesting.CreateTestClient() fakeRecorder := &record.FakeRecorder{} fakeHandler := volumetesting.NewBlockVolumePathHandler() @@ -472,12 +498,17 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteOnce(t *testing. dsw.AddNode(nodeName1, false /*keepTerminatedPodVolumes*/) dsw.AddNode(nodeName2, false /*keepTerminatedPodVolumes*/) + volumeNameFromSpec, getVolumeNameErr := volumehelper.GetUniqueVolumeNameFromSpec(fakePlugin, volumeSpec) + if getVolumeNameErr != nil { + t.Fatalf("GetUniqueVolumeNameFromSpec. Expected: Actual: <%v>", getVolumeNameErr) + } + // Add both pods at the same time to provoke a potential race condition in the reconciler - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName1), controllervolumetesting.NewPod(podName1, podName1), volumeSpec, nodeName1) + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName1), controllervolumetesting.NewPod(podName1, podName1), volumeSpec, nodeName1, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } - _, podAddErr = dsw.AddPod(types.UniquePodName(podName2), controllervolumetesting.NewPod(podName2, podName2), volumeSpec, nodeName2) + _, podAddErr = dsw.AddPod(types.UniquePodName(podName2), controllervolumetesting.NewPod(podName2, podName2), volumeSpec, nodeName2, volumeNameFromSpec) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } diff --git a/pkg/controller/volume/cache/actual_state_of_world.go b/pkg/controller/volume/cache/actual_state_of_world.go index 4d4a7523d8da2..706d420983b04 100644 --- a/pkg/controller/volume/cache/actual_state_of_world.go +++ b/pkg/controller/volume/cache/actual_state_of_world.go @@ -32,7 +32,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util/operationexecutor" - "k8s.io/kubernetes/pkg/volume/util/volumehelper" ) // ActualStateOfWorld defines a set of thread-safe operations supported on @@ -145,11 +144,10 @@ type AttachedVolume struct { } // NewActualStateOfWorld returns a new instance of ActualStateOfWorld. -func NewActualStateOfWorld(volumePluginMgr *volume.VolumePluginMgr) ActualStateOfWorld { +func NewActualStateOfWorld() ActualStateOfWorld { return &actualStateOfWorld{ attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume), nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor), - volumePluginMgr: volumePluginMgr, } } @@ -166,10 +164,6 @@ type actualStateOfWorld struct { // the node (including the list of volumes to report attached). nodesToUpdateStatusFor map[types.NodeName]nodeToUpdateStatusFor - // volumePluginMgr is the volume plugin manager used to create volume - // plugin objects. - volumePluginMgr *volume.VolumePluginMgr - sync.RWMutex } @@ -261,36 +255,10 @@ func (asw *actualStateOfWorld) AddVolumeToReportAsAttached( } func (asw *actualStateOfWorld) AddVolumeNode( - uniqueName v1.UniqueVolumeName, volumeSpec *volume.Spec, nodeName types.NodeName, devicePath string) (v1.UniqueVolumeName, error) { + volumeName v1.UniqueVolumeName, volumeSpec *volume.Spec, nodeName types.NodeName, devicePath string) (v1.UniqueVolumeName, error) { asw.Lock() defer asw.Unlock() - var volumeName v1.UniqueVolumeName - if volumeSpec != nil { - attachableVolumePlugin, err := asw.volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) - if err != nil || attachableVolumePlugin == nil { - return "", fmt.Errorf( - "failed to get AttachablePlugin from volumeSpec for volume %q err=%v", - volumeSpec.Name(), - err) - } - - volumeName, err = volumehelper.GetUniqueVolumeNameFromSpec( - attachableVolumePlugin, volumeSpec) - if err != nil { - return "", fmt.Errorf( - "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q err=%v", - volumeSpec.Name(), - err) - } - } else { - // volumeSpec is nil - // This happens only on controller startup when reading the volumes from node - // status; if the pods using the volume have been removed and are unreachable - // the volumes should be detached immediately and the spec is not needed - volumeName = uniqueName - } - volumeObj, volumeExists := asw.attachedVolumes[volumeName] if !volumeExists { volumeObj = attachedVolume{ diff --git a/pkg/controller/volume/cache/actual_state_of_world_test.go b/pkg/controller/volume/cache/actual_state_of_world_test.go index 14a531ba10366..c2b2a24cddfa2 100644 --- a/pkg/controller/volume/cache/actual_state_of_world_test.go +++ b/pkg/controller/volume/cache/actual_state_of_world_test.go @@ -24,14 +24,14 @@ import ( "k8s.io/apimachinery/pkg/types" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" volumetesting "k8s.io/kubernetes/pkg/volume/testing" + "k8s.io/kubernetes/pkg/volume/util/volumehelper" ) // Calls AddVolumeNode() once. // Verifies a single volume/node entry exists. func Test_AddVolumeNode_Positive_NewVolumeNewNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) @@ -63,8 +63,7 @@ func Test_AddVolumeNode_Positive_NewVolumeNewNode(t *testing.T) { // Verifies two volume/node entries exist with the same volumeSpec. func Test_AddVolumeNode_Positive_ExistingVolumeNewNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := types.NodeName("node1-name") @@ -113,8 +112,7 @@ func Test_AddVolumeNode_Positive_ExistingVolumeNewNode(t *testing.T) { // Verifies a single volume/node entry exists. func Test_AddVolumeNode_Positive_ExistingVolumeExistingNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -157,8 +155,7 @@ func Test_AddVolumeNode_Positive_ExistingVolumeExistingNode(t *testing.T) { // Verifies no volume/node entries exists. func Test_DeleteVolumeNode_Positive_VolumeExistsNodeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -187,8 +184,7 @@ func Test_DeleteVolumeNode_Positive_VolumeExistsNodeExists(t *testing.T) { // Verifies no volume/node entries exists. func Test_DeleteVolumeNode_Positive_VolumeDoesntExistNodeDoesntExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") nodeName := types.NodeName("node-name") @@ -213,8 +209,7 @@ func Test_DeleteVolumeNode_Positive_VolumeDoesntExistNodeDoesntExist(t *testing. // Verifies only second volume/node entry exists. func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := types.NodeName("node1-name") @@ -262,8 +257,7 @@ func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) { // Verifies the populated volume/node entry exists. func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -294,8 +288,7 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) { // Verifies requested entry does not exist, but populated entry does. func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := types.NodeName("node1-name") @@ -326,8 +319,7 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) { // Verifies requested entry does not exist. func Test_VolumeNodeExists_Positive_VolumeAndNodeDontExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") nodeName := types.NodeName("node-name") @@ -349,8 +341,7 @@ func Test_VolumeNodeExists_Positive_VolumeAndNodeDontExist(t *testing.T) { // Verifies no volume/node entries are returned. func Test_GetAttachedVolumes_Positive_NoVolumesOrNodes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() // Act attachedVolumes := asw.GetAttachedVolumes() @@ -366,8 +357,7 @@ func Test_GetAttachedVolumes_Positive_NoVolumesOrNodes(t *testing.T) { // Verifies one volume/node entry is returned. func Test_GetAttachedVolumes_Positive_OneVolumeOneNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -393,8 +383,7 @@ func Test_GetAttachedVolumes_Positive_OneVolumeOneNode(t *testing.T) { // Verifies both volume/node entries are returned. func Test_GetAttachedVolumes_Positive_TwoVolumeTwoNodes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) node1Name := types.NodeName("node1-name") @@ -429,17 +418,22 @@ func Test_GetAttachedVolumes_Positive_TwoVolumeTwoNodes(t *testing.T) { func Test_GetAttachedVolumes_Positive_OneVolumeTwoNodes(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) + realVolumeName, getNameErr := volumehelper.GetVolumeNameFromPluginMgr( + v1.UniqueVolumeName(""), volumeSpec, volumePluginMgr) + if getNameErr != nil { + t.Fatalf("GetVolumeNameFromPluginMgr failed. Expected: Actual: <%v>", getNameErr) + } node1Name := types.NodeName("node1-name") devicePath := "fake/device/path" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath) + generatedVolumeName1, add1Err := asw.AddVolumeNode(realVolumeName, volumeSpec, node1Name, devicePath) if add1Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add1Err) } node2Name := types.NodeName("node2-name") - generatedVolumeName2, add2Err := asw.AddVolumeNode(v1.UniqueVolumeName(""), volumeSpec, node2Name, devicePath) + generatedVolumeName2, add2Err := asw.AddVolumeNode(realVolumeName, volumeSpec, node2Name, devicePath) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -467,8 +461,7 @@ func Test_GetAttachedVolumes_Positive_OneVolumeTwoNodes(t *testing.T) { // Verifies mountedByNode is true and DetachRequestedTime is zero. func Test_SetVolumeMountedByNode_Positive_Set(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -494,8 +487,7 @@ func Test_SetVolumeMountedByNode_Positive_Set(t *testing.T) { // Verifies mountedByNode is false. func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -530,8 +522,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) { // Verifies mountedByNode is false because value is overwritten func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -570,8 +561,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) { // Verifies mountedByNode is false and detachRequestedTime is zero. func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotReset(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -611,8 +601,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotRes // Verifies mountedByNode is false and detachRequestedTime is NOT zero. func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequestedTimePerserved(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -658,8 +647,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequest // Verifies mountedByNode is true and detachRequestedTime is zero (default values). func Test_RemoveVolumeFromReportAsAttached_Positive_Set(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") devicePath := "fake/device/path" volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) @@ -685,8 +673,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_Set(t *testing.T) { // Verifies mountedByNode is true and detachRequestedTime is NOT zero. func Test_RemoveVolumeFromReportAsAttached_Positive_Marked(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -721,8 +708,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_Marked(t *testing.T) { // Verifies mountedByNode is true and detachRequestedTime is reset to zero. func Test_MarkDesireToDetach_Positive_MarkedAddVolumeNodeReset(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -764,8 +750,7 @@ func Test_MarkDesireToDetach_Positive_MarkedAddVolumeNodeReset(t *testing.T) { // Verifies mountedByNode is false and detachRequestedTime is NOT zero. func Test_RemoveVolumeFromReportAsAttached_Positive_UnsetWithInitialSetVolumeMountedByNodePreserved(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -807,8 +792,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_UnsetWithInitialSetVolumeMou // Verifyies there is no valume as reported as attached func Test_RemoveVolumeFromReportAsAttached(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -840,8 +824,7 @@ func Test_RemoveVolumeFromReportAsAttached(t *testing.T) { // Verifyies there is one volume as reported as attached func Test_RemoveVolumeFromReportAsAttached_AddVolumeToReportAsAttached_Positive(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -883,8 +866,7 @@ func Test_RemoveVolumeFromReportAsAttached_AddVolumeToReportAsAttached_Positive( // Verifyies there is no volume as reported as attached func Test_RemoveVolumeFromReportAsAttached_Delete_AddVolumeNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -928,8 +910,7 @@ func Test_RemoveVolumeFromReportAsAttached_Delete_AddVolumeNode(t *testing.T) { // The elapsed time returned from the second SetDetachRequestTime call should be larger than maxWaitTime func Test_SetDetachRequestTime_Positive(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -960,8 +941,7 @@ func Test_SetDetachRequestTime_Positive(t *testing.T) { func Test_GetAttachedVolumesForNode_Positive_NoVolumesOrNodes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() node := types.NodeName("random") // Act @@ -975,8 +955,7 @@ func Test_GetAttachedVolumesForNode_Positive_NoVolumesOrNodes(t *testing.T) { func Test_GetAttachedVolumesForNode_Positive_OneVolumeOneNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") @@ -999,8 +978,7 @@ func Test_GetAttachedVolumesForNode_Positive_OneVolumeOneNode(t *testing.T) { func Test_GetAttachedVolumesForNode_Positive_TwoVolumeTwoNodes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) node1Name := types.NodeName("node1-name") @@ -1031,17 +1009,22 @@ func Test_GetAttachedVolumesForNode_Positive_TwoVolumeTwoNodes(t *testing.T) { func Test_GetAttachedVolumesForNode_Positive_OneVolumeTwoNodes(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) + realVolumeName, getNameErr := volumehelper.GetVolumeNameFromPluginMgr( + v1.UniqueVolumeName(""), volumeSpec, volumePluginMgr) + if getNameErr != nil { + t.Fatalf("GetVolumeNameFromPluginMgr failed. Expected: Actual: <%v>", getNameErr) + } node1Name := types.NodeName("node1-name") devicePath := "fake/device/path" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath) + generatedVolumeName1, add1Err := asw.AddVolumeNode(realVolumeName, volumeSpec, node1Name, devicePath) if add1Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add1Err) } node2Name := types.NodeName("node2-name") - generatedVolumeName2, add2Err := asw.AddVolumeNode(v1.UniqueVolumeName(""), volumeSpec, node2Name, devicePath) + generatedVolumeName2, add2Err := asw.AddVolumeNode(realVolumeName, volumeSpec, node2Name, devicePath) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -1067,18 +1050,23 @@ func Test_GetAttachedVolumesForNode_Positive_OneVolumeTwoNodes(t *testing.T) { func Test_OneVolumeTwoNodes_TwoDevicePaths(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) + realVolumeName, getNameErr := volumehelper.GetVolumeNameFromPluginMgr( + v1.UniqueVolumeName(""), volumeSpec, volumePluginMgr) + if getNameErr != nil { + t.Fatalf("GetVolumeNameFromPluginMgr failed. Expected: Actual: <%v>", getNameErr) + } node1Name := types.NodeName("node1-name") devicePath1 := "fake/device/path1" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath1) + generatedVolumeName1, add1Err := asw.AddVolumeNode(realVolumeName, volumeSpec, node1Name, devicePath1) if add1Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add1Err) } node2Name := types.NodeName("node2-name") devicePath2 := "fake/device/path2" - generatedVolumeName2, add2Err := asw.AddVolumeNode(v1.UniqueVolumeName(""), volumeSpec, node2Name, devicePath2) + generatedVolumeName2, add2Err := asw.AddVolumeNode(realVolumeName, volumeSpec, node2Name, devicePath2) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -1106,8 +1094,7 @@ func Test_OneVolumeTwoNodes_TwoDevicePaths(t *testing.T) { // does not exist in the actual state of the world func Test_SetNodeStatusUpdateNeededError(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - asw := NewActualStateOfWorld(volumePluginMgr) + asw := NewActualStateOfWorld() nodeName := types.NodeName("node-1") // Act @@ -1124,11 +1111,9 @@ func Test_SetNodeStatusUpdateNeededError(t *testing.T) { // updateNodeStatusUpdateNeeded is called on a node that exists in the actual state of the world func Test_updateNodeStatusUpdateNeeded(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) asw := &actualStateOfWorld{ attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume), nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor), - volumePluginMgr: volumePluginMgr, } nodeName := types.NodeName("node-1") nodeToUpdate := nodeToUpdateStatusFor{ @@ -1155,11 +1140,9 @@ func Test_updateNodeStatusUpdateNeeded(t *testing.T) { // updateNodeStatusUpdateNeeded is called on a node that does not exist in the actual state of the world func Test_updateNodeStatusUpdateNeededError(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) asw := &actualStateOfWorld{ attachedVolumes: make(map[v1.UniqueVolumeName]attachedVolume), nodesToUpdateStatusFor: make(map[types.NodeName]nodeToUpdateStatusFor), - volumePluginMgr: volumePluginMgr, } nodeName := types.NodeName("node-1") diff --git a/pkg/controller/volume/cache/desired_state_of_world.go b/pkg/controller/volume/cache/desired_state_of_world.go index 57ee9253ec1d0..b4c3ce857999a 100644 --- a/pkg/controller/volume/cache/desired_state_of_world.go +++ b/pkg/controller/volume/cache/desired_state_of_world.go @@ -30,7 +30,6 @@ import ( "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util/operationexecutor" "k8s.io/kubernetes/pkg/volume/util/types" - "k8s.io/kubernetes/pkg/volume/util/volumehelper" ) // DesiredStateOfWorld defines a set of thread-safe operations supported on @@ -60,7 +59,8 @@ type DesiredStateOfWorld interface { // should be attached to the specified node, the volume is implicitly added. // If no node with the name nodeName exists in list of nodes managed by the // attach/detach attached controller, an error is returned. - AddPod(podName types.UniquePodName, pod *v1.Pod, volumeSpec *volume.Spec, nodeName k8stypes.NodeName) (v1.UniqueVolumeName, error) + AddPod(podName types.UniquePodName, pod *v1.Pod, volumeSpec *volume.Spec, + nodeName k8stypes.NodeName, volumeName v1.UniqueVolumeName) (v1.UniqueVolumeName, error) // DeleteNode removes the given node from the list of nodes managed by the // attach/detach controller. @@ -126,10 +126,9 @@ type PodToAdd struct { } // NewDesiredStateOfWorld returns a new instance of DesiredStateOfWorld. -func NewDesiredStateOfWorld(volumePluginMgr *volume.VolumePluginMgr) DesiredStateOfWorld { +func NewDesiredStateOfWorld() DesiredStateOfWorld { return &desiredStateOfWorld{ - nodesManaged: make(map[k8stypes.NodeName]nodeManaged), - volumePluginMgr: volumePluginMgr, + nodesManaged: make(map[k8stypes.NodeName]nodeManaged), } } @@ -138,9 +137,6 @@ type desiredStateOfWorld struct { // detach controller. The key in this map is the name of the node and the // value is a node object containing more information about the node. nodesManaged map[k8stypes.NodeName]nodeManaged - // volumePluginMgr is the volume plugin manager used to create volume - // plugin objects. - volumePluginMgr *volume.VolumePluginMgr sync.RWMutex } @@ -208,7 +204,8 @@ func (dsw *desiredStateOfWorld) AddPod( podName types.UniquePodName, podToAdd *v1.Pod, volumeSpec *volume.Spec, - nodeName k8stypes.NodeName) (v1.UniqueVolumeName, error) { + nodeName k8stypes.NodeName, + volumeName v1.UniqueVolumeName) (v1.UniqueVolumeName, error) { dsw.Lock() defer dsw.Unlock() @@ -219,23 +216,6 @@ func (dsw *desiredStateOfWorld) AddPod( nodeName) } - attachableVolumePlugin, err := dsw.volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) - if err != nil || attachableVolumePlugin == nil { - return "", fmt.Errorf( - "failed to get AttachablePlugin from volumeSpec for volume %q err=%v", - volumeSpec.Name(), - err) - } - - volumeName, err := volumehelper.GetUniqueVolumeNameFromSpec( - attachableVolumePlugin, volumeSpec) - if err != nil { - return "", fmt.Errorf( - "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q err=%v", - volumeSpec.Name(), - err) - } - volumeObj, volumeExists := nodeObj.volumesToAttach[volumeName] if !volumeExists { volumeObj = volumeToAttach{ diff --git a/pkg/controller/volume/cache/desired_state_of_world_test.go b/pkg/controller/volume/cache/desired_state_of_world_test.go index 7d909e5d3d600..64445e45dff15 100644 --- a/pkg/controller/volume/cache/desired_state_of_world_test.go +++ b/pkg/controller/volume/cache/desired_state_of_world_test.go @@ -22,7 +22,6 @@ import ( "k8s.io/api/core/v1" k8stypes "k8s.io/apimachinery/pkg/types" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" - volumetesting "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util/types" ) @@ -30,8 +29,7 @@ import ( // Verifies node exists, and zero volumes to attach. func Test_AddNode_Positive_NewNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") // Act @@ -55,8 +53,7 @@ func Test_AddNode_Positive_NewNode(t *testing.T) { // Verifies node exists, and zero volumes to attach. func Test_AddNode_Positive_ExistingNode(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") // Act @@ -89,8 +86,7 @@ func Test_AddNode_Positive_ExistingNode(t *testing.T) { func Test_AddPod_Positive_NewPodNodeExistsVolumeDoesntExist(t *testing.T) { // Arrange podName := "pod-uid" - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := k8stypes.NodeName("node-name") @@ -104,7 +100,7 @@ func Test_AddPod_Positive_NewPodNodeExistsVolumeDoesntExist(t *testing.T) { } // Act - generatedVolumeName, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + generatedVolumeName, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) // Assert if podErr != nil { @@ -135,8 +131,7 @@ func Test_AddPod_Positive_NewPodNodeExistsVolumeDoesntExist(t *testing.T) { // Verifies the same node/volume exists, and 1 volumes to attach. func Test_AddPod_Positive_NewPodNodeExistsVolumeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() pod1Name := "pod1-uid" pod2Name := "pod2-uid" volumeName := v1.UniqueVolumeName("volume-name") @@ -152,7 +147,7 @@ func Test_AddPod_Positive_NewPodNodeExistsVolumeExists(t *testing.T) { } // Act - generatedVolumeName, podErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volumeSpec, nodeName) + generatedVolumeName, podErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volumeSpec, nodeName, volumeName) // Assert if podErr != nil { @@ -172,7 +167,7 @@ func Test_AddPod_Positive_NewPodNodeExistsVolumeExists(t *testing.T) { } // Act - generatedVolumeName, podErr = dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volumeSpec, nodeName) + generatedVolumeName, podErr = dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volumeSpec, nodeName, volumeName) // Assert if podErr != nil { @@ -209,8 +204,7 @@ func Test_AddPod_Positive_NewPodNodeExistsVolumeExists(t *testing.T) { // Verifies the same node/volume exists, and 1 volumes to attach. func Test_AddPod_Positive_PodExistsNodeExistsVolumeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() podName := "pod-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) @@ -225,7 +219,7 @@ func Test_AddPod_Positive_PodExistsNodeExistsVolumeExists(t *testing.T) { } // Act - generatedVolumeName, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + generatedVolumeName, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) // Assert if podErr != nil { @@ -245,7 +239,7 @@ func Test_AddPod_Positive_PodExistsNodeExistsVolumeExists(t *testing.T) { } // Act - generatedVolumeName, podErr = dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + generatedVolumeName, podErr = dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) // Assert if podErr != nil { @@ -275,8 +269,7 @@ func Test_AddPod_Positive_PodExistsNodeExistsVolumeExists(t *testing.T) { // Verifies call fails because node does not exist. func Test_AddPod_Negative_NewPodNodeDoesntExistVolumeDoesntExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() podName := "pod-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) @@ -290,7 +283,7 @@ func Test_AddPod_Negative_NewPodNodeDoesntExistVolumeDoesntExist(t *testing.T) { } // Act - _, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + _, podErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) // Assert if podErr == nil { @@ -316,8 +309,7 @@ func Test_AddPod_Negative_NewPodNodeDoesntExistVolumeDoesntExist(t *testing.T) { // Verifies node no longer exists, and zero volumes to attach. func Test_DeleteNode_Positive_NodeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) @@ -344,8 +336,7 @@ func Test_DeleteNode_Positive_NodeExists(t *testing.T) { // Verifies no error is returned, and zero volumes to attach. func Test_DeleteNode_Positive_NodeDoesntExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() notAddedNodeName := k8stypes.NodeName("node-not-added-name") // Act @@ -372,14 +363,13 @@ func Test_DeleteNode_Positive_NodeDoesntExist(t *testing.T) { // Verifies call fails because node still contains child volumes. func Test_DeleteNode_Negative_NodeExistsHasChildVolumes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) podName := "pod-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -413,14 +403,13 @@ func Test_DeleteNode_Negative_NodeExistsHasChildVolumes(t *testing.T) { // Verifies volume no longer exists, and zero volumes to attach. func Test_DeletePod_Positive_PodExistsNodeExistsVolumeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() podName := "pod-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -460,22 +449,21 @@ func Test_DeletePod_Positive_PodExistsNodeExistsVolumeExists(t *testing.T) { // Verifies volume still exists, and one volumes to attach. func Test_DeletePod_Positive_2PodsExistNodeExistsVolumesExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() pod1Name := "pod1-uid" pod2Name := "pod2-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) - generatedVolumeName1, pod1AddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volumeSpec, nodeName) + generatedVolumeName1, pod1AddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volumeSpec, nodeName, volumeName) if pod1AddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", pod1Name, pod1AddErr) } - generatedVolumeName2, pod2AddErr := dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volumeSpec, nodeName) + generatedVolumeName2, pod2AddErr := dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volumeSpec, nodeName, volumeName) if pod2AddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -521,15 +509,14 @@ func Test_DeletePod_Positive_2PodsExistNodeExistsVolumesExist(t *testing.T) { // Verifies volume still exists, and one volumes to attach. func Test_DeletePod_Positive_PodDoesNotExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() pod1Name := "pod1-uid" pod2Name := "pod2-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) - generatedVolumeName, pod1AddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volumeSpec, nodeName) + generatedVolumeName, pod1AddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volumeSpec, nodeName, volumeName) if pod1AddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -570,14 +557,13 @@ func Test_DeletePod_Positive_PodDoesNotExist(t *testing.T) { // Verifies volume still exists, and one volumes to attach. func Test_DeletePod_Positive_NodeDoesNotExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() podName := "pod-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := k8stypes.NodeName("node1-name") dsw.AddNode(node1Name, false /*keepTerminatedPodVolumes*/) - generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, node1Name) + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, node1Name, volumeName) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -625,14 +611,13 @@ func Test_DeletePod_Positive_NodeDoesNotExist(t *testing.T) { // Verifies volume still exists, and one volumes to attach. func Test_DeletePod_Positive_VolumeDoesNotExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() podName := "pod-uid" volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) - generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volume1Spec, nodeName) + generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volume1Spec, nodeName, volume1Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -679,8 +664,7 @@ func Test_DeletePod_Positive_VolumeDoesNotExist(t *testing.T) { // Verifies node does not exist, and no volumes to attach. func Test_NodeExists_Positive_NodeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() notAddedNodeName := k8stypes.NodeName("node-not-added-name") // Act @@ -702,8 +686,7 @@ func Test_NodeExists_Positive_NodeExists(t *testing.T) { // Verifies node exists, and no volumes to attach. func Test_NodeExists_Positive_NodeDoesntExist(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) @@ -726,14 +709,13 @@ func Test_NodeExists_Positive_NodeDoesntExist(t *testing.T) { // Verifies volume/node exists, and one volume to attach. func Test_VolumeExists_Positive_VolumeExistsNodeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) podName := "pod-uid" volumeName := v1.UniqueVolumeName("volume-name") volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) - generatedVolumeName, _ := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName) + generatedVolumeName, _ := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volumeSpec, nodeName, volumeName) // Act volumeExists := dsw.VolumeExists(generatedVolumeName, nodeName) @@ -756,14 +738,13 @@ func Test_VolumeExists_Positive_VolumeExistsNodeExists(t *testing.T) { // Verifies volume2/node does not exist, and one volume to attach. func Test_VolumeExists_Positive_VolumeDoesntExistNodeExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") dsw.AddNode(nodeName, false /*keepTerminatedPodVolumes*/) podName := "pod-uid" volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) - generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volume1Spec, nodeName) + generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(podName), controllervolumetesting.NewPod(podName, podName), volume1Spec, nodeName, volume1Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -792,8 +773,7 @@ func Test_VolumeExists_Positive_VolumeDoesntExistNodeExists(t *testing.T) { // Verifies volume/node do not exist, and zero volumes to attach. func Test_VolumeExists_Positive_VolumeDoesntExistNodeDoesntExists(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() nodeName := k8stypes.NodeName("node-name") volumeName := v1.UniqueVolumeName("volume-name") @@ -815,8 +795,7 @@ func Test_VolumeExists_Positive_VolumeDoesntExistNodeDoesntExists(t *testing.T) // Verifies zero volumes to attach. func Test_GetVolumesToAttach_Positive_NoNodes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() // Act volumesToAttach := dsw.GetVolumesToAttach() @@ -832,8 +811,7 @@ func Test_GetVolumesToAttach_Positive_NoNodes(t *testing.T) { // Verifies zero volumes to attach. func Test_GetVolumesToAttach_Positive_TwoNodes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() node1Name := k8stypes.NodeName("node1-name") node2Name := k8stypes.NodeName("node2-name") dsw.AddNode(node1Name, false /*keepTerminatedPodVolumes*/) @@ -853,14 +831,13 @@ func Test_GetVolumesToAttach_Positive_TwoNodes(t *testing.T) { // Verifies two volumes to attach. func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEach(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() node1Name := k8stypes.NodeName("node1-name") pod1Name := "pod1-uid" volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) dsw.AddNode(node1Name, false /*keepTerminatedPodVolumes*/) - generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volume1Spec, node1Name) + generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volume1Spec, node1Name, volume1Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -872,7 +849,7 @@ func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEach(t *testing.T) { volume2Name := v1.UniqueVolumeName("volume2-name") volume2Spec := controllervolumetesting.GetTestVolumeSpec(string(volume2Name), volume2Name) dsw.AddNode(node2Name, false /*keepTerminatedPodVolumes*/) - generatedVolume2Name, podAddErr := dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volume2Spec, node2Name) + generatedVolume2Name, podAddErr := dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volume2Spec, node2Name, volume2Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -898,14 +875,13 @@ func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEach(t *testing.T) { // Verifies two volumes to attach. func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEachExtraPod(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() node1Name := k8stypes.NodeName("node1-name") pod1Name := "pod1-uid" volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) dsw.AddNode(node1Name, false /*keepTerminatedPodVolumes*/) - generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volume1Spec, node1Name) + generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volume1Spec, node1Name, volume1Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -917,7 +893,7 @@ func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEachExtraPod(t *testing.T volume2Name := v1.UniqueVolumeName("volume2-name") volume2Spec := controllervolumetesting.GetTestVolumeSpec(string(volume2Name), volume2Name) dsw.AddNode(node2Name, false /*keepTerminatedPodVolumes*/) - generatedVolume2Name, podAddErr := dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volume2Spec, node2Name) + generatedVolume2Name, podAddErr := dsw.AddPod(types.UniquePodName(pod2Name), controllervolumetesting.NewPod(pod2Name, pod2Name), volume2Spec, node2Name, volume2Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -925,8 +901,8 @@ func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEachExtraPod(t *testing.T podAddErr) } pod3Name := "pod3-uid" - dsw.AddPod(types.UniquePodName(pod3Name), controllervolumetesting.NewPod(pod3Name, pod3Name), volume2Spec, node2Name) - _, podAddErr = dsw.AddPod(types.UniquePodName(pod3Name), controllervolumetesting.NewPod(pod3Name, pod3Name), volume2Spec, node2Name) + dsw.AddPod(types.UniquePodName(pod3Name), controllervolumetesting.NewPod(pod3Name, pod3Name), volume2Spec, node2Name, volume2Name) + _, podAddErr = dsw.AddPod(types.UniquePodName(pod3Name), controllervolumetesting.NewPod(pod3Name, pod3Name), volume2Spec, node2Name, volume2Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -952,14 +928,13 @@ func Test_GetVolumesToAttach_Positive_TwoNodesOneVolumeEachExtraPod(t *testing.T // Verifies three volumes to attach. func Test_GetVolumesToAttach_Positive_TwoNodesThreeVolumes(t *testing.T) { // Arrange - volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) - dsw := NewDesiredStateOfWorld(volumePluginMgr) + dsw := NewDesiredStateOfWorld() node1Name := k8stypes.NodeName("node1-name") pod1Name := "pod1-uid" volume1Name := v1.UniqueVolumeName("volume1-name") volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) dsw.AddNode(node1Name, false /*keepTerminatedPodVolumes*/) - generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volume1Spec, node1Name) + generatedVolume1Name, podAddErr := dsw.AddPod(types.UniquePodName(pod1Name), controllervolumetesting.NewPod(pod1Name, pod1Name), volume1Spec, node1Name, volume1Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -971,7 +946,7 @@ func Test_GetVolumesToAttach_Positive_TwoNodesThreeVolumes(t *testing.T) { volume2Name := v1.UniqueVolumeName("volume2-name") volume2Spec := controllervolumetesting.GetTestVolumeSpec(string(volume2Name), volume2Name) dsw.AddNode(node2Name, false /*keepTerminatedPodVolumes*/) - generatedVolume2Name1, podAddErr := dsw.AddPod(types.UniquePodName(pod2aName), controllervolumetesting.NewPod(pod2aName, pod2aName), volume2Spec, node2Name) + generatedVolume2Name1, podAddErr := dsw.AddPod(types.UniquePodName(pod2aName), controllervolumetesting.NewPod(pod2aName, pod2aName), volume2Spec, node2Name, volume2Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -979,7 +954,7 @@ func Test_GetVolumesToAttach_Positive_TwoNodesThreeVolumes(t *testing.T) { podAddErr) } pod2bName := "pod2b-name" - generatedVolume2Name2, podAddErr := dsw.AddPod(types.UniquePodName(pod2bName), controllervolumetesting.NewPod(pod2bName, pod2bName), volume2Spec, node2Name) + generatedVolume2Name2, podAddErr := dsw.AddPod(types.UniquePodName(pod2bName), controllervolumetesting.NewPod(pod2bName, pod2bName), volume2Spec, node2Name, volume2Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", @@ -995,7 +970,7 @@ func Test_GetVolumesToAttach_Positive_TwoNodesThreeVolumes(t *testing.T) { pod3Name := "pod3-uid" volume3Name := v1.UniqueVolumeName("volume3-name") volume3Spec := controllervolumetesting.GetTestVolumeSpec(string(volume3Name), volume3Name) - generatedVolume3Name, podAddErr := dsw.AddPod(types.UniquePodName(pod3Name), controllervolumetesting.NewPod(pod3Name, pod3Name), volume3Spec, node1Name) + generatedVolume3Name, podAddErr := dsw.AddPod(types.UniquePodName(pod3Name), controllervolumetesting.NewPod(pod3Name, pod3Name), volume3Spec, node1Name, volume3Name) if podAddErr != nil { t.Fatalf( "AddPod failed for pod %q. Expected: Actual: <%v>", diff --git a/pkg/controller/volume/util/util.go b/pkg/controller/volume/util/util.go index 55f65ebf70513..1db7b2cb9a5c8 100644 --- a/pkg/controller/volume/util/util.go +++ b/pkg/controller/volume/util/util.go @@ -216,11 +216,23 @@ func ProcessPodVolumes(pod *v1.Pod, addVolumes bool, desiredStateOfWorld cache.D continue } + uniqueVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec( + attachableVolumePlugin, volumeSpec) + if err != nil { + glog.V(10).Infof( + "Skipping volume %q for pod %q/%q: GetUniqueVolumeNameFromSpec failed with %v", + podVolume.Name, + pod.Namespace, + pod.Name, + err) + continue + } + uniquePodName := volumehelper.GetUniquePodName(pod) if addVolumes { // Add volume to desired state of world _, err := desiredStateOfWorld.AddPod( - uniquePodName, pod, volumeSpec, nodeName) + uniquePodName, pod, volumeSpec, nodeName, uniqueVolumeName) if err != nil { glog.V(10).Infof( "Failed to add volume %q for pod %q/%q to desiredStateOfWorld. %v", @@ -232,17 +244,6 @@ func ProcessPodVolumes(pod *v1.Pod, addVolumes bool, desiredStateOfWorld cache.D } else { // Remove volume from desired state of world - uniqueVolumeName, err := volumehelper.GetUniqueVolumeNameFromSpec( - attachableVolumePlugin, volumeSpec) - if err != nil { - glog.V(10).Infof( - "Failed to delete volume %q for pod %q/%q from desiredStateOfWorld. GetUniqueVolumeNameFromSpec failed with %v", - podVolume.Name, - pod.Namespace, - pod.Name, - err) - continue - } desiredStateOfWorld.DeletePod( uniquePodName, uniqueVolumeName, nodeName) } diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 2353e12615feb..2ab2fbcfd4fc1 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -310,8 +310,14 @@ func (og *operationGenerator) GenerateAttachVolumeFunc( if attachErr != nil { if derr, ok := attachErr.(*util.DanglingAttachError); ok { + volumeName, getVolumeNameErr := volumehelper.GetVolumeNameFromPluginMgr( + v1.UniqueVolumeName(""), volumeToAttach.VolumeSpec, og.volumePluginMgr) + if getVolumeNameErr != nil { + return volumeToAttach.GenerateError("AttachVolume.GetVolumeNameFromPluginMgr failed", getVolumeNameErr) + } + addErr := actualStateOfWorld.MarkVolumeAsAttached( - v1.UniqueVolumeName(""), + volumeName, volumeToAttach.VolumeSpec, derr.CurrentNode, derr.DevicePath) @@ -333,8 +339,13 @@ func (og *operationGenerator) GenerateAttachVolumeFunc( glog.Infof(volumeToAttach.GenerateMsgDetailed("AttachVolume.Attach succeeded", "")) // Update actual state of world + volumeName, getVolumeNameErr := volumehelper.GetVolumeNameFromPluginMgr( + v1.UniqueVolumeName(""), volumeToAttach.VolumeSpec, og.volumePluginMgr) + if getVolumeNameErr != nil { + return volumeToAttach.GenerateError("AttachVolume.GetVolumeNameFromPluginMgr failed", getVolumeNameErr) + } addVolumeNodeErr := actualStateOfWorld.MarkVolumeAsAttached( - v1.UniqueVolumeName(""), volumeToAttach.VolumeSpec, volumeToAttach.NodeName, devicePath) + volumeName, volumeToAttach.VolumeSpec, volumeToAttach.NodeName, devicePath) if addVolumeNodeErr != nil { // On failure, return error. Caller will log and retry. return volumeToAttach.GenerateError("AttachVolume.MarkVolumeAsAttached failed", addVolumeNodeErr) diff --git a/pkg/volume/util/volumehelper/volumehelper.go b/pkg/volume/util/volumehelper/volumehelper.go index 74b14be5de808..131d68a6a7a91 100644 --- a/pkg/volume/util/volumehelper/volumehelper.go +++ b/pkg/volume/util/volumehelper/volumehelper.go @@ -157,3 +157,36 @@ func GetPersistentVolumeClaimVolumeMode(claim *v1.PersistentVolumeClaim) (v1.Per } return "", fmt.Errorf("cannot get volumeMode from pvc: %v", claim.Name) } + +func GetVolumeNameFromPluginMgr( + uniqueName v1.UniqueVolumeName, + volumeSpec *volume.Spec, + volumePluginMgr *volume.VolumePluginMgr) (v1.UniqueVolumeName, error) { + var volumeName v1.UniqueVolumeName + if volumeSpec != nil { + attachableVolumePlugin, err := volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) + if err != nil || attachableVolumePlugin == nil { + return "", fmt.Errorf( + "failed to get AttachablePlugin from volumeSpec for volume %q err=%v", + volumeSpec.Name(), + err) + } + + volumeName, err = GetUniqueVolumeNameFromSpec( + attachableVolumePlugin, volumeSpec) + if err != nil { + return "", fmt.Errorf( + "failed to GetUniqueVolumeNameFromSpec for volumeSpec %q err=%v", + volumeSpec.Name(), + err) + } + } else { + // volumeSpec is nil + // This happens only on controller startup when reading the volumes from node + // status; if the pods using the volume have been removed and are unreachable + // the volumes should be detached immediately and the spec is not needed + volumeName = uniqueName + } + + return volumeName, nil +}