From 036ea238bf2c171974898287fece95c14f5dc8f5 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 14 Jul 2020 16:47:00 -0700 Subject: [PATCH 1/3] wrap CreateImage call in a retry to account for eventual consistencey issues with image state --- builder/amazon/ebs/step_create_ami.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/builder/amazon/ebs/step_create_ami.go b/builder/amazon/ebs/step_create_ami.go index c9163d720..5a706ad92 100644 --- a/builder/amazon/ebs/step_create_ami.go +++ b/builder/amazon/ebs/step_create_ami.go @@ -4,11 +4,14 @@ import ( "context" "fmt" "log" + "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" awscommon "github.com/hashicorp/packer/builder/amazon/common" "github.com/hashicorp/packer/common/random" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -48,7 +51,22 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi BlockDeviceMappings: config.AMIMappings.BuildEC2BlockDeviceMappings(), } - createResp, err := ec2conn.CreateImage(createOpts) + var createResp *ec2.CreateImageOutput + var err error + + err = retry.Config{ + Tries: 11, + ShouldRetry: func(err error) bool { + if err, ok := err.(awserr.Error); ok { + return err.Code() == "InvalidParameterValue" + } + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + createResp, err = ec2conn.CreateImage(createOpts) + return err + }) if err != nil { err := fmt.Errorf("Error creating AMI: %s", err) state.Put("error", err) From a56942d3c703122aab44a43457f1c9fb441d47e2 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 15 Jul 2020 09:47:07 -0700 Subject: [PATCH 2/3] change isAwsErr to an exported func so I can use it in other aws modules. --- builder/amazon/common/access_config.go | 2 +- builder/amazon/common/helper_funcs.go | 6 +++--- builder/amazon/common/ssm_driver.go | 2 +- builder/amazon/common/step_create_tags.go | 2 +- builder/amazon/common/step_pre_validate.go | 4 ++-- builder/amazon/common/step_run_source_instance.go | 8 ++++---- builder/amazon/common/step_run_spot_instance.go | 2 +- builder/amazon/common/step_stop_ebs_instance.go | 2 +- builder/amazon/ebs/step_create_ami.go | 5 ++--- 9 files changed, 16 insertions(+), 17 deletions(-) diff --git a/builder/amazon/common/access_config.go b/builder/amazon/common/access_config.go index d8e5cbc4f..c967eee26 100644 --- a/builder/amazon/common/access_config.go +++ b/builder/amazon/common/access_config.go @@ -188,7 +188,7 @@ func (c *AccessConfig) Session() (*session.Session, error) { cp, err := c.session.Config.Credentials.Get() - if isAWSErr(err, "NoCredentialProviders", "") { + if IsAWSErr(err, "NoCredentialProviders", "") { return nil, fmt.Errorf("No valid credential sources found for AWS Builder. " + "Please see https://www.packer.io/docs/builders/amazon#specifying-amazon-credentials " + "for more information on providing credentials for the AWS Builder.") diff --git a/builder/amazon/common/helper_funcs.go b/builder/amazon/common/helper_funcs.go index f614b15ef..5f75e2568 100644 --- a/builder/amazon/common/helper_funcs.go +++ b/builder/amazon/common/helper_funcs.go @@ -31,7 +31,7 @@ func DestroyAMIs(imageids []*string, ec2conn *ec2.EC2) error { err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - return isAWSErr(err, "UnauthorizedOperation", "") + return IsAWSErr(err, "UnauthorizedOperation", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) error { @@ -54,7 +54,7 @@ func DestroyAMIs(imageids []*string, ec2conn *ec2.EC2) error { err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - return isAWSErr(err, "UnauthorizedOperation", "") + return IsAWSErr(err, "UnauthorizedOperation", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) error { @@ -79,7 +79,7 @@ func DestroyAMIs(imageids []*string, ec2conn *ec2.EC2) error { // * err is of type awserr.Error // * Error.Code() matches code // * Error.Message() contains message -func isAWSErr(err error, code string, message string) bool { +func IsAWSErr(err error, code string, message string) bool { if err, ok := err.(awserr.Error); ok { return err.Code() == code && strings.Contains(err.Message(), message) } diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index e37475595..5ba73c8bd 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -50,7 +50,7 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu var output *ssm.StartSessionOutput err := retry.Config{ - ShouldRetry: func(err error) bool { return isAWSErr(err, "TargetNotConnected", "") }, + ShouldRetry: func(err error) bool { return IsAWSErr(err, "TargetNotConnected", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) (err error) { output, err = d.SvcClient.StartSessionWithContext(ctx, &input) diff --git a/builder/amazon/common/step_create_tags.go b/builder/amazon/common/step_create_tags.go index 1a6257c18..15ed105d0 100644 --- a/builder/amazon/common/step_create_tags.go +++ b/builder/amazon/common/step_create_tags.go @@ -91,7 +91,7 @@ func (s *StepCreateTags) Run(ctx context.Context, state multistep.StateBag) mult // Retry creating tags for about 2.5 minutes err = retry.Config{Tries: 11, ShouldRetry: func(error) bool { - if isAWSErr(err, "InvalidAMIID.NotFound", "") || isAWSErr(err, "InvalidSnapshot.NotFound", "") { + if IsAWSErr(err, "InvalidAMIID.NotFound", "") || IsAWSErr(err, "InvalidSnapshot.NotFound", "") { return true } return false diff --git a/builder/amazon/common/step_pre_validate.go b/builder/amazon/common/step_pre_validate.go index 3125cc698..b86e04f61 100644 --- a/builder/amazon/common/step_pre_validate.go +++ b/builder/amazon/common/step_pre_validate.go @@ -39,7 +39,7 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul err := retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - if isAWSErr(err, "AuthFailure", "") { + if IsAWSErr(err, "AuthFailure", "") { log.Printf("Waiting for Vault-generated AWS credentials" + " to pass authentication... trying again.") return true @@ -131,7 +131,7 @@ func (s *StepPreValidate) checkVpc(conn ec2iface.EC2API) error { } res, err := conn.DescribeVpcs(&ec2.DescribeVpcsInput{VpcIds: []*string{aws.String(s.VpcId)}}) - if isAWSErr(err, "InvalidVpcID.NotFound", "") || err != nil { + if IsAWSErr(err, "InvalidVpcID.NotFound", "") || err != nil { return fmt.Errorf("Error retrieving VPC information for vpc_id %s: %s", s.VpcId, err) } diff --git a/builder/amazon/common/step_run_source_instance.go b/builder/amazon/common/step_run_source_instance.go index 7b8610c06..34f0c076a 100644 --- a/builder/amazon/common/step_run_source_instance.go +++ b/builder/amazon/common/step_run_source_instance.go @@ -198,7 +198,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - if isAWSErr(err, "InvalidParameterValue", "iamInstanceProfile") { + if IsAWSErr(err, "InvalidParameterValue", "iamInstanceProfile") { return true } return false @@ -209,7 +209,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa return err }) - if isAWSErr(err, "VPCIdNotSpecified", "No default VPC for this user") && subnetId == "" { + if IsAWSErr(err, "VPCIdNotSpecified", "No default VPC for this user") && subnetId == "" { err := fmt.Errorf("Error launching source instance: a valid Subnet Id was not specified") state.Put("error", err) ui.Error(err.Error()) @@ -247,7 +247,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa var r *ec2.DescribeInstancesOutput err = retry.Config{Tries: 11, ShouldRetry: func(err error) bool { - if isAWSErr(err, "InvalidInstanceID.NotFound", "") { + if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { return true } return false @@ -292,7 +292,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa ec2Tags.Report(ui) // Retry creating tags for about 2.5 minutes err = retry.Config{Tries: 11, ShouldRetry: func(error) bool { - if isAWSErr(err, "InvalidInstanceID.NotFound", "") { + if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { return true } return false diff --git a/builder/amazon/common/step_run_spot_instance.go b/builder/amazon/common/step_run_spot_instance.go index acf6b0478..6ae97985e 100644 --- a/builder/amazon/common/step_run_spot_instance.go +++ b/builder/amazon/common/step_run_spot_instance.go @@ -377,7 +377,7 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag) // Retry creating tags for about 2.5 minutes err = retry.Config{Tries: 11, ShouldRetry: func(error) bool { - if isAWSErr(err, "InvalidInstanceID.NotFound", "") { + if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { return true } return false diff --git a/builder/amazon/common/step_stop_ebs_instance.go b/builder/amazon/common/step_stop_ebs_instance.go index 9c90269d3..794f1c6bb 100644 --- a/builder/amazon/common/step_stop_ebs_instance.go +++ b/builder/amazon/common/step_stop_ebs_instance.go @@ -41,7 +41,7 @@ func (s *StepStopEBSBackedInstance) Run(ctx context.Context, state multistep.Sta // Work around this by retrying a few times, up to about 5 minutes. err := retry.Config{Tries: 6, ShouldRetry: func(error) bool { - if isAWSErr(err, "InvalidInstanceID.NotFound", "") { + if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { return true } return false diff --git a/builder/amazon/ebs/step_create_ami.go b/builder/amazon/ebs/step_create_ami.go index 5a706ad92..22bb2f5ed 100644 --- a/builder/amazon/ebs/step_create_ami.go +++ b/builder/amazon/ebs/step_create_ami.go @@ -7,7 +7,6 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" awscommon "github.com/hashicorp/packer/builder/amazon/common" "github.com/hashicorp/packer/common/random" @@ -57,8 +56,8 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - if err, ok := err.(awserr.Error); ok { - return err.Code() == "InvalidParameterValue" + if awscommon.IsAWSErr(err, "InvalidParameterValue", "Instance is not in state") { + return true } return false }, From 1f3b3f8fd99bc029bebe073c8d0d4115e61334a9 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 21 Jul 2020 15:55:39 -0700 Subject: [PATCH 3/3] change retry func to a 15 min timeout --- builder/amazon/ebs/step_create_ami.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builder/amazon/ebs/step_create_ami.go b/builder/amazon/ebs/step_create_ami.go index 22bb2f5ed..799737617 100644 --- a/builder/amazon/ebs/step_create_ami.go +++ b/builder/amazon/ebs/step_create_ami.go @@ -53,8 +53,12 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi var createResp *ec2.CreateImageOutput var err error + // Create a timeout for the CreateImage call. + timeoutCtx, cancel := context.WithTimeout(ctx, time.Minute*15) + defer cancel() + err = retry.Config{ - Tries: 11, + Tries: 0, ShouldRetry: func(err error) bool { if awscommon.IsAWSErr(err, "InvalidParameterValue", "Instance is not in state") { return true @@ -62,7 +66,7 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi return false }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, - }.Run(ctx, func(ctx context.Context) error { + }.Run(timeoutCtx, func(ctx context.Context) error { createResp, err = ec2conn.CreateImage(createOpts) return err })