Skip to content

Commit

Permalink
utilize latest go sdk to ensure createVolume idempotency
Browse files Browse the repository at this point in the history
  • Loading branch information
AndyXiangLi committed Jul 21, 2021
1 parent 25af052 commit 8257372
Show file tree
Hide file tree
Showing 149 changed files with 29,653 additions and 3,243 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module github.com/kubernetes-sigs/aws-ebs-csi-driver

require (
github.com/aws/aws-sdk-go v1.35.37
github.com/aws/aws-sdk-go v1.40.4
github.com/container-storage-interface/spec v1.3.0
github.com/elazarl/goproxy v0.0.0-20181111060418-2ce16c963a8a // indirect
github.com/golang/mock v1.5.0
Expand All @@ -13,7 +13,7 @@ require (
github.com/onsi/ginkgo v1.11.0
github.com/onsi/gomega v1.7.1
github.com/stretchr/testify v1.6.1
golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073
golang.org/x/sys v0.0.0-20210423082822-04245dca01da
google.golang.org/grpc v1.34.0
k8s.io/api v0.21.0
k8s.io/apimachinery v0.21.0
Expand Down
10 changes: 8 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:l
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
github.com/auth0/go-jwt-middleware v0.0.0-20170425171159-5493cabe49f7/go.mod h1:LWMyo4iOLWXHGdBki7NIht1kHru/0wM179h+d3g8ATM=
github.com/aws/aws-sdk-go v1.35.24/go.mod h1:tlPOdRjfxPBpNIwqDj61rmsnA85v9jc0Ps9+muhnW+k=
github.com/aws/aws-sdk-go v1.35.37 h1:XA71k5PofXJ/eeXdWrTQiuWPEEyq8liguR+Y/QUELhI=
github.com/aws/aws-sdk-go v1.35.37/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
github.com/aws/aws-sdk-go v1.40.4 h1:kxTX1kVjuXN1vuq6JgZvWI/Lt9zCfUFuAAxFoq0dHYI=
github.com/aws/aws-sdk-go v1.40.4/go.mod h1:585smgzpB/KqRA+K3y/NL/oYRqQvpNJYvLm+LY1U59Q=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
Expand Down Expand Up @@ -690,6 +690,8 @@ golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwY
golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20210224082022-3d97a244fca7 h1:OgUuv8lsRpBibGNbSizVwKWlysjaNzmC9gYMhPVfqFM=
golang.org/x/net v0.0.0-20210224082022-3d97a244fca7/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20210614182718-04defd469f4e h1:XpT3nA5TvE525Ne3hInMh6+GETgn27Zfm9dxsThnX2Q=
golang.org/x/net v0.0.0-20210614182718-04defd469f4e/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand Down Expand Up @@ -757,6 +759,8 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073 h1:8qxJSnu+7dRq6upnbntrmriWByIakBuct5OM/MdQC1M=
golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da h1:b3NXsE2LusjYGGjL5bxEVZZORm/YEFFrWFjR8eFrw/c=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d h1:SZxvLBoTP5yHO3Frd4z4vrF+DBX9vMVanchswa69toE=
Expand All @@ -768,6 +772,8 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.4 h1:0YWbFKbhXG/wIiuHDSKpS0Iy7FSA+u45VtBMfQcFTTc=
golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
Expand Down
30 changes: 22 additions & 8 deletions pkg/cloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ var (
// ErrNotFound is returned when a resource is not found.
ErrNotFound = errors.New("Resource was not found")

// ErrIdempotent is returned when another request with same idempotent token is in-flight.
ErrIdempotentParameterMismatch = errors.New("Parameters on this idempotent request are inconsistent with parameters used in previous request(s)")

// ErrAlreadyExists is returned when a resource is already existent.
ErrAlreadyExists = errors.New("Resource already exists")

Expand Down Expand Up @@ -317,8 +320,9 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
}
}

request := &ec2.CreateVolumeInput{
requestInput := &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(zone),
ClientToken: aws.String(volumeName),
Size: aws.Int64(capacityGiB),
VolumeType: aws.String(createType),
TagSpecifications: []*ec2.TagSpecification{&tagSpec},
Expand All @@ -327,29 +331,32 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *

// EBS doesn't handle empty outpost arn, so we have to include it only when it's non-empty
if len(diskOptions.OutpostArn) > 0 {
request.OutpostArn = aws.String(diskOptions.OutpostArn)
requestInput.OutpostArn = aws.String(diskOptions.OutpostArn)
}

if len(diskOptions.KmsKeyID) > 0 {
request.KmsKeyId = aws.String(diskOptions.KmsKeyID)
request.Encrypted = aws.Bool(true)
requestInput.KmsKeyId = aws.String(diskOptions.KmsKeyID)
requestInput.Encrypted = aws.Bool(true)
}
if iops > 0 {
request.Iops = aws.Int64(iops)
requestInput.Iops = aws.Int64(iops)
}
if throughput > 0 && diskOptions.VolumeType == VolumeTypeGP3 {
request.Throughput = aws.Int64(throughput)
requestInput.Throughput = aws.Int64(throughput)
}
snapshotID := diskOptions.SnapshotID
if len(snapshotID) > 0 {
request.SnapshotId = aws.String(snapshotID)
requestInput.SnapshotId = aws.String(snapshotID)
}

response, err := c.ec2.CreateVolumeWithContext(ctx, request)
response, err := c.ec2.CreateVolumeWithContext(ctx, requestInput)
if err != nil {
if isAWSErrorSnapshotNotFound(err) {
return nil, ErrNotFound
}
if isAWSErrorIdempotentParameterMismatch(err) {
return nil, ErrIdempotentParameterMismatch
}
return nil, fmt.Errorf("could not create volume in EC2: %v", err)
}

Expand Down Expand Up @@ -989,6 +996,13 @@ func isAWSErrorSnapshotNotFound(err error) bool {
return isAWSError(err, "InvalidSnapshot.NotFound")
}

// isAWSErrorIdempotentParameterMismatch returns a boolean indicating whether the
// given error is an AWS IdempotentParameterMismatch error.
// This error is reported when the two request contains same client-token but different parameters
func isAWSErrorIdempotentParameterMismatch(err error) bool {
return isAWSError(err, "IdempotentParameterMismatch")
}

// ResizeDisk resizes an EBS volume in GiB increments, rouding up to the next possible allocatable unit.
// It returns the volume size after this call or an error if the size couldn't be determined.
func (c *cloud) ResizeDisk(ctx context.Context, volumeID string, newSizeBytes int64) (int64, error) {
Expand Down
24 changes: 24 additions & 0 deletions pkg/cloud/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,30 @@ func TestCreateDisk(t *testing.T) {
expErr: fmt.Errorf("could not create volume in EC2: CreateVolume generic error"),
expCreateVolumeErr: fmt.Errorf("CreateVolume generic error"),
},
{
name: "fail: ec2.CreateVolume returned snapshot not found error",
volumeName: "vol-test-name-error",
diskOptions: &DiskOptions{
CapacityBytes: util.GiBToBytes(1),
Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"},
AvailabilityZone: expZone,
},
expCreateVolumeInput: &ec2.CreateVolumeInput{},
expErr: ErrNotFound,
expCreateVolumeErr: awserr.New("InvalidSnapshot.NotFound", "Snapshot not found", fmt.Errorf("not able to find source snapshot")),
},
{
name: "fail: ec2.CreateVolume returned Idempotent Parameter Mismatch error",
volumeName: "vol-test-name-error",
diskOptions: &DiskOptions{
CapacityBytes: util.GiBToBytes(1),
Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"},
AvailabilityZone: expZone,
},
expCreateVolumeInput: &ec2.CreateVolumeInput{},
expErr: ErrIdempotentParameterMismatch,
expCreateVolumeErr: awserr.New("IdempotentParameterMismatch", "Another request is in-flight", fmt.Errorf("another request is in-flight")),
},
{
name: "fail: ec2.DescribeVolumes error after volume created",
volumeName: "vol-test-name-error",
Expand Down
26 changes: 4 additions & 22 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,6 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
}
defer d.inFlight.Delete(volName)

disk, err := d.cloud.GetDiskByName(ctx, volName, volSizeBytes)
if err != nil {
switch err {
case cloud.ErrNotFound:
case cloud.ErrMultiDisks:
return nil, status.Error(codes.Internal, err.Error())
case cloud.ErrDiskExistsDiffSize:
return nil, status.Error(codes.AlreadyExists, err.Error())
default:
return nil, status.Error(codes.Internal, err.Error())
}
}

var (
volumeType string
iopsPerGB int
Expand Down Expand Up @@ -203,14 +190,6 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
snapshotID = sourceSnapshot.GetSnapshotId()
}

// volume exists already
if disk != nil {
if disk.SnapshotID != snapshotID {
return nil, status.Errorf(codes.AlreadyExists, "Volume already exists, but was restored from a different snapshot than %s", snapshotID)
}
return newCreateVolumeResponse(disk), nil
}

// create a new volume
zone := pickAvailabilityZone(req.GetAccessibilityRequirements())
outpostArn := getOutpostArn(req.GetAccessibilityRequirements())
Expand Down Expand Up @@ -241,12 +220,15 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
SnapshotID: snapshotID,
}

disk, err = d.cloud.CreateDisk(ctx, volName, opts)
disk, err := d.cloud.CreateDisk(ctx, volName, opts)
if err != nil {
errCode := codes.Internal
if err == cloud.ErrNotFound {
errCode = codes.NotFound
}
if err == cloud.ErrIdempotentParameterMismatch {
errCode = codes.AlreadyExists
}
return nil, status.Errorf(errCode, "Could not create volume %q: %v", volName, err)
}
return newCreateVolumeResponse(disk), nil
Expand Down
Loading

0 comments on commit 8257372

Please sign in to comment.