Merge pull request kubernetes#122790 from carlory/fix-121696

Fix flaky test: Test_Run_OneVolumeDetachFailNodeWithReadWriteOnce

pkg/controller/volume/attachdetach/reconciler/reconciler_test.go

@@ -630,9 +630,7 @@ func Test_Run_OneVolumeAttachAndDetachUncertainNodesWithReadWriteOnce(t *testing
 	if podAddErr != nil {
 		t.Fatalf("AddPod failed. Expected: <no error> Actual: <%v>", podAddErr)
 	}
-	waitForVolumeAttachedToNode(t, generatedVolumeName, nodeName2, asw)
-	verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName2, cache.AttachStateAttached, asw)
-
+	waitForVolumeAttachStateToNode(t, generatedVolumeName, nodeName2, cache.AttachStateAttached, asw)
 }
 
 func Test_Run_UpdateNodeStatusFailBeforeOneVolumeDetachNodeWithReadWriteOnce(t *testing.T) {
@@ -747,13 +745,7 @@ func Test_Run_OneVolumeDetachFailNodeWithReadWriteOnce(t *testing.T) {
 	// Delete the pod, but detach will fail
 	dsw.DeletePod(types.UniquePodName(podName1), generatedVolumeName, nodeName1)
 
-	// The first detach will be triggered after at least 50ms (maxWaitForUnmountDuration in test).
-	// Right before detach operation is performed, the volume will be first removed from being reported
-	// as attached on node status (RemoveVolumeFromReportAsAttached). After detach operation which is expected to fail,
-	// controller then treats the attachment as Uncertain.
-	// Here it sleeps 100ms so that detach should be triggered already at this point.
-	time.Sleep(100 * time.Millisecond)
-	verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName1, cache.AttachStateUncertain, asw)
+	waitForVolumeAttachStateToNode(t, generatedVolumeName, nodeName1, cache.AttachStateUncertain, asw)
 	verifyVolumeReportedAsAttachedToNode(t, logger, generatedVolumeName, nodeName1, false, asw, volumeAttachedCheckTimeout)
 
 	// Add a second pod which tries to attach the volume to the same node.
@@ -843,9 +835,7 @@ func Test_Run_OneVolumeAttachAndDetachTimeoutNodesWithReadWriteOnce(t *testing.T
 	if podAddErr != nil {
 		t.Fatalf("AddPod failed. Expected: <no error> Actual: <%v>", podAddErr)
 	}
-	waitForVolumeAttachedToNode(t, generatedVolumeName, nodeName2, asw)
-	verifyVolumeAttachedToNode(t, generatedVolumeName, nodeName2, cache.AttachStateAttached, asw)
-
+	waitForVolumeAttachStateToNode(t, generatedVolumeName, nodeName2, cache.AttachStateAttached, asw)
 }
 
 // Populates desiredStateOfWorld cache with one node/volume/pod tuple.
@@ -1626,35 +1616,33 @@ func verifyNewAttacherCallCount(
 	}
 }
 
-func waitForVolumeAttachedToNode(
+func waitForVolumeAttachStateToNode(
 	t *testing.T,
 	volumeName v1.UniqueVolumeName,
 	nodeName k8stypes.NodeName,
+	expectedAttachState cache.AttachState,
 	asw cache.ActualStateOfWorld) {
 
 	err := retryWithExponentialBackOff(
 		time.Duration(500*time.Millisecond),
 		func() (bool, error) {
 			attachState := asw.GetAttachState(volumeName, nodeName)
-			if attachState == cache.AttachStateAttached {
+			if attachState == expectedAttachState {
 				return true, nil
 			}
-			t.Logf(
-				"Warning: Volume <%v> is not attached to node  <%v> yet. Will retry.",
-				volumeName,
-				nodeName)
-
+			t.Logf("Warning: expected attach state: %v, actual attach state: %v. Will retry.",
+				expectedAttachState, attachState)
 			return false, nil
 		},
 	)
 
 	attachState := asw.GetAttachState(volumeName, nodeName)
-	if err != nil && attachState != cache.AttachStateAttached {
-		t.Fatalf(
-			"Volume <%v> is not attached to node  <%v>.",
-			volumeName,
-			nodeName)
+	if err != nil && attachState != expectedAttachState {
+		t.Fatalf("Volume <%v> is not in expected attach state: %v, actual attach state: %v",
+			volumeName, expectedAttachState, attachState)
 	}
+
+	t.Logf("Volume <%v> is attached to node <%v>: %v", volumeName, nodeName, expectedAttachState)
 }
 
 func waitForVolumeAddedToNode(