From 562d0fea53ded469368b4d2a0bdfd0e33ece1037 Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Mon, 8 Oct 2018 14:37:11 -0700 Subject: [PATCH] Handle failed attach operation leave uncertain volume attach state This commit adds the unit tests for the PR. It also includes some files that are affected by the function name changes. --- .../attachdetach/attach_detach_controller.go | 4 +- .../attach_detach_controller_test.go | 4 +- .../volume/attachdetach/cache/BUILD | 1 + .../cache/actual_state_of_world.go | 8 +- .../cache/actual_state_of_world_test.go | 380 ++++++++++++++---- .../volume/attachdetach/metrics/metrics.go | 2 +- .../attachdetach/metrics/metrics_test.go | 2 +- .../attachdetach/reconciler/reconciler.go | 9 +- .../reconciler/reconciler_test.go | 212 +++++++++- .../attachdetach/testing/testvolumespec.go | 2 +- .../cache/actual_state_of_world.go | 5 + pkg/volume/testing/testing.go | 81 +++- 12 files changed, 598 insertions(+), 112 deletions(-) diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index f76e3224a6e..7a8fdb99b5e 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -435,7 +435,7 @@ func (adc *attachDetachController) populateDesiredStateOfWorld() error { err) continue } - if adc.actualStateOfWorld.VolumeNodeExists(volumeName, nodeName) { + if adc.actualStateOfWorld.IsVolumeAttachedToNode(volumeName, nodeName) { devicePath, err := adc.getNodeVolumeDevicePath(volumeName, nodeName) if err != nil { klog.Errorf("Failed to find device path: %v", err) @@ -632,7 +632,7 @@ func (adc *attachDetachController) syncPVCByKey(key string) error { func (adc *attachDetachController) processVolumesInUse( nodeName types.NodeName, volumesInUse []v1.UniqueVolumeName) { klog.V(4).Infof("processVolumesInUse for node %q", nodeName) - for _, attachedVolume := range adc.actualStateOfWorld.GetAttachedVolumesForNode(nodeName) { + for _, attachedVolume := range adc.actualStateOfWorld.GetAllVolumesForNode(nodeName) { mounted := false for _, volumeInUse := range volumesInUse { if attachedVolume.VolumeName == volumeInUse { diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go index f8503025217..58894ddde8a 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go @@ -110,7 +110,7 @@ func Test_AttachDetachControllerStateOfWolrdPopulators_Positive(t *testing.T) { for _, node := range nodes { nodeName := types.NodeName(node.Name) for _, attachedVolume := range node.Status.VolumesAttached { - found := adc.actualStateOfWorld.VolumeNodeExists(attachedVolume.Name, nodeName) + found := adc.actualStateOfWorld.IsVolumeAttachedToNode(attachedVolume.Name, nodeName) if !found { t.Fatalf("Run failed with error. Node %s, volume %s not found", nodeName, attachedVolume.Name) } @@ -278,7 +278,7 @@ func attachDetachRecoveryTestCase(t *testing.T, extraPods1 []*v1.Pod, extraPods2 var detachedVolumesNum int = 0 time.Sleep(time.Second * 1) // Wait for a second - for _, volumeList := range testPlugin.GetAttachedVolumes() { + for _, volumeList := range testPlugin.GetAllVolumes() { attachedVolumesNum += len(volumeList) } for _, volumeList := range testPlugin.GetDetachedVolumes() { diff --git a/pkg/controller/volume/attachdetach/cache/BUILD b/pkg/controller/volume/attachdetach/cache/BUILD index a2890223e0a..9b86e8711ff 100644 --- a/pkg/controller/volume/attachdetach/cache/BUILD +++ b/pkg/controller/volume/attachdetach/cache/BUILD @@ -34,6 +34,7 @@ go_test( deps = [ "//pkg/controller/volume/attachdetach/testing:go_default_library", "//pkg/volume/testing:go_default_library", + "//pkg/volume/util:go_default_library", "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go index 33f0a9fd342..11df3a4c8fa 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go +++ b/pkg/controller/volume/attachdetach/cache/actual_state_of_world.go @@ -47,8 +47,10 @@ type ActualStateOfWorld interface { // operationexecutor to interact with it. operationexecutor.ActualStateOfWorldAttacherUpdater - // AddVolumeNode adds the given volume and node to the underlying store - // indicating the specified volume is attached to the specified node. + // AddVolumeNode adds the given volume and node to the underlying store. + // If attached is set to true, it indicates the specified volume is already + // attached to the specified node. If attached set to false, it means that + // the volume is not confirmed to be attached to the node yet. // A unique volume name is generated from the volumeSpec and returned on // success. // If volumeSpec is not an attachable volume plugin, an error is returned. @@ -332,7 +334,7 @@ func (asw *actualStateOfWorld) AddVolumeNode( nodeName) } volumeObj.nodesAttachedTo[nodeName] = node - + if isAttached { asw.addVolumeToReportAsAttached(volumeName, nodeName) } diff --git a/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go b/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go index 14a531ba103..ab45693ec23 100644 --- a/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go +++ b/pkg/controller/volume/attachdetach/cache/actual_state_of_world_test.go @@ -24,9 +24,10 @@ import ( "k8s.io/apimachinery/pkg/types" controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing" volumetesting "k8s.io/kubernetes/pkg/volume/testing" + volumeutil "k8s.io/kubernetes/pkg/volume/util" ) -// Calls AddVolumeNode() once. +// Calls AddVolumeNode() once with attached set to true. // Verifies a single volume/node entry exists. func Test_AddVolumeNode_Positive_NewVolumeNewNode(t *testing.T) { // Arrange @@ -39,19 +40,19 @@ func Test_AddVolumeNode_Positive_NewVolumeNewNode(t *testing.T) { devicePath := "fake/device/path" // Act - generatedVolumeName, err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) // Assert if err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", err) } - volumeNodeComboExists := asw.VolumeNodeExists(generatedVolumeName, nodeName) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName) if !volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName, nodeName) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -59,6 +60,183 @@ func Test_AddVolumeNode_Positive_NewVolumeNewNode(t *testing.T) { verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) } +// Calls AddVolumeNode() once with attached set to false. +// Verifies a single volume/node entry exists. +// Then calls AddVolumeNode() with attached set to true +// Verifies volume is attached to the node according to asw. +func Test_AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached(t *testing.T) { + // Arrange + volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) + asw := NewActualStateOfWorld(volumePluginMgr) + volumeName := v1.UniqueVolumeName("volume-name") + volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) + + nodeName := types.NodeName("node-name") + devicePath := "fake/device/path" + + // Act + generatedVolumeName, err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, false) + + // Assert + if err != nil { + t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", err) + } + + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName) + if volumeNodeComboExists { + t.Fatalf("%q/%q volume/node combo does exist, it should not.", generatedVolumeName, nodeName) + } + + allVolumes := asw.GetAllVolumes() + if len(allVolumes) != 1 { + t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(allVolumes)) + } + verifyAttachedVolume(t, allVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + + reportAsAttachedVolumesMap := asw.GetVolumesToReportAttached() + _, exists := reportAsAttachedVolumesMap[nodeName] + if exists { + t.Fatalf("AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached failed. Actual: Expect: Actual: <%v>", len(volumesForNode)) + } + verifyAttachedVolume(t, volumesForNode, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + + attachedVolumesMap := asw.GetAttachedVolumesPerNode() + _, exists = attachedVolumesMap[nodeName] + if exists { + t.Fatalf("AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached failed. Actual: Expect: 0 { + t.Fatalf("AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached failed. Expect no nodes returned.") + } + + // Add the volume to the node second time with attached set to true + generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) + + // Assert + if add2Err != nil { + t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) + } + + if generatedVolumeName != generatedVolumeName2 { + t.Fatalf( + "Generated volume names for the same volume should be the same but they are not: %q and %q", + generatedVolumeName, + generatedVolumeName2) + } + + volumeNodeComboExists = asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName) + if !volumeNodeComboExists { + t.Fatalf("%q/%q combo does not exist, it should.", generatedVolumeName, nodeName) + } + + attachedVolumes := asw.GetAllVolumes() + if len(attachedVolumes) != 1 { + t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) + } + + verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + + nodes = asw.GetNodesForVolume(volumeName) + if len(nodes) != 1 { + t.Fatalf("AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached failed. Expect one node returned.") + } + if nodes[0] != nodeName { + t.Fatalf("AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached failed. Expect node %v, Actual node %v", nodeName, nodes[0]) + } + + attachedVolumesMap = asw.GetAttachedVolumesPerNode() + _, exists = attachedVolumesMap[nodeName] + if !exists { + t.Fatalf("AddVolumeNode_Positive_NewVolumeNewNodeWithFalseAttached failed. Actual: Expect: Actual: <%v>", err) + } + + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, node1Name) + if volumeNodeComboExists { + t.Fatalf("%q/%q volume/node combo does exist, it should not.", generatedVolumeName, node1Name) + } + + generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, node2Name, devicePath, true) + + // Assert + if add2Err != nil { + t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) + } + + if generatedVolumeName != generatedVolumeName2 { + t.Fatalf( + "Generated volume names for the same volume should be the same but they are not: %q and %q", + generatedVolumeName, + generatedVolumeName2) + } + + volumeNodeComboExists = asw.IsVolumeAttachedToNode(generatedVolumeName, node2Name) + if !volumeNodeComboExists { + t.Fatalf("%q/%q combo does not exist, it should.", generatedVolumeName, node2Name) + } + + attachedVolumes := asw.GetAllVolumes() + if len(attachedVolumes) != 2 { + t.Fatalf("len(attachedVolumes) Expected: <2> Actual: <%v>", len(attachedVolumes)) + } + verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), node1Name, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), node2Name, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + + volumesForNode := asw.GetAllVolumesForNode(node2Name) + if len(volumesForNode) != 1 { + t.Fatalf("len(attachedVolumes) Expected: <2> Actual: <%v>", len(volumesForNode)) + } + verifyAttachedVolume(t, volumesForNode, generatedVolumeName, string(volumeName), node2Name, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) + + attachedVolumesMap := asw.GetAttachedVolumesPerNode() + attachedVolumesPerNode, exists := attachedVolumesMap[node2Name] + if !exists || len(attachedVolumesPerNode) != 1 { + t.Fatalf("AddVolumeNode_Positive_NewVolumeTwoNodesWithFalseAttached failed. Actual: Expect: Expect: Actual: <%v>", len(attachedVolumes)) } @@ -121,8 +299,8 @@ func Test_AddVolumeNode_Positive_ExistingVolumeExistingNode(t *testing.T) { devicePath := "fake/device/path" // Act - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) - generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) + generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) // Assert if add1Err != nil { @@ -139,12 +317,12 @@ func Test_AddVolumeNode_Positive_ExistingVolumeExistingNode(t *testing.T) { generatedVolumeName2) } - volumeNodeComboExists := asw.VolumeNodeExists(generatedVolumeName1, nodeName) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName1, nodeName) if !volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName1, nodeName) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -163,7 +341,7 @@ func Test_DeleteVolumeNode_Positive_VolumeExistsNodeExists(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -172,12 +350,12 @@ func Test_DeleteVolumeNode_Positive_VolumeExistsNodeExists(t *testing.T) { asw.DeleteVolumeNode(generatedVolumeName, nodeName) // Assert - volumeNodeComboExists := asw.VolumeNodeExists(generatedVolumeName, nodeName) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName) if volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo exists, it should not.", generatedVolumeName, nodeName) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 0 { t.Fatalf("len(attachedVolumes) Expected: <0> Actual: <%v>", len(attachedVolumes)) } @@ -196,12 +374,12 @@ func Test_DeleteVolumeNode_Positive_VolumeDoesntExistNodeDoesntExist(t *testing. asw.DeleteVolumeNode(volumeName, nodeName) // Assert - volumeNodeComboExists := asw.VolumeNodeExists(volumeName, nodeName) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(volumeName, nodeName) if volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo exists, it should not.", volumeName, nodeName) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 0 { t.Fatalf("len(attachedVolumes) Expected: <0> Actual: <%v>", len(attachedVolumes)) } @@ -220,11 +398,11 @@ func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) { node1Name := types.NodeName("node1-name") node2Name := types.NodeName("node2-name") devicePath := "fake/device/path" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath) + generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath, true) if add1Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add1Err) } - generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, node2Name, devicePath) + generatedVolumeName2, add2Err := asw.AddVolumeNode(volumeName, volumeSpec, node2Name, devicePath, true) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -239,17 +417,17 @@ func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) { asw.DeleteVolumeNode(generatedVolumeName1, node1Name) // Assert - volumeNodeComboExists := asw.VolumeNodeExists(generatedVolumeName1, node1Name) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName1, node1Name) if volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo exists, it should not.", generatedVolumeName1, node1Name) } - volumeNodeComboExists = asw.VolumeNodeExists(generatedVolumeName1, node2Name) + volumeNodeComboExists = asw.IsVolumeAttachedToNode(generatedVolumeName1, node2Name) if !volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName1, node2Name) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -258,7 +436,7 @@ func Test_DeleteVolumeNode_Positive_TwoNodesOneDeleted(t *testing.T) { } // Populates data struct with one volume/node entry. -// Calls VolumeNodeExists() to verify entry. +// Calls IsVolumeAttachedToNode() to verify entry. // Verifies the populated volume/node entry exists. func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) { // Arrange @@ -268,20 +446,20 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } // Act - volumeNodeComboExists := asw.VolumeNodeExists(generatedVolumeName, nodeName) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, nodeName) // Assert if !volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo does not exist, it should.", generatedVolumeName, nodeName) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -290,7 +468,7 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeExists(t *testing.T) { } // Populates data struct with one volume1/node1 entry. -// Calls VolumeNodeExists() with volume1/node2. +// Calls IsVolumeAttachedToNode() with volume1/node2. // Verifies requested entry does not exist, but populated entry does. func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) { // Arrange @@ -301,20 +479,20 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) { node1Name := types.NodeName("node1-name") node2Name := types.NodeName("node2-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } // Act - volumeNodeComboExists := asw.VolumeNodeExists(generatedVolumeName, node2Name) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(generatedVolumeName, node2Name) // Assert if volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo exists, it should not.", generatedVolumeName, node2Name) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -322,7 +500,7 @@ func Test_VolumeNodeExists_Positive_VolumeExistsNodeDoesntExist(t *testing.T) { verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), node1Name, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) } -// Calls VolumeNodeExists() on empty data struct. +// Calls IsVolumeAttachedToNode() on empty data struct. // Verifies requested entry does not exist. func Test_VolumeNodeExists_Positive_VolumeAndNodeDontExist(t *testing.T) { // Arrange @@ -332,20 +510,20 @@ func Test_VolumeNodeExists_Positive_VolumeAndNodeDontExist(t *testing.T) { nodeName := types.NodeName("node-name") // Act - volumeNodeComboExists := asw.VolumeNodeExists(volumeName, nodeName) + volumeNodeComboExists := asw.IsVolumeAttachedToNode(volumeName, nodeName) // Assert if volumeNodeComboExists { t.Fatalf("%q/%q volume/node combo exists, it should not.", volumeName, nodeName) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 0 { t.Fatalf("len(attachedVolumes) Expected: <0> Actual: <%v>", len(attachedVolumes)) } } -// Calls GetAttachedVolumes() on empty data struct. +// Calls GetAllVolumes() on empty data struct. // Verifies no volume/node entries are returned. func Test_GetAttachedVolumes_Positive_NoVolumesOrNodes(t *testing.T) { // Arrange @@ -353,7 +531,7 @@ func Test_GetAttachedVolumes_Positive_NoVolumesOrNodes(t *testing.T) { asw := NewActualStateOfWorld(volumePluginMgr) // Act - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() // Assert if len(attachedVolumes) != 0 { @@ -362,7 +540,7 @@ func Test_GetAttachedVolumes_Positive_NoVolumesOrNodes(t *testing.T) { } // Populates data struct with one volume/node entry. -// Calls GetAttachedVolumes() to get list of entries. +// Calls GetAllVolumes() to get list of entries. // Verifies one volume/node entry is returned. func Test_GetAttachedVolumes_Positive_OneVolumeOneNode(t *testing.T) { // Arrange @@ -372,13 +550,13 @@ func Test_GetAttachedVolumes_Positive_OneVolumeOneNode(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } // Act - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() // Assert if len(attachedVolumes) != 1 { @@ -389,7 +567,7 @@ func Test_GetAttachedVolumes_Positive_OneVolumeOneNode(t *testing.T) { } // Populates data struct with two volume/node entries (different node and volume). -// Calls GetAttachedVolumes() to get list of entries. +// Calls GetAllVolumes() to get list of entries. // Verifies both volume/node entries are returned. func Test_GetAttachedVolumes_Positive_TwoVolumeTwoNodes(t *testing.T) { // Arrange @@ -399,20 +577,20 @@ func Test_GetAttachedVolumes_Positive_TwoVolumeTwoNodes(t *testing.T) { volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) node1Name := types.NodeName("node1-name") devicePath := "fake/device/path" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volume1Name, volume1Spec, node1Name, devicePath) + generatedVolumeName1, add1Err := asw.AddVolumeNode(volume1Name, volume1Spec, node1Name, devicePath, true) if add1Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add1Err) } volume2Name := v1.UniqueVolumeName("volume2-name") volume2Spec := controllervolumetesting.GetTestVolumeSpec(string(volume2Name), volume2Name) node2Name := types.NodeName("node2-name") - generatedVolumeName2, add2Err := asw.AddVolumeNode(volume2Name, volume2Spec, node2Name, devicePath) + generatedVolumeName2, add2Err := asw.AddVolumeNode(volume2Name, volume2Spec, node2Name, devicePath, true) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } // Act - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() // Assert if len(attachedVolumes) != 2 { @@ -424,7 +602,7 @@ func Test_GetAttachedVolumes_Positive_TwoVolumeTwoNodes(t *testing.T) { } // Populates data struct with two volume/node entries (same volume different node). -// Calls GetAttachedVolumes() to get list of entries. +// Calls GetAllVolumes() to get list of entries. // Verifies both volume/node entries are returned. func Test_GetAttachedVolumes_Positive_OneVolumeTwoNodes(t *testing.T) { // Arrange @@ -434,12 +612,20 @@ func Test_GetAttachedVolumes_Positive_OneVolumeTwoNodes(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := types.NodeName("node1-name") devicePath := "fake/device/path" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath) + plugin, err := volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) + if err != nil || plugin == nil { + t.Fatalf("Failed to get volume plugin from spec %v, %v", volumeSpec, err) + } + uniqueVolumeName, err := volumeutil.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) + if err != nil || plugin == nil { + t.Fatalf("Failed to get uniqueVolumeName from spec %v, %v", volumeSpec, err) + } + generatedVolumeName1, add1Err := asw.AddVolumeNode(uniqueVolumeName, volumeSpec, node1Name, devicePath, true) 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(v1.UniqueVolumeName(""), volumeSpec, node2Name, devicePath, true) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -452,7 +638,7 @@ func Test_GetAttachedVolumes_Positive_OneVolumeTwoNodes(t *testing.T) { } // Act - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() // Assert if len(attachedVolumes) != 2 { @@ -473,7 +659,7 @@ func Test_SetVolumeMountedByNode_Positive_Set(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -481,7 +667,7 @@ func Test_SetVolumeMountedByNode_Positive_Set(t *testing.T) { // Act: do not mark -- test default value // Assert - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -500,7 +686,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -517,7 +703,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSet(t *testing.T) { t.Fatalf("SetVolumeMountedByNode2 failed. Expected Actual: <%v>", setVolumeMountedErr2) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -536,12 +722,12 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -556,7 +742,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithoutInitialSet(t *testing.T) { t.Fatalf("SetVolumeMountedByNode failed. Expected Actual: <%v>", setVolumeMountedErr) } - attachedVolumes = asw.GetAttachedVolumes() + attachedVolumes = asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -576,7 +762,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotRes volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -584,7 +770,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotRes // Act setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) setVolumeMountedErr2 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, false /* mounted */) - generatedVolumeName, addErr = asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr = asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) // Assert if setVolumeMountedErr1 != nil { @@ -597,7 +783,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetAddVolumeNodeNotRes t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -617,7 +803,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequest volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -629,7 +815,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequest if err != nil { t.Fatalf("RemoveVolumeFromReportAsAttached failed. Expected: Actual: <%v>", err) } - expectedDetachRequestedTime := asw.GetAttachedVolumes()[0].DetachRequestedTime + expectedDetachRequestedTime := asw.GetAllVolumes()[0].DetachRequestedTime // Act setVolumeMountedErr1 := asw.SetVolumeMountedByNode(generatedVolumeName, nodeName, true /* mounted */) @@ -643,7 +829,7 @@ func Test_SetVolumeMountedByNode_Positive_UnsetWithInitialSetVerifyDetachRequest t.Fatalf("SetVolumeMountedByNode2 failed. Expected Actual: <%v>", setVolumeMountedErr2) } - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -664,7 +850,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_Set(t *testing.T) { devicePath := "fake/device/path" volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -672,7 +858,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_Set(t *testing.T) { // Act: do not mark -- test default value // Assert - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -691,7 +877,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_Marked(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -707,7 +893,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_Marked(t *testing.T) { } // Assert - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -727,7 +913,7 @@ func Test_MarkDesireToDetach_Positive_MarkedAddVolumeNodeReset(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -750,7 +936,7 @@ func Test_MarkDesireToDetach_Positive_MarkedAddVolumeNodeReset(t *testing.T) { } // Assert - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -770,7 +956,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_UnsetWithInitialSetVolumeMou volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -794,7 +980,7 @@ func Test_RemoveVolumeFromReportAsAttached_Positive_UnsetWithInitialSetVolumeMou } // Assert - attachedVolumes := asw.GetAttachedVolumes() + attachedVolumes := asw.GetAllVolumes() if len(attachedVolumes) != 1 { t.Fatalf("len(attachedVolumes) Expected: <1> Actual: <%v>", len(attachedVolumes)) } @@ -813,7 +999,7 @@ func Test_RemoveVolumeFromReportAsAttached(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -846,7 +1032,7 @@ func Test_RemoveVolumeFromReportAsAttached_AddVolumeToReportAsAttached_Positive( volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -889,7 +1075,7 @@ func Test_RemoveVolumeFromReportAsAttached_Delete_AddVolumeNode(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -910,7 +1096,7 @@ func Test_RemoveVolumeFromReportAsAttached_Delete_AddVolumeNode(t *testing.T) { asw.DeleteVolumeNode(generatedVolumeName, nodeName) - asw.AddVolumeNode(volumeName, volumeSpec, nodeName, "" /*device path*/) + asw.AddVolumeNode(volumeName, volumeSpec, nodeName, "" /*device path*/, true) reportAsAttachedVolumesMap = asw.GetVolumesToReportAttached() volumes, exists = reportAsAttachedVolumesMap[nodeName] @@ -934,7 +1120,7 @@ func Test_SetDetachRequestTime_Positive(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } @@ -958,14 +1144,14 @@ func Test_SetDetachRequestTime_Positive(t *testing.T) { } } -func Test_GetAttachedVolumesForNode_Positive_NoVolumesOrNodes(t *testing.T) { +func Test_GetAllVolumesForNode_Positive_NoVolumesOrNodes(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) asw := NewActualStateOfWorld(volumePluginMgr) node := types.NodeName("random") // Act - attachedVolumes := asw.GetAttachedVolumesForNode(node) + attachedVolumes := asw.GetAllVolumesForNode(node) // Assert if len(attachedVolumes) != 0 { @@ -973,7 +1159,7 @@ func Test_GetAttachedVolumesForNode_Positive_NoVolumesOrNodes(t *testing.T) { } } -func Test_GetAttachedVolumesForNode_Positive_OneVolumeOneNode(t *testing.T) { +func Test_GetAllVolumesForNode_Positive_OneVolumeOneNode(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) asw := NewActualStateOfWorld(volumePluginMgr) @@ -981,13 +1167,13 @@ func Test_GetAttachedVolumesForNode_Positive_OneVolumeOneNode(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) nodeName := types.NodeName("node-name") devicePath := "fake/device/path" - generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath) + generatedVolumeName, addErr := asw.AddVolumeNode(volumeName, volumeSpec, nodeName, devicePath, true) if addErr != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", addErr) } // Act - attachedVolumes := asw.GetAttachedVolumesForNode(nodeName) + attachedVolumes := asw.GetAllVolumesForNode(nodeName) // Assert if len(attachedVolumes) != 1 { @@ -997,7 +1183,7 @@ func Test_GetAttachedVolumesForNode_Positive_OneVolumeOneNode(t *testing.T) { verifyAttachedVolume(t, attachedVolumes, generatedVolumeName, string(volumeName), nodeName, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) } -func Test_GetAttachedVolumesForNode_Positive_TwoVolumeTwoNodes(t *testing.T) { +func Test_GetAllVolumesForNode_Positive_TwoVolumeTwoNodes(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) asw := NewActualStateOfWorld(volumePluginMgr) @@ -1005,20 +1191,20 @@ func Test_GetAttachedVolumesForNode_Positive_TwoVolumeTwoNodes(t *testing.T) { volume1Spec := controllervolumetesting.GetTestVolumeSpec(string(volume1Name), volume1Name) node1Name := types.NodeName("node1-name") devicePath := "fake/device/path" - _, add1Err := asw.AddVolumeNode(volume1Name, volume1Spec, node1Name, devicePath) + _, add1Err := asw.AddVolumeNode(volume1Name, volume1Spec, node1Name, devicePath, true) if add1Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add1Err) } volume2Name := v1.UniqueVolumeName("volume2-name") volume2Spec := controllervolumetesting.GetTestVolumeSpec(string(volume2Name), volume2Name) node2Name := types.NodeName("node2-name") - generatedVolumeName2, add2Err := asw.AddVolumeNode(volume2Name, volume2Spec, node2Name, devicePath) + generatedVolumeName2, add2Err := asw.AddVolumeNode(volume2Name, volume2Spec, node2Name, devicePath, true) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } // Act - attachedVolumes := asw.GetAttachedVolumesForNode(node2Name) + attachedVolumes := asw.GetAllVolumesForNode(node2Name) // Assert if len(attachedVolumes) != 1 { @@ -1028,7 +1214,7 @@ func Test_GetAttachedVolumesForNode_Positive_TwoVolumeTwoNodes(t *testing.T) { verifyAttachedVolume(t, attachedVolumes, generatedVolumeName2, string(volume2Name), node2Name, devicePath, true /* expectedMountedByNode */, false /* expectNonZeroDetachRequestedTime */) } -func Test_GetAttachedVolumesForNode_Positive_OneVolumeTwoNodes(t *testing.T) { +func Test_GetAllVolumesForNode_Positive_OneVolumeTwoNodes(t *testing.T) { // Arrange volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) asw := NewActualStateOfWorld(volumePluginMgr) @@ -1036,12 +1222,20 @@ func Test_GetAttachedVolumesForNode_Positive_OneVolumeTwoNodes(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := types.NodeName("node1-name") devicePath := "fake/device/path" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath) + plugin, err := volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) + if err != nil || plugin == nil { + t.Fatalf("Failed to get volume plugin from spec %v, %v", volumeSpec, err) + } + uniqueVolumeName, err := volumeutil.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) + if err != nil || plugin == nil { + t.Fatalf("Failed to get uniqueVolumeName from spec %v, %v", volumeSpec, err) + } + generatedVolumeName1, add1Err := asw.AddVolumeNode(uniqueVolumeName, volumeSpec, node1Name, devicePath, true) 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(v1.UniqueVolumeName(""), volumeSpec, node2Name, devicePath, true) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -1054,7 +1248,7 @@ func Test_GetAttachedVolumesForNode_Positive_OneVolumeTwoNodes(t *testing.T) { } // Act - attachedVolumes := asw.GetAttachedVolumesForNode(node1Name) + attachedVolumes := asw.GetAllVolumesForNode(node1Name) // Assert if len(attachedVolumes) != 1 { @@ -1072,13 +1266,21 @@ func Test_OneVolumeTwoNodes_TwoDevicePaths(t *testing.T) { volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) node1Name := types.NodeName("node1-name") devicePath1 := "fake/device/path1" - generatedVolumeName1, add1Err := asw.AddVolumeNode(volumeName, volumeSpec, node1Name, devicePath1) + plugin, err := volumePluginMgr.FindAttachablePluginBySpec(volumeSpec) + if err != nil || plugin == nil { + t.Fatalf("Failed to get volume plugin from spec %v, %v", volumeSpec, err) + } + uniqueVolumeName, err := volumeutil.GetUniqueVolumeNameFromSpec(plugin, volumeSpec) + if err != nil || plugin == nil { + t.Fatalf("Failed to get uniqueVolumeName from spec %v, %v", volumeSpec, err) + } + generatedVolumeName1, add1Err := asw.AddVolumeNode(uniqueVolumeName, volumeSpec, node1Name, devicePath1, true) 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(v1.UniqueVolumeName(""), volumeSpec, node2Name, devicePath2, true) if add2Err != nil { t.Fatalf("AddVolumeNode failed. Expected: Actual: <%v>", add2Err) } @@ -1091,7 +1293,7 @@ func Test_OneVolumeTwoNodes_TwoDevicePaths(t *testing.T) { } // Act - attachedVolumes := asw.GetAttachedVolumesForNode(node2Name) + attachedVolumes := asw.GetAllVolumesForNode(node2Name) // Assert if len(attachedVolumes) != 1 { diff --git a/pkg/controller/volume/attachdetach/metrics/metrics.go b/pkg/controller/volume/attachdetach/metrics/metrics.go index 86f99c5e8b6..1ff42733cf1 100644 --- a/pkg/controller/volume/attachdetach/metrics/metrics.go +++ b/pkg/controller/volume/attachdetach/metrics/metrics.go @@ -185,7 +185,7 @@ func (collector *attachDetachStateCollector) getTotalVolumesCount() volumeCount stateVolumeMap.add("desired_state_of_world", pluginName) } } - for _, v := range collector.asw.GetAttachedVolumes() { + for _, v := range collector.asw.GetAllVolumes() { if plugin, err := collector.volumePluginMgr.FindPluginBySpec(v.VolumeSpec); err == nil { pluginName := pluginNameNotAvailable if plugin != nil { diff --git a/pkg/controller/volume/attachdetach/metrics/metrics_test.go b/pkg/controller/volume/attachdetach/metrics/metrics_test.go index 00efe32c1aa..239e5cfc5e9 100644 --- a/pkg/controller/volume/attachdetach/metrics/metrics_test.go +++ b/pkg/controller/volume/attachdetach/metrics/metrics_test.go @@ -143,7 +143,7 @@ func TestTotalVolumesMetricCollection(t *testing.T) { if err != nil { t.Fatalf("Expected no error, got %v", err) } - asw.AddVolumeNode(volumeName, volumeSpec, nodeName, "") + asw.AddVolumeNode(volumeName, volumeSpec, nodeName, "", true) metricCollector := newAttachDetachStateCollector( nil, diff --git a/pkg/controller/volume/attachdetach/reconciler/reconciler.go b/pkg/controller/volume/attachdetach/reconciler/reconciler.go index deb257e00f4..7ccff1bb7fd 100644 --- a/pkg/controller/volume/attachdetach/reconciler/reconciler.go +++ b/pkg/controller/volume/attachdetach/reconciler/reconciler.go @@ -175,16 +175,15 @@ func (rc *reconciler) reconcile() { // pods that are rescheduled to a different node are detached first. // Ensure volumes that should be detached are detached. - for _, attachedVolume := range rc.actualStateOfWorld.GetAttachedVolumes() { + for _, attachedVolume := range rc.actualStateOfWorld.GetAllVolumes() { if !rc.desiredStateOfWorld.VolumeExists( attachedVolume.VolumeName, attachedVolume.NodeName) { - // Don't even try to start an operation if there is already one running // This check must be done before we do any other checks, as otherwise the other checks // may pass while at the same time the volume leaves the pending state, resulting in // double detach attempts if rc.attacherDetacher.IsOperationPending(attachedVolume.VolumeName, "") { - klog.V(10).Infof("Operation for volume %q is already running. Can't start detach for %q", attachedVolume.VolumeName, attachedVolume.NodeName) + klog.V(5).Infof("Operation for volume %q is already running. Can't start detach for %q", attachedVolume.VolumeName, attachedVolume.NodeName) continue } @@ -198,7 +197,7 @@ func (rc *reconciler) reconcile() { timeout := elapsedTime > rc.maxWaitForUnmountDuration // Check whether volume is still mounted. Skip detach if it is still mounted unless timeout if attachedVolume.MountedByNode && !timeout { - klog.V(12).Infof(attachedVolume.GenerateMsgDetailed("Cannot detach volume because it is still mounted", "")) + klog.V(5).Infof(attachedVolume.GenerateMsgDetailed("Cannot detach volume because it is still mounted", "")) continue } @@ -253,7 +252,7 @@ func (rc *reconciler) reconcile() { func (rc *reconciler) attachDesiredVolumes() { // Ensure volumes that should be attached are attached. for _, volumeToAttach := range rc.desiredStateOfWorld.GetVolumesToAttach() { - if rc.actualStateOfWorld.VolumeNodeExists(volumeToAttach.VolumeName, volumeToAttach.NodeName) { + if rc.actualStateOfWorld.IsVolumeAttachedToNode(volumeToAttach.VolumeName, volumeToAttach.NodeName) { // Volume/Node exists, touch it to reset detachRequestedTime if klog.V(5) { klog.Infof(volumeToAttach.GenerateMsgDetailed("Volume attached--touching", "")) diff --git a/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go b/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go index 72e2db61ab7..aa2d5ccd219 100644 --- a/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go +++ b/pkg/controller/volume/attachdetach/reconciler/reconciler_test.go @@ -377,7 +377,7 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteMany(t *testing. volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) volumeSpec.PersistentVolume.Spec.AccessModes = []v1.PersistentVolumeAccessMode{v1.ReadWriteMany} nodeName1 := k8stypes.NodeName("node-name1") - nodeName2 := k8stypes.NodeName("node-name2") + nodeName2 := k8stypes.NodeName(volumetesting.MultiAttachNode) dsw.AddNode(nodeName1, false /*keepTerminatedPodVolumes*/) dsw.AddNode(nodeName2, false /*keepTerminatedPodVolumes*/) @@ -385,6 +385,7 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteMany(t *testing. if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) } + _, podAddErr = dsw.AddPod(types.UniquePodName(podName2), controllervolumetesting.NewPod(podName2, podName2), volumeSpec, nodeName2) if podAddErr != nil { t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) @@ -532,6 +533,72 @@ func Test_Run_OneVolumeAttachAndDetachMultipleNodesWithReadWriteOnce(t *testing. waitForTotalAttachCallCount(t, 2 /* expectedAttachCallCount */, fakePlugin) } +// Creates a volume with accessMode ReadWriteOnce +// First create a pod which will try to attach the volume to the a node named "uncertain-node". The attach call for this node will +// fail for timeout, but the volume will be actually attached to the node after the call. +// Secondly, delete the this pod. +// Lastly, create a pod scheduled to a normal node which will trigger attach volume to the node. The attach should return successfully. +func Test_Run_OneVolumeAttachAndDetachUncertainNodesWithReadWriteOnce(t *testing.T) { + // Arrange + volumePluginMgr, _ := volumetesting.GetTestVolumePluginMgr(t) + dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) + asw := cache.NewActualStateOfWorld(volumePluginMgr) + fakeKubeClient := controllervolumetesting.CreateTestClient() + fakeRecorder := &record.FakeRecorder{} + fakeHandler := volumetesting.NewBlockVolumePathHandler() + ad := operationexecutor.NewOperationExecutor(operationexecutor.NewOperationGenerator( + fakeKubeClient, + volumePluginMgr, + fakeRecorder, + false, /* checkNodeCapabilitiesBeforeMount */ + fakeHandler)) + nsu := statusupdater.NewFakeNodeStatusUpdater(false /* returnError */) + reconciler := NewReconciler( + reconcilerLoopPeriod, maxWaitForUnmountDuration, syncLoopPeriod, false, dsw, asw, ad, nsu, fakeRecorder) + podName1 := "pod-uid1" + podName2 := "pod-uid2" + volumeName := v1.UniqueVolumeName("volume-name") + volumeSpec := controllervolumetesting.GetTestVolumeSpec(string(volumeName), volumeName) + volumeSpec.PersistentVolume.Spec.AccessModes = []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce} + nodeName1 := k8stypes.NodeName(volumetesting.UncertainAttachNode) + nodeName2 := k8stypes.NodeName("node-name2") + dsw.AddNode(nodeName1, false /*keepTerminatedPodVolumes*/) + dsw.AddNode(nodeName2, false /*keepTerminatedPodVolumes*/) + + // Act + ch := make(chan struct{}) + go reconciler.Run(ch) + defer close(ch) + + // Add the pod in which the volume is attached to the uncertain node + generatedVolumeName, podAddErr := dsw.AddPod(types.UniquePodName(podName1), controllervolumetesting.NewPod(podName1, podName1), volumeSpec, nodeName1) + if podAddErr != nil { + t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) + } + + // Volume is added to asw. Because attach operation fails, volume should not reported as attached to the node. + waitForVolumeAddedToNode(t, generatedVolumeName, nodeName1, asw) + veriryVolumeAttachedToNode(t, generatedVolumeName, nodeName1, false, asw) + veriryVolumeReportedAsAttachedToNode(t, generatedVolumeName, nodeName1, false, asw) + + // When volume is added to the node, it is set to mounted by default. Then the status will be updated by checking node status VolumeInUse. + // Without this, the delete operation will be delayed due to mounted status + asw.SetVolumeMountedByNode(generatedVolumeName, nodeName1, false /* mounted */) + + dsw.DeletePod(types.UniquePodName(podName1), generatedVolumeName, nodeName1) + + waitForVolumeRemovedFromNode(t, generatedVolumeName, nodeName1, asw) + + // Add a second pod which tries to attach the volume to a different node. + generatedVolumeName, podAddErr = dsw.AddPod(types.UniquePodName(podName2), controllervolumetesting.NewPod(podName2, podName2), volumeSpec, nodeName2) + if podAddErr != nil { + t.Fatalf("AddPod failed. Expected: Actual: <%v>", podAddErr) + } + waitForVolumeAttachedToNode(t, generatedVolumeName, nodeName2, asw) + veriryVolumeAttachedToNode(t, generatedVolumeName, nodeName2, true, asw) + +} + func Test_ReportMultiAttachError(t *testing.T) { type nodeWithPods struct { name k8stypes.NodeName @@ -905,6 +972,149 @@ func verifyNewAttacherCallCount( } } +func waitForVolumeAttachedToNode( + t *testing.T, + volumeName v1.UniqueVolumeName, + nodeName k8stypes.NodeName, + asw cache.ActualStateOfWorld) { + + err := retryWithExponentialBackOff( + time.Duration(500*time.Millisecond), + func() (bool, error) { + if asw.IsVolumeAttachedToNode(volumeName, nodeName) { + return true, nil + } + t.Logf( + "Warning: Volume <%v> is not attached to node <%v> yet. Will retry.", + volumeName, + nodeName) + + return false, nil + }, + ) + + if err != nil && !asw.IsVolumeAttachedToNode(volumeName, nodeName) { + t.Fatalf( + "Volume <%v> is not attached to node <%v>.", + volumeName, + nodeName) + } +} + +func waitForVolumeAddedToNode( + t *testing.T, + volumeName v1.UniqueVolumeName, + nodeName k8stypes.NodeName, + asw cache.ActualStateOfWorld) { + + err := retryWithExponentialBackOff( + time.Duration(500*time.Millisecond), + func() (bool, error) { + volumes := asw.GetAllVolumes() + for _, volume := range volumes { + if volume.VolumeName == volumeName && volume.NodeName == nodeName { + return true, nil + } + } + t.Logf( + "Warning: Volume <%v> is not added to node <%v> yet. Will retry.", + volumeName, + nodeName) + + return false, nil + }, + ) + + if err != nil { + t.Fatalf( + "Volume <%v> is not added to node <%v>. %v", + volumeName, + nodeName, err) + } +} + +func waitForVolumeRemovedFromNode( + t *testing.T, + volumeName v1.UniqueVolumeName, + nodeName k8stypes.NodeName, + asw cache.ActualStateOfWorld) { + + err := retryWithExponentialBackOff( + time.Duration(500*time.Millisecond), + func() (bool, error) { + volumes := asw.GetAllVolumes() + exist := false + for _, volume := range volumes { + if volume.VolumeName == volumeName && volume.NodeName == nodeName { + exist = true + } + } + if exist { + t.Logf( + "Warning: Volume <%v> is not removed from the node <%v> yet. Will retry.", + volumeName, + nodeName) + + return false, nil + } + return true, nil + + }, + ) + + if err != nil { + t.Fatalf( + "Volume <%v> is not removed from node <%v>. %v", + volumeName, + nodeName, err) + } +} + +func veriryVolumeAttachedToNode( + t *testing.T, + volumeName v1.UniqueVolumeName, + nodeName k8stypes.NodeName, + isAttached bool, + asw cache.ActualStateOfWorld, +) { + result := asw.IsVolumeAttachedToNode(volumeName, nodeName) + if result == isAttached { + return + } + t.Fatalf("Check volume <%v> is attached to node <%v>, got %v, expected %v", + volumeName, + nodeName, + result, + isAttached) + +} + +func veriryVolumeReportedAsAttachedToNode( + t *testing.T, + volumeName v1.UniqueVolumeName, + nodeName k8stypes.NodeName, + isAttached bool, + asw cache.ActualStateOfWorld, +) { + result := false + volumes := asw.GetVolumesToReportAttached() + for _, volume := range volumes[nodeName] { + if volume.Name == volumeName { + result = true + } + } + + if result == isAttached { + return + } + t.Fatalf("Check volume <%v> is reported as attached to node <%v>, got %v, expected %v", + volumeName, + nodeName, + result, + isAttached) + +} + func verifyNewDetacherCallCount( t *testing.T, expectZeroNewDetacherCallCount bool, diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index a243e724b25..15af6eb1827 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -328,7 +328,7 @@ func (plugin *TestPlugin) GetErrorEncountered() bool { return plugin.ErrorEncountered } -func (plugin *TestPlugin) GetAttachedVolumes() map[string][]string { +func (plugin *TestPlugin) GetAllVolumes() map[string][]string { plugin.pluginLock.RLock() defer plugin.pluginLock.RUnlock() ret := make(map[string][]string) diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index a7b06af94ea..ef70976c4c7 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go @@ -308,6 +308,11 @@ func (asw *actualStateOfWorld) MarkVolumeAsAttached( return asw.addVolume(volumeName, volumeSpec, devicePath) } +func (asw *actualStateOfWorld) MarkVolumeAsUncertain( + volumeName v1.UniqueVolumeName, volumeSpec *volume.Spec, _ types.NodeName) error { + return nil +} + func (asw *actualStateOfWorld) MarkVolumeAsDetached( volumeName v1.UniqueVolumeName, nodeName types.NodeName) { asw.DeleteVolume(volumeName) diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 4c116aa5934..852373e1bd4 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -46,9 +46,16 @@ import ( "k8s.io/kubernetes/pkg/volume/util/volumepathhandler" ) -// A hook specified in storage class to indicate it's provisioning -// is expected to fail. -const ExpectProvisionFailureKey = "expect-provision-failure" +const ( + // A hook specified in storage class to indicate it's provisioning + // is expected to fail. + ExpectProvisionFailureKey = "expect-provision-failure" + // The node is marked as uncertain. The attach operation will fail and return timeout error + // but the operation is actually succeeded. + UncertainAttachNode = "uncertain-attach-node" + // The node is marked as multi-attach which means it is allowed to attach the volume to multiple nodes. + MultiAttachNode = "multi-attach-node" +) // fakeVolumeHost is useful for testing volume plugins. type fakeVolumeHost struct { @@ -273,10 +280,15 @@ var _ DeviceMountableVolumePlugin = &FakeVolumePlugin{} var _ FSResizableVolumePlugin = &FakeVolumePlugin{} func (plugin *FakeVolumePlugin) getFakeVolume(list *[]*FakeVolume) *FakeVolume { + volumeList := *list + if list != nil && len(volumeList) > 0 { + return volumeList[0] + } volume := &FakeVolume{ WaitForAttachHook: plugin.WaitForAttachHook, UnmountDeviceHook: plugin.UnmountDeviceHook, } + volume.VolumesAttached = make(map[string]types.NodeName) *list = append(*list, volume) return volume } @@ -329,6 +341,8 @@ func (plugin *FakeVolumePlugin) NewMounter(spec *Spec, pod *v1.Pod, opts VolumeO plugin.Lock() defer plugin.Unlock() volume := plugin.getFakeVolume(&plugin.Mounters) + volume.Lock() + defer volume.Unlock() volume.PodUID = pod.UID volume.VolName = spec.Name() volume.Plugin = plugin @@ -346,6 +360,8 @@ func (plugin *FakeVolumePlugin) NewUnmounter(volName string, podUID types.UID) ( plugin.Lock() defer plugin.Unlock() volume := plugin.getFakeVolume(&plugin.Unmounters) + volume.Lock() + defer volume.Unlock() volume.PodUID = podUID volume.VolName = volName volume.Plugin = plugin @@ -364,6 +380,8 @@ func (plugin *FakeVolumePlugin) NewBlockVolumeMapper(spec *Spec, pod *v1.Pod, op plugin.Lock() defer plugin.Unlock() volume := plugin.getFakeVolume(&plugin.BlockVolumeMappers) + volume.Lock() + defer volume.Unlock() if pod != nil { volume.PodUID = pod.UID } @@ -384,6 +402,8 @@ func (plugin *FakeVolumePlugin) NewBlockVolumeUnmapper(volName string, podUID ty plugin.Lock() defer plugin.Unlock() volume := plugin.getFakeVolume(&plugin.BlockVolumeUnmappers) + volume.Lock() + defer volume.Unlock() volume.PodUID = podUID volume.VolName = volName volume.Plugin = plugin @@ -424,7 +444,16 @@ func (plugin *FakeVolumePlugin) NewDetacher() (Detacher, error) { plugin.Lock() defer plugin.Unlock() plugin.NewDetacherCallCount = plugin.NewDetacherCallCount + 1 - return plugin.getFakeVolume(&plugin.Detachers), nil + detacher := plugin.getFakeVolume(&plugin.Detachers) + attacherList := plugin.Attachers + if attacherList != nil && len(attacherList) > 0 { + detacherList := plugin.Detachers + if detacherList != nil && len(detacherList) > 0 { + detacherList[0].VolumesAttached = attacherList[0].VolumesAttached + } + + } + return detacher, nil } func (plugin *FakeVolumePlugin) NewDeviceUnmounter() (DeviceUnmounter, error) { @@ -557,6 +586,7 @@ type FakeVolume struct { VolName string Plugin *FakeVolumePlugin MetricsNil + VolumesAttached map[string]types.NodeName // Add callbacks as needed WaitForAttachHook func(spec *Spec, devicePath string, pod *v1.Pod, spectimeout time.Duration) (string, error) @@ -577,6 +607,20 @@ type FakeVolume struct { PodDeviceMapPathCallCount int } +func getUniqueVolumeName(spec *Spec) (string, error) { + var volumeName string + if spec.Volume != nil && spec.Volume.GCEPersistentDisk != nil { + volumeName = spec.Volume.GCEPersistentDisk.PDName + } else if spec.PersistentVolume != nil && + spec.PersistentVolume.Spec.GCEPersistentDisk != nil { + volumeName = spec.PersistentVolume.Spec.GCEPersistentDisk.PDName + } + if volumeName == "" { + volumeName = spec.Name() + } + return volumeName, nil +} + func (_ *FakeVolume) GetAttributes() Attributes { return Attributes{ ReadOnly: false, @@ -722,6 +766,25 @@ func (fv *FakeVolume) Attach(spec *Spec, nodeName types.NodeName) (string, error fv.Lock() defer fv.Unlock() fv.AttachCallCount++ + volumeName, err := getUniqueVolumeName(spec) + if err != nil { + return "", err + } + volumeNode, exist := fv.VolumesAttached[volumeName] + if exist { + if nodeName == UncertainAttachNode { + return "", fmt.Errorf("Timed out to attach volume %q to node %q", volumeName, nodeName) + } + if volumeNode == nodeName || volumeNode == MultiAttachNode || nodeName == MultiAttachNode { + return "/dev/vdb-test", nil + } + return "", fmt.Errorf("volume %q trying to attach to node %q is already attached to node %q", volumeName, nodeName, volumeNode) + } + + fv.VolumesAttached[volumeName] = nodeName + if nodeName == UncertainAttachNode { + return "", fmt.Errorf("Timed out to attach volume %q to node %q", volumeName, nodeName) + } return "/dev/vdb-test", nil } @@ -771,6 +834,10 @@ func (fv *FakeVolume) Detach(volumeName string, nodeName types.NodeName) error { fv.Lock() defer fv.Unlock() fv.DetachCallCount++ + if _, exist := fv.VolumesAttached[volumeName]; !exist { + return fmt.Errorf("Trying to detach volume %q that is not attached to the node %q", volumeName, nodeName) + } + delete(fv.VolumesAttached, volumeName) return nil } @@ -937,7 +1004,7 @@ func VerifyAttachCallCount( fakeVolumePlugin *FakeVolumePlugin) error { for _, attacher := range fakeVolumePlugin.GetAttachers() { actualCallCount := attacher.GetAttachCallCount() - if actualCallCount == expectedAttachCallCount { + if actualCallCount >= expectedAttachCallCount { return nil } } @@ -970,7 +1037,7 @@ func VerifyWaitForAttachCallCount( fakeVolumePlugin *FakeVolumePlugin) error { for _, attacher := range fakeVolumePlugin.GetAttachers() { actualCallCount := attacher.GetWaitForAttachCallCount() - if actualCallCount == expectedWaitForAttachCallCount { + if actualCallCount >= expectedWaitForAttachCallCount { return nil } } @@ -1003,7 +1070,7 @@ func VerifyMountDeviceCallCount( fakeVolumePlugin *FakeVolumePlugin) error { for _, attacher := range fakeVolumePlugin.GetAttachers() { actualCallCount := attacher.GetMountDeviceCallCount() - if actualCallCount == expectedMountDeviceCallCount { + if actualCallCount >= expectedMountDeviceCallCount { return nil } }