From 70350abd851d9faeaef5a3e792e7a7956a04ee04 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 31 Aug 2020 07:44:29 -0400 Subject: [PATCH] carry: Drop optimizing check from resizing call --- pkg/cloud/cloud.go | 49 ++++++++++++++++++++++++++++++++--------- pkg/cloud/cloud_test.go | 22 +++++++++++++++--- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 46ea999cfa..c1435636b2 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -648,7 +648,6 @@ func (c *cloud) ec2SnapshotResponseToStruct(ec2Snapshot *ec2.Snapshot) *Snapshot func (c *cloud) getVolume(ctx context.Context, request *ec2.DescribeVolumesInput) (*ec2.Volume, error) { var volumes []*ec2.Volume var nextToken *string - for { response, err := c.ec2.DescribeVolumesWithContext(ctx, request) if err != nil { @@ -865,8 +864,9 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in if err == nil && latestMod != nil { state := aws.StringValue(latestMod.ModificationState) targetSize := aws.Int64Value(latestMod.TargetSize) - if (state == ec2.VolumeModificationStateCompleted || state == ec2.VolumeModificationStateOptimizing) && targetSize >= newSizeGiB { - return targetSize, nil + klog.Infof("volume %q is being modified to %d size and is in %s state", volumeID, targetSize, state) + if state == ec2.VolumeModificationStateCompleted && targetSize >= newSizeGiB { + return c.checkDesiredSize(ctx, volumeID, newSizeGiB) } if state == ec2.VolumeModificationStateModifying { @@ -878,7 +878,6 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in VolumeId: aws.String(volumeID), Size: aws.Int64(newSizeGiB), } - var mod *ec2.VolumeModification response, err := c.ec2.ModifyVolumeWithContext(ctx, req) if err != nil { @@ -886,9 +885,9 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in return 0, fmt.Errorf("could not modify AWS volume %q: %v", volumeID, err) } - m, err := c.getLatestVolumeModification(ctx, volumeID) - if err != nil { - return 0, err + m, modFetchError := c.getLatestVolumeModification(ctx, volumeID) + if modFetchError != nil { + return 0, modFetchError } mod = m } @@ -898,11 +897,38 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in } state := aws.StringValue(mod.ModificationState) - if state == ec2.VolumeModificationStateCompleted || state == ec2.VolumeModificationStateOptimizing { - return aws.Int64Value(mod.TargetSize), nil + if state == ec2.VolumeModificationStateCompleted { + return c.checkDesiredSize(ctx, volumeID, newSizeGiB) + } + + _, err = c.waitForVolumeSize(ctx, volumeID) + if err != nil { + return oldSizeGiB, err } + return c.checkDesiredSize(ctx, volumeID, newSizeGiB) +} - return c.waitForVolumeSize(ctx, volumeID) +// Checks for desired size on volume by also verifying volume size by describing volume. +// This is to get around potential eventual consistency problems with describing volume modifications +// objects and ensuring that we read two different objects to verify volume state. +func (c *cloud) checkDesiredSize(ctx context.Context, volumeID string, newSizeGiB int64) (int64, error) { + request := &ec2.DescribeVolumesInput{ + VolumeIds: []*string{ + aws.String(volumeID), + }, + } + volume, err := c.getVolume(ctx, request) + if err != nil { + return 0, err + } + + // AWS resizes in chunks of GiB (not GB) + oldSizeGiB := aws.Int64Value(volume.Size) + klog.Infof("volume %q is of %d size and expected size is %d", volumeID, oldSizeGiB, newSizeGiB) + if oldSizeGiB >= newSizeGiB { + return oldSizeGiB, nil + } + return oldSizeGiB, fmt.Errorf("volume %q is still being expanded to %d size", volumeID, newSizeGiB) } // waitForVolumeSize waits for a volume modification to finish and return its size. @@ -922,7 +948,8 @@ func (c *cloud) waitForVolumeSize(ctx context.Context, volumeID string) (int64, } state := aws.StringValue(m.ModificationState) - if state == ec2.VolumeModificationStateCompleted || state == ec2.VolumeModificationStateOptimizing { + klog.Infof("volume %q is being modified to %d size and is in %s state", volumeID, aws.Int64Value(m.TargetSize), state) + if state == ec2.VolumeModificationStateCompleted { modVolSizeGiB = aws.Int64Value(m.TargetSize) return true, nil } diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index c89fefdd23..754d23e778 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -626,7 +626,7 @@ func TestResizeDisk(t *testing.T) { VolumeModification: &ec2.VolumeModification{ VolumeId: aws.String("vol-test"), TargetSize: aws.Int64(2), - ModificationState: aws.String(ec2.VolumeModificationStateOptimizing), + ModificationState: aws.String(ec2.VolumeModificationStateCompleted), }, }, reqSizeGiB: 2, @@ -719,8 +719,24 @@ func TestResizeDisk(t *testing.T) { if tc.existingVolume != nil || tc.existingVolumeError != nil { mockEC2.EXPECT().DescribeVolumesWithContext(gomock.Eq(ctx), gomock.Any()).Return( &ec2.DescribeVolumesOutput{ - Volumes: []*ec2.Volume{tc.existingVolume}, - }, tc.existingVolumeError).AnyTimes() + Volumes: []*ec2.Volume{ + tc.existingVolume, + }, + }, tc.existingVolumeError) + + if tc.expErr == nil { + resizedVolume := &ec2.Volume{ + VolumeId: aws.String("vol-test"), + Size: aws.Int64(tc.reqSizeGiB), + AvailabilityZone: aws.String(defaultZone), + } + mockEC2.EXPECT().DescribeVolumesWithContext(gomock.Eq(ctx), gomock.Any()).Return( + &ec2.DescribeVolumesOutput{ + Volumes: []*ec2.Volume{ + resizedVolume, + }, + }, tc.existingVolumeError) + } } if tc.modifiedVolume != nil || tc.modifiedVolumeError != nil { mockEC2.EXPECT().ModifyVolumeWithContext(gomock.Eq(ctx), gomock.Any()).Return(tc.modifiedVolume, tc.modifiedVolumeError).AnyTimes()