From 0f5ece630e6a2e63da58eff7f64a5926c9108d1f Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 21 Sep 2020 15:16:50 -0400 Subject: [PATCH] Remove code that handled resize progress Now we are preventing such errors by checking volume modifications first --- pkg/cloud/cloud.go | 36 ++++++++++++------------------------ pkg/cloud/cloud_test.go | 23 ++--------------------- 2 files changed, 14 insertions(+), 45 deletions(-) diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 80abaaa4fd..cfa44be698 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -59,7 +59,10 @@ var ( VolumeTypeST1, VolumeTypeStandard, } - VolumeNotBeingModified = fmt.Errorf("volume is not being modified") + + volumeModificationDuration = 1 * time.Second + volumeModificationWaitFactor = 1.7 + volumeModificationWaitSteps = 10 ) // AWS provisioning limits. @@ -115,6 +118,9 @@ var ( // ErrInvalidMaxResults is returned when a MaxResults pagination parameter is between 1 and 4 ErrInvalidMaxResults = errors.New("MaxResults parameter must be 0 or greater than or equal to 5") + + // VolumeNotBeingModified is returned if volume being described is not being modified + VolumeNotBeingModified = fmt.Errorf("volume is not being modified") ) // Disk represents a EBS volume @@ -799,13 +805,6 @@ func isAWSError(err error, code string) bool { return false } -// isAWSErrorIncorrectModification returns a boolean indicating whether the given error -// is an AWS IncorrectModificationState error. This error means that a modification action -// on an EBS volume cannot occur because the volume is currently being modified. -func isAWSErrorIncorrectModification(err error) bool { - return isAWSError(err, "IncorrectModificationState") -} - // isAWSErrorInstanceNotFound returns a boolean indicating whether the // given error is an AWS InvalidInstanceID.NotFound error. This error is // reported when the specified instance doesn't exist. @@ -900,23 +899,12 @@ func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes in } klog.Infof("expanding volume %q to size %d", volumeID, newSizeGiB) - var mod *ec2.VolumeModification response, err := c.ec2.ModifyVolumeWithContext(ctx, req) if err != nil { - if !isAWSErrorIncorrectModification(err) { - return 0, fmt.Errorf("could not modify AWS volume %q: %v", volumeID, err) - } - - m, modFetchError := c.getLatestVolumeModification(ctx, volumeID) - if modFetchError != nil { - return 0, modFetchError - } - mod = m + return 0, fmt.Errorf("could not modify AWS volume %q: %v", volumeID, err) } - if mod == nil { - mod = response.VolumeModification - } + mod := response.VolumeModification state := aws.StringValue(mod.ModificationState) if volumeModificationDone(state) { @@ -955,9 +943,9 @@ func (c *cloud) checkDesiredSize(ctx context.Context, volumeID string, newSizeGi // waitForVolumeSize waits for a volume modification to finish and return its size. func (c *cloud) waitForVolumeSize(ctx context.Context, volumeID string) (int64, error) { backoff := wait.Backoff{ - Duration: 1 * time.Second, - Factor: 1.7, - Steps: 10, + Duration: volumeModificationDuration, + Factor: volumeModificationWaitFactor, + Steps: volumeModificationWaitSteps, } var modVolSizeGiB int64 diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index baac3c94b0..e7f6c1ccce 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -686,27 +686,6 @@ func TestResizeDisk(t *testing.T) { reqSizeGiB: 2, expErr: fmt.Errorf("ResizeDisk generic error"), }, - { - name: "success: there is a resizing in progress", - volumeID: "vol-test", - existingVolume: &ec2.Volume{ - VolumeId: aws.String("vol-test"), - Size: aws.Int64(1), - AvailabilityZone: aws.String(defaultZone), - }, - modifiedVolumeError: awserr.New("IncorrectModificationState", "", nil), - descModVolume: &ec2.DescribeVolumesModificationsOutput{ - VolumesModifications: []*ec2.VolumeModification{ - { - VolumeId: aws.String("vol-test"), - TargetSize: aws.Int64(2), - ModificationState: aws.String(ec2.VolumeModificationStateCompleted), - }, - }, - }, - reqSizeGiB: 2, - expErr: nil, - }, { name: "failure: volume in modifying state", volumeID: "vol-test", @@ -733,6 +712,8 @@ func TestResizeDisk(t *testing.T) { t.Run(tc.name, func(t *testing.T) { mockCtrl := gomock.NewController(t) mockEC2 := mocks.NewMockEC2(mockCtrl) + // reduce number of steps to reduce test time + volumeModificationWaitSteps = 3 c := newCloud(mockEC2) ctx := context.Background()