Skip to content

Commit

Permalink
Merge pull request #2005 from torredil/attach-volume-71236
Browse files Browse the repository at this point in the history
Add explicit `AttachVolume` call in `WaitForAttachmentState`
  • Loading branch information
k8s-ci-robot authored Apr 11, 2024
2 parents e92b955 + f3cc303 commit 85563fb
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
9 changes: 9 additions & 0 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,15 @@ func (c *cloud) WaitForAttachmentState(ctx context.Context, volumeID, expectedSt
// 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) {
request := &ec2.AttachVolumeInput{
Device: aws.String(expectedDevice),
InstanceId: aws.String(expectedInstance),
VolumeId: aws.String(volumeID),
}
_, err := c.ec2.AttachVolume(ctx, request)
if err != nil {
return false, fmt.Errorf("WaitForAttachmentState AttachVolume error, expected device but be attached but was %s, volumeID=%q, instanceID=%q, Device=%q, err=%w", attachmentState, volumeID, expectedInstance, expectedDevice, err)
}
return false, fmt.Errorf("attachment of disk %q failed, expected device to be attached but was %s", volumeID, attachmentState)
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3029,8 +3029,11 @@ func TestWaitForAttachmentState(t *testing.T) {
defer cancel()

switch tc.name {
case "success: detached", "failure: already assigned but wrong state":
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":
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 "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",
Expand Down

0 comments on commit 85563fb

Please sign in to comment.