From 0ef0e76add66703313ddb0bf1b446ce26c41f18d Mon Sep 17 00:00:00 2001
From: Connor Catlett <conncatl@amazon.com>
Date: Mon, 14 Oct 2024 14:40:57 +0000
Subject: [PATCH] Only bail to AttachVolume if volume is detached

Signed-off-by: Connor Catlett <conncatl@amazon.com>
---
 pkg/cloud/cloud.go      |  2 +-
 pkg/cloud/cloud_test.go | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
index da71ef1381..8a483c932e 100644
--- a/pkg/cloud/cloud.go
+++ b/pkg/cloud/cloud.go
@@ -1080,7 +1080,7 @@ func (c *cloud) WaitForAttachmentState(ctx context.Context, volumeID, expectedSt
 		// if we expected volume to be attached and it was reported as already attached via DescribeInstance call
 		// but DescribeVolume told us volume is detached, we will short-circuit this long wait loop and return error
 		// so as AttachDisk can be retried without waiting for 20 minutes.
-		if (expectedState == volumeAttachedState) && alreadyAssigned && (attachmentState != expectedState) {
+		if (expectedState == volumeAttachedState) && alreadyAssigned && (attachmentState == volumeDetachedState) {
 			request := &ec2.AttachVolumeInput{
 				Device:     aws.String(expectedDevice),
 				InstanceId: aws.String(expectedInstance),
diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go
index 4f43a1bc0b..84100c8562 100644
--- a/pkg/cloud/cloud_test.go
+++ b/pkg/cloud/cloud_test.go
@@ -3301,7 +3301,7 @@ func TestWaitForAttachmentState(t *testing.T) {
 			expectError:      true,
 		},
 		{
-			name:             "failure: already assigned but wrong state",
+			name:             "failure: already assigned but detached state",
 			volumeID:         "vol-test-1234",
 			expectedState:    volumeAttachedState,
 			expectedInstance: "1234",
@@ -3309,6 +3309,15 @@ func TestWaitForAttachmentState(t *testing.T) {
 			alreadyAssigned:  true,
 			expectError:      true,
 		},
+		{
+			name:             "failure: already assigned but attaching state",
+			volumeID:         "vol-test-1234",
+			expectedState:    volumeAttachedState,
+			expectedInstance: "1234",
+			expectedDevice:   defaultPath,
+			alreadyAssigned:  true,
+			expectError:      false,
+		},
 		{
 			name:             "failure: multiple attachments with Multi-Attach disabled",
 			volumeID:         "vol-test-1234",
@@ -3353,9 +3362,12 @@ func TestWaitForAttachmentState(t *testing.T) {
 			switch tc.name {
 			case "success: detached":
 				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []types.Volume{detachedVol}}, nil).AnyTimes()
-			case "failure: already assigned but wrong state":
+			case "failure: already assigned but detached state":
 				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []types.Volume{detachedVol}}, nil)
 				mockEC2.EXPECT().AttachVolume(gomock.Any(), gomock.Any()).Return(nil, nil)
+			case "failure: already assigned but attaching state":
+				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []types.Volume{attachingVol}}, nil)
+				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []types.Volume{attachedVol}}, nil)
 			case "success: disk not found, assumed detached", "failure: disk not found, expected attached":
 				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(nil, &smithy.GenericAPIError{
 					Code:    "InvalidVolume.NotFound",