Skip to content

Commit

Permalink
Remove code that handled resize progress
Browse files Browse the repository at this point in the history
Now we are preventing such errors by checking volume
modifications first
  • Loading branch information
gnufied committed Sep 22, 2020
1 parent bbf2ce6 commit 0f5ece6
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 45 deletions.
36 changes: 12 additions & 24 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
23 changes: 2 additions & 21 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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()
Expand Down

0 comments on commit 0f5ece6

Please sign in to comment.