From d72040f4fa167e8ac7acc07b9fe19c7448c02678 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 8 Apr 2019 17:57:27 +0200 Subject: [PATCH] move retry code into the common/retry pkg and make retry context aware --- builder/amazon/common/step_create_tags.go | 34 ++++---- builder/amazon/common/step_pre_validate.go | 26 +++--- .../amazon/common/step_run_source_instance.go | 27 ++++--- .../amazon/common/step_run_spot_instance.go | 35 ++++---- .../amazon/common/step_stop_ebs_instance.go | 36 ++++----- .../azure/arm/step_delete_resource_group.go | 12 +-- builder/googlecompute/driver_gce.go | 17 ++-- .../googlecompute/step_wait_startup_script.go | 17 ++-- builder/qemu/step_convert_disk.go | 18 +++-- .../tencentcloud/cvm/step_config_key_pair.go | 19 ++--- .../cvm/step_config_security_group.go | 19 ++--- .../tencentcloud/cvm/step_config_subnet.go | 19 ++--- builder/tencentcloud/cvm/step_config_vpc.go | 19 ++--- builder/virtualbox/common/driver_4_2.go | 19 +++-- .../virtualbox/common/step_remove_devices.go | 20 ++--- common/retry/retry.go | 81 +++++++++++++++++++ common/retry/retry_test.go | 79 ++++++++++++++++++ post-processor/vagrant-cloud/step_upload.go | 19 +++-- provisioner/powershell/provisioner.go | 31 +------ provisioner/powershell/provisioner_test.go | 33 -------- provisioner/shell/provisioner.go | 33 +------- provisioner/windows-restart/provisioner.go | 29 +------ .../windows-restart/provisioner_test.go | 31 ------- provisioner/windows-shell/provisioner.go | 29 +------ provisioner/windows-shell/provisioner_test.go | 34 -------- 25 files changed, 364 insertions(+), 372 deletions(-) create mode 100644 common/retry/retry.go create mode 100644 common/retry/retry_test.go diff --git a/builder/amazon/common/step_create_tags.go b/builder/amazon/common/step_create_tags.go index c7d3fb155..06aa2f356 100644 --- a/builder/amazon/common/step_create_tags.go +++ b/builder/amazon/common/step_create_tags.go @@ -3,12 +3,13 @@ package common import ( "context" "fmt" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -90,17 +91,26 @@ func (s *StepCreateTags) Run(ctx context.Context, state multistep.StateBag) mult snapshotTags.Report(ui) // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { + err = retry.Config{ + Tries: 11, + ShouldRetry: func(error) bool { + if awsErr, ok := err.(awserr.Error); ok { + switch awsErr.Code() { + case "InvalidAMIID.NotFound", "InvalidSnapshot.NotFound": + return true + } + } + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { // Tag images and snapshots _, err := regionConn.CreateTags(&ec2.CreateTagsInput{ Resources: resourceIds, Tags: amiTags, }) - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidAMIID.NotFound" || - awsErr.Code() == "InvalidSnapshot.NotFound" { - return false, nil - } + if err != nil { + return err } // Override tags on snapshots @@ -110,15 +120,7 @@ func (s *StepCreateTags) Run(ctx context.Context, state multistep.StateBag) mult Tags: snapshotTags, }) } - if err == nil { - return true, nil - } - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidSnapshot.NotFound" { - return false, nil - } - } - return true, err + return err }) if err != nil { diff --git a/builder/amazon/common/step_pre_validate.go b/builder/amazon/common/step_pre_validate.go index fca189934..f8ba32b16 100644 --- a/builder/amazon/common/step_pre_validate.go +++ b/builder/amazon/common/step_pre_validate.go @@ -4,11 +4,12 @@ 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" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -31,21 +32,24 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul // time to become eventually-consistent ui.Say("You're using Vault-generated AWS credentials. It may take a " + "few moments for them to become available on AWS. Waiting...") - err := retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { - ec2conn, err := accessconf.NewEC2Connection() - if err != nil { - return true, err - } - _, err = listEC2Regions(ec2conn) - if err != nil { + err := retry.Config{ + Tries: 11, + ShouldRetry: func(err error) bool { if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "AuthFailure" { log.Printf("Waiting for Vault-generated AWS credentials" + " to pass authentication... trying again.") - return false, nil + return true } - return true, err + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + ec2conn, err := accessconf.NewEC2Connection() + if err != nil { + return err } - return true, nil + _, err = listEC2Regions(ec2conn) + return err }) if err != nil { diff --git a/builder/amazon/common/step_run_source_instance.go b/builder/amazon/common/step_run_source_instance.go index 8f06997f8..32b8dce98 100644 --- a/builder/amazon/common/step_run_source_instance.go +++ b/builder/amazon/common/step_run_source_instance.go @@ -6,12 +6,13 @@ import ( "fmt" "io/ioutil" "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" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -230,20 +231,24 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa if s.IsRestricted { ec2Tags.Report(ui) // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { + err = retry.Config{ + Tries: 11, + ShouldRetry: func(error) bool { + if awsErr, ok := err.(awserr.Error); ok { + switch awsErr.Code() { + case "InvalidInstanceID.NotFound": + return true + } + } + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := ec2conn.CreateTags(&ec2.CreateTagsInput{ Tags: ec2Tags, Resources: []*string{instance.InstanceId}, }) - if err == nil { - return true, nil - } - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidInstanceID.NotFound" { - return false, nil - } - } - return true, err + return err }) if err != nil { diff --git a/builder/amazon/common/step_run_spot_instance.go b/builder/amazon/common/step_run_spot_instance.go index f36615073..3bec69747 100644 --- a/builder/amazon/common/step_run_spot_instance.go +++ b/builder/amazon/common/step_run_spot_instance.go @@ -13,7 +13,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -241,13 +241,16 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag) spotTags.Report(ui) if len(spotTags) > 0 && s.SpotTags.IsSet() { - // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { + err = retry.Config{ + Tries: 11, + ShouldRetry: func(error) bool { return false }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := ec2conn.CreateTags(&ec2.CreateTagsInput{ Tags: spotTags, Resources: []*string{spotRequestId}, }) - return true, err + return err }) if err != nil { err := fmt.Errorf("Error tagging spot request: %s", err) @@ -284,20 +287,24 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag) instance := r.Reservations[0].Instances[0] // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func(_ uint) (bool, error) { + err = retry.Config{ + Tries: 11, + ShouldRetry: func(error) bool { + if awsErr, ok := err.(awserr.Error); ok { + switch awsErr.Code() { + case "InvalidInstanceID.NotFound": + return true + } + } + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := ec2conn.CreateTags(&ec2.CreateTagsInput{ Tags: ec2Tags, Resources: []*string{instance.InstanceId}, }) - if err == nil { - return true, nil - } - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidInstanceID.NotFound" { - return false, nil - } - } - return true, err + return err }) if err != nil { diff --git a/builder/amazon/common/step_stop_ebs_instance.go b/builder/amazon/common/step_stop_ebs_instance.go index aee29b432..c48bb87c0 100644 --- a/builder/amazon/common/step_stop_ebs_instance.go +++ b/builder/amazon/common/step_stop_ebs_instance.go @@ -3,10 +3,11 @@ package common import ( "context" "fmt" + "time" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -40,29 +41,26 @@ func (s *StepStopEBSBackedInstance) Run(ctx context.Context, state multistep.Sta // does not exist. // Work around this by retrying a few times, up to about 5 minutes. - err := common.Retry(10, 60, 6, func(i uint) (bool, error) { - ui.Message(fmt.Sprintf("Stopping instance, attempt %d", i+1)) + err := retry.Config{ + Tries: 6, + ShouldRetry: func(error) bool { + if awsErr, ok := err.(awserr.Error); ok { + switch awsErr.Code() { + case "InvalidInstanceID.NotFound": + return true + } + } + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + ui.Message(fmt.Sprintf("Stopping instance")) _, err = ec2conn.StopInstances(&ec2.StopInstancesInput{ InstanceIds: []*string{instance.InstanceId}, }) - if err == nil { - // success - return true, nil - } - - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidInstanceID.NotFound" { - ui.Message(fmt.Sprintf( - "Error stopping instance; will retry ..."+ - "Error: %s", err)) - // retry - return false, nil - } - } - // errored, but not in expected way. Don't want to retry - return true, err + return err }) if err != nil { diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index 29e881f38..ec62870ae 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -3,9 +3,10 @@ package arm import ( "context" "fmt" + "time" "github.com/hashicorp/packer/builder/azure/common/constants" - retry "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -94,17 +95,18 @@ func (s *StepDeleteResourceGroup) deleteDeploymentResources(ctx context.Context, resourceType, resourceName)) - err := retry.Retry(10, 600, 10, func(attempt uint) (bool, error) { + err := retry.Config{ + Tries: 10, + RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 600 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { err := deleteResource(ctx, s.client, resourceType, resourceName, resourceGroupName) if err != nil { s.reportIfError(err, resourceName) - return false, nil } - - return true, nil + return err }) if err = deploymentOperations.Next(); err != nil { diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 61bb293b7..153e7ac20 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -1,6 +1,7 @@ package googlecompute import ( + "context" "crypto/rand" "crypto/rsa" "crypto/sha1" @@ -15,7 +16,7 @@ import ( compute "google.golang.org/api/compute/v1" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/useragent" "github.com/hashicorp/packer/packer" @@ -608,14 +609,18 @@ type stateRefreshFunc func() (string, error) // waitForState will spin in a loop forever waiting for state to // reach a certain target. func waitForState(errCh chan<- error, target string, refresh stateRefreshFunc) error { - err := common.Retry(2, 2, 0, func(_ uint) (bool, error) { + ctx := context.TODO() + err := retry.Config{ + RetryDelay: (&retry.Backoff{InitialBackoff: 2 * time.Second, MaxBackoff: 2 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { state, err := refresh() if err != nil { - return false, err - } else if state == target { - return true, nil + return err } - return false, nil + if state == target { + return nil + } + return fmt.Errorf("retrying for state %s, got %s", target, state) }) errCh <- err return err diff --git a/builder/googlecompute/step_wait_startup_script.go b/builder/googlecompute/step_wait_startup_script.go index e68fa1d2b..ede82ba8c 100644 --- a/builder/googlecompute/step_wait_startup_script.go +++ b/builder/googlecompute/step_wait_startup_script.go @@ -4,8 +4,9 @@ import ( "context" "errors" "fmt" + "time" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -23,26 +24,32 @@ func (s *StepWaitStartupScript) Run(ctx context.Context, state multistep.StateBa ui.Say("Waiting for any running startup script to finish...") // Keep checking the serial port output to see if the startup script is done. - err := common.Retry(10, 60, 0, func(_ uint) (bool, error) { + err := retry.Config{ + ShouldRetry: func(error) bool { + return true + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { status, err := driver.GetInstanceMetadata(config.Zone, instanceName, StartupScriptStatusKey) if err != nil { err := fmt.Errorf("Error getting startup script status: %s", err) - return false, err + return err } if status == StartupScriptStatusError { err = errors.New("Startup script error.") - return false, err + return err } done := status == StartupScriptStatusDone if !done { ui.Say("Startup script not finished yet. Waiting...") + return errors.New("Startup script not done.") } - return done, nil + return nil }) if err != nil { diff --git a/builder/qemu/step_convert_disk.go b/builder/qemu/step_convert_disk.go index 8fe2c22b9..067722be1 100644 --- a/builder/qemu/step_convert_disk.go +++ b/builder/qemu/step_convert_disk.go @@ -5,8 +5,10 @@ import ( "fmt" "path/filepath" "strings" + "time" "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -50,16 +52,18 @@ func (s *stepConvertDisk) Run(ctx context.Context, state multistep.StateBag) mul ui.Say("Converting hard drive...") // Retry the conversion a few times in case it takes the qemu process a // moment to release the lock - err := common.Retry(1, 10, 10, func(_ uint) (bool, error) { - if err := driver.QemuImg(command...); err != nil { + err := retry.Config{ + Tries: 10, + ShouldRetry: func(err error) bool { if strings.Contains(err.Error(), `Failed to get shared "write" lock`) { ui.Say("Error getting file lock for conversion; retrying...") - return false, nil + return true } - err = fmt.Errorf("Error converting hard drive: %s", err) - return true, err - } - return true, nil + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 1 * time.Second, MaxBackoff: 10 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + return driver.QemuImg(command...) }) if err != nil { diff --git a/builder/tencentcloud/cvm/step_config_key_pair.go b/builder/tencentcloud/cvm/step_config_key_pair.go index 9f1569f68..a1768f67d 100644 --- a/builder/tencentcloud/cvm/step_config_key_pair.go +++ b/builder/tencentcloud/cvm/step_config_key_pair.go @@ -6,9 +6,9 @@ import ( "io/ioutil" "os" "runtime" - "strings" + "time" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -102,6 +102,7 @@ func (s *stepConfigKeyPair) Cleanup(state multistep.StateBag) { if s.Comm.SSHPrivateKeyFile != "" || (s.Comm.SSHKeyPairName == "" && s.keyID == "") { return } + ctx := context.TODO() client := state.Get("cvm_client").(*cvm.Client) ui := state.Get("ui").(packer.Ui) @@ -109,16 +110,12 @@ func (s *stepConfigKeyPair) Cleanup(state multistep.StateBag) { ui.Say("Deleting temporary keypair...") req := cvm.NewDeleteKeyPairsRequest() req.KeyIds = []*string{&s.keyID} - err := common.Retry(5, 5, 60, func(u uint) (bool, error) { + err := retry.Config{ + Tries: 60, + RetryDelay: (&retry.Backoff{InitialBackoff: 5 * time.Second, MaxBackoff: 5 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := client.DeleteKeyPairs(req) - if err == nil { - return true, nil - } - if strings.Index(err.Error(), "NotSupported") != -1 { - return false, nil - } else { - return false, err - } + return err }) if err != nil { ui.Error(fmt.Sprintf( diff --git a/builder/tencentcloud/cvm/step_config_security_group.go b/builder/tencentcloud/cvm/step_config_security_group.go index d3206db68..9244f4cc6 100644 --- a/builder/tencentcloud/cvm/step_config_security_group.go +++ b/builder/tencentcloud/cvm/step_config_security_group.go @@ -2,11 +2,11 @@ package cvm import ( "context" + "time" "fmt" - "strings" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/pkg/errors" @@ -102,22 +102,19 @@ func (s *stepConfigSecurityGroup) Cleanup(state multistep.StateBag) { if !s.isCreate { return } + ctx := context.TODO() vpcClient := state.Get("vpc_client").(*vpc.Client) ui := state.Get("ui").(packer.Ui) MessageClean(state, "VPC") req := vpc.NewDeleteSecurityGroupRequest() req.SecurityGroupId = &s.SecurityGroupId - err := common.Retry(5, 5, 60, func(u uint) (bool, error) { + err := retry.Config{ + Tries: 60, + RetryDelay: (&retry.Backoff{InitialBackoff: 5 * time.Second, MaxBackoff: 5 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := vpcClient.DeleteSecurityGroup(req) - if err == nil { - return true, nil - } - if strings.Index(err.Error(), "ResourceInUse") != -1 { - return false, nil - } else { - return false, err - } + return err }) if err != nil { ui.Error(fmt.Sprintf("delete security group(%s) failed: %s, you need to delete it by hand", diff --git a/builder/tencentcloud/cvm/step_config_subnet.go b/builder/tencentcloud/cvm/step_config_subnet.go index 4be31c3cb..a200591da 100644 --- a/builder/tencentcloud/cvm/step_config_subnet.go +++ b/builder/tencentcloud/cvm/step_config_subnet.go @@ -3,9 +3,9 @@ package cvm import ( "context" "fmt" - "strings" + "time" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/pkg/errors" @@ -77,6 +77,7 @@ func (s *stepConfigSubnet) Cleanup(state multistep.StateBag) { if !s.isCreate { return } + ctx := context.TODO() vpcClient := state.Get("vpc_client").(*vpc.Client) ui := state.Get("ui").(packer.Ui) @@ -84,16 +85,12 @@ func (s *stepConfigSubnet) Cleanup(state multistep.StateBag) { MessageClean(state, "SUBNET") req := vpc.NewDeleteSubnetRequest() req.SubnetId = &s.SubnetId - err := common.Retry(5, 5, 60, func(u uint) (bool, error) { + err := retry.Config{ + Tries: 60, + RetryDelay: (&retry.Backoff{InitialBackoff: 5 * time.Second, MaxBackoff: 5 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := vpcClient.DeleteSubnet(req) - if err == nil { - return true, nil - } - if strings.Index(err.Error(), "ResourceInUse") != -1 { - return false, nil - } else { - return false, err - } + return err }) if err != nil { ui.Error(fmt.Sprintf("delete subnet(%s) failed: %s, you need to delete it by hand", diff --git a/builder/tencentcloud/cvm/step_config_vpc.go b/builder/tencentcloud/cvm/step_config_vpc.go index e34991aec..46630278a 100644 --- a/builder/tencentcloud/cvm/step_config_vpc.go +++ b/builder/tencentcloud/cvm/step_config_vpc.go @@ -3,9 +3,9 @@ package cvm import ( "context" "fmt" - "strings" + "time" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/pkg/errors" @@ -66,6 +66,7 @@ func (s *stepConfigVPC) Cleanup(state multistep.StateBag) { if !s.isCreate { return } + ctx := context.TODO() vpcClient := state.Get("vpc_client").(*vpc.Client) ui := state.Get("ui").(packer.Ui) @@ -73,16 +74,12 @@ func (s *stepConfigVPC) Cleanup(state multistep.StateBag) { MessageClean(state, "VPC") req := vpc.NewDeleteVpcRequest() req.VpcId = &s.VpcId - err := common.Retry(5, 5, 60, func(u uint) (bool, error) { + err := retry.Config{ + Tries: 60, + RetryDelay: (&retry.Backoff{InitialBackoff: 5 * time.Second, MaxBackoff: 5 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { _, err := vpcClient.DeleteVpc(req) - if err == nil { - return true, nil - } - if strings.Index(err.Error(), "ResourceInUse") != -1 { - return false, nil - } else { - return false, err - } + return err }) if err != nil { ui.Error(fmt.Sprintf("delete vpc(%s) failed: %s, you need to delete it by hand", diff --git a/builder/virtualbox/common/driver_4_2.go b/builder/virtualbox/common/driver_4_2.go index 17e2499de..fc065a678 100644 --- a/builder/virtualbox/common/driver_4_2.go +++ b/builder/virtualbox/common/driver_4_2.go @@ -2,6 +2,7 @@ package common import ( "bytes" + "context" "fmt" "log" "os/exec" @@ -11,8 +12,7 @@ import ( "time" versionUtil "github.com/hashicorp/go-version" - - packer "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" ) type VBox42Driver struct { @@ -64,14 +64,13 @@ func (d *VBox42Driver) CreateSCSIController(vmName string, name string) error { } func (d *VBox42Driver) Delete(name string) error { - return packer.Retry(1, 1, 5, func(i uint) (bool, error) { - if err := d.VBoxManage("unregistervm", name, "--delete"); err != nil { - if i+1 == 5 { - return false, err - } - return false, nil - } - return true, nil + ctx := context.TODO() + return retry.Config{ + Tries: 5, + RetryDelay: (&retry.Backoff{InitialBackoff: 1 * time.Second, MaxBackoff: 1 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + err := d.VBoxManage("unregistervm", name, "--delete") + return err }) } diff --git a/builder/virtualbox/common/step_remove_devices.go b/builder/virtualbox/common/step_remove_devices.go index 5478a56da..de6b70853 100644 --- a/builder/virtualbox/common/step_remove_devices.go +++ b/builder/virtualbox/common/step_remove_devices.go @@ -4,8 +4,9 @@ import ( "context" "fmt" "log" + "time" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -46,25 +47,26 @@ func (s *StepRemoveDevices) Run(ctx context.Context, state multistep.StateBag) m return multistep.ActionHalt } - var vboxErr error // Retry for 10 minutes to remove the floppy controller. log.Printf("Trying for 10 minutes to remove floppy controller.") - err := common.Retry(15, 15, 40, func(_ uint) (bool, error) { + err := retry.Config{ + Tries: 40, + RetryDelay: (&retry.Backoff{InitialBackoff: 15 * time.Second, MaxBackoff: 15 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { // Don't forget to remove the floppy controller as well command = []string{ "storagectl", vmName, "--name", "Floppy Controller", "--remove", } - vboxErr = driver.VBoxManage(command...) - if vboxErr != nil { + err := driver.VBoxManage(command...) + if err != nil { log.Printf("Error removing floppy controller. Retrying.") - return false, nil } - return true, nil + return err }) - if err == common.RetryExhaustedError { - err := fmt.Errorf("Error removing floppy controller: %s", vboxErr) + if err != nil { + err := fmt.Errorf("Error removing floppy controller: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt diff --git a/common/retry/retry.go b/common/retry/retry.go new file mode 100644 index 000000000..2e0cdc0b3 --- /dev/null +++ b/common/retry/retry.go @@ -0,0 +1,81 @@ +package retry + +import ( + "context" + "fmt" + "log" + "time" +) + +// Config represents a retry config +type Config struct { + // The operation will be retried until StartTimeout has elapsed. 0 means + // forever. + StartTimeout time.Duration + + // RetryDelay gives the time elapsed after a failure and before we try + // again. Returns 2s by default. + RetryDelay func() time.Duration + + // Max number of retries, 0 means infinite + Tries int + + // ShouldRetry tells wether error should be retried. Nil defaults to always + // true. + ShouldRetry func(error) bool +} + +// Run fn until context is cancelled up until StartTimeout time has passed. +func (cfg Config) Run(ctx context.Context, fn func(context.Context) error) error { + retryDelay := func() time.Duration { return 2 * time.Second } + if cfg.RetryDelay != nil { + retryDelay = cfg.RetryDelay + } + shouldRetry := func(error) bool { return true } + if cfg.ShouldRetry != nil { + shouldRetry = cfg.ShouldRetry + } + var startTimeout <-chan time.Time // nil chans never unlock ! + if cfg.StartTimeout != 0 { + startTimeout = time.After(cfg.StartTimeout) + } + + for try := 0; ; try++ { + var err error + if cfg.Tries != 0 && try == cfg.Tries { + return err + } + if err = fn(ctx); err == nil { + return nil + } + if !shouldRetry(err) { + return err + } + + log.Print(fmt.Errorf("Retryable error: %s", err)) + + select { + case <-ctx.Done(): + return err + case <-startTimeout: + return err + default: + time.Sleep(retryDelay()) + } + } +} + +type Backoff struct { + InitialBackoff time.Duration + MaxBackoff time.Duration + Multiplier float64 +} + +func (lb *Backoff) Linear() time.Duration { + wait := lb.InitialBackoff + lb.InitialBackoff = time.Duration(lb.Multiplier * float64(lb.InitialBackoff)) + if lb.MaxBackoff != 0 && lb.InitialBackoff > lb.MaxBackoff { + lb.InitialBackoff = lb.MaxBackoff + } + return wait +} diff --git a/common/retry/retry_test.go b/common/retry/retry_test.go new file mode 100644 index 000000000..334707c0b --- /dev/null +++ b/common/retry/retry_test.go @@ -0,0 +1,79 @@ +package retry + +import ( + "context" + "errors" + "testing" + "time" +) + +func success(context.Context) error { return nil } + +func wait(ctx context.Context) error { + <-ctx.Done() + return ctx.Err() +} + +var failErr = errors.New("woops !") + +func fail(context.Context) error { return failErr } + +func TestConfig_Run(t *testing.T) { + cancelledCtx, cancel := context.WithCancel(context.Background()) + cancel() + type fields struct { + StartTimeout time.Duration + RetryDelay func() time.Duration + } + type args struct { + ctx context.Context + fn func(context.Context) error + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + {"success", + fields{StartTimeout: time.Second, RetryDelay: nil}, + args{context.Background(), success}, + nil}, + {"context cancelled", + fields{StartTimeout: time.Second, RetryDelay: nil}, + args{cancelledCtx, wait}, + context.Canceled}, + {"timeout", + fields{StartTimeout: 20 * time.Millisecond, RetryDelay: func() time.Duration { return 10 * time.Millisecond }}, + args{cancelledCtx, fail}, + failErr}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := Config{ + StartTimeout: tt.fields.StartTimeout, + RetryDelay: tt.fields.RetryDelay, + } + if err := cfg.Run(tt.args.ctx, tt.args.fn); err != tt.wantErr { + t.Fatalf("Config.Run() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestBackoff_Linear(t *testing.T) { + b := Backoff{ + InitialBackoff: 2 * time.Minute, + Multiplier: 2, + } + + linear := (&b).Linear + + if linear() != 2*time.Minute { + t.Fatal("first backoff should be 2 minutes") + } + + if linear() != 4*time.Minute { + t.Fatal("second backoff should be 4 minutes") + } +} diff --git a/post-processor/vagrant-cloud/step_upload.go b/post-processor/vagrant-cloud/step_upload.go index 9d8a084f3..1eb046ded 100644 --- a/post-processor/vagrant-cloud/step_upload.go +++ b/post-processor/vagrant-cloud/step_upload.go @@ -4,8 +4,9 @@ import ( "context" "fmt" "log" + "time" - "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -25,23 +26,27 @@ func (s *stepUpload) Run(ctx context.Context, state multistep.StateBag) multiste "Depending on your internet connection and the size of the box,\n" + "this may take some time") - err := common.Retry(10, 10, 3, func(i uint) (bool, error) { - ui.Message(fmt.Sprintf("Uploading box, attempt %d", i+1)) + err := retry.Config{ + Tries: 3, + RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 10 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + ui.Message(fmt.Sprintf("Uploading box")) resp, err := client.Upload(artifactFilePath, url) if err != nil { ui.Message(fmt.Sprintf( "Error uploading box! Will retry in 10 seconds. Error: %s", err)) - return false, nil + return err } if resp.StatusCode != 200 { - log.Printf("bad HTTP status: %d", resp.StatusCode) + err := fmt.Errorf("bad HTTP status: %d", resp.StatusCode) + log.Print(err) ui.Message(fmt.Sprintf( "Error uploading box! Will retry in 10 seconds. Status: %d", resp.StatusCode)) - return false, nil + return err } - return true, nil + return err }) if err != nil { diff --git a/provisioner/powershell/provisioner.go b/provisioner/powershell/provisioner.go index 04e301007..2da03ffa2 100644 --- a/provisioner/powershell/provisioner.go +++ b/provisioner/powershell/provisioner.go @@ -14,6 +14,7 @@ import ( "time" "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/common/shell" "github.com/hashicorp/packer/common/uuid" commonhelper "github.com/hashicorp/packer/helper/common" @@ -253,7 +254,7 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C // that the upload succeeded, a restart is initiated, and then the // command is executed but the file doesn't exist any longer. var cmd *packer.RemoteCmd - err = p.retryable(func() error { + retry.Config{StartTimeout: p.config.StartRetryTimeout}.Run(ctx, func(ctx context.Context) error { if _, err := f.Seek(0, 0); err != nil { return err } @@ -285,31 +286,6 @@ func (p *Provisioner) Cancel() { os.Exit(0) } -// retryable will retry the given function over and over until a non-error is -// returned. -func (p *Provisioner) retryable(f func() error) error { - startTimeout := time.After(p.config.StartRetryTimeout) - for { - var err error - if err = f(); err == nil { - return nil - } - - // Create an error and log it - err = fmt.Errorf("Retryable error: %s", err) - log.Print(err.Error()) - - // Check if we timed out, otherwise we retry. It is safe to retry - // since the only error case above is if the command failed to START. - select { - case <-startTimeout: - return err - default: - time.Sleep(retryableSleep) - } - } -} - // Environment variables required within the remote environment are uploaded // within a PS script and then enabled by 'dot sourcing' the script // immediately prior to execution of the main command @@ -387,13 +363,14 @@ func (p *Provisioner) createFlattenedEnvVars(elevated bool) (flattened string) { } func (p *Provisioner) uploadEnvVars(flattenedEnvVars string) (err error) { + ctx := context.TODO() // Upload all env vars to a powershell script on the target build file // system. Do this in the context of a single retryable function so that // we gracefully handle any errors created by transient conditions such as // a system restart envVarReader := strings.NewReader(flattenedEnvVars) log.Printf("Uploading env vars to %s", p.config.RemoteEnvVarPath) - err = p.retryable(func() error { + err = retry.Config{StartTimeout: p.config.StartRetryTimeout}.Run(ctx, func(context.Context) error { if err := p.communicator.Upload(p.config.RemoteEnvVarPath, envVarReader, nil); err != nil { return fmt.Errorf("Error uploading ps script containing env vars: %s", err) } diff --git a/provisioner/powershell/provisioner_test.go b/provisioner/powershell/provisioner_test.go index c2252ac12..8d48fa2ee 100644 --- a/provisioner/powershell/provisioner_test.go +++ b/provisioner/powershell/provisioner_test.go @@ -3,14 +3,11 @@ package powershell import ( "bytes" "context" - "errors" - "fmt" "io/ioutil" "os" "regexp" "strings" "testing" - "time" "github.com/hashicorp/packer/packer" ) @@ -643,36 +640,6 @@ func TestProvision_uploadEnvVars(t *testing.T) { } } -func TestRetryable(t *testing.T) { - config := testConfig() - - count := 0 - retryMe := func() error { - t.Logf("RetryMe, attempt number %d", count) - if count == 2 { - return nil - } - count++ - return errors.New(fmt.Sprintf("Still waiting %d more times...", 2-count)) - } - retryableSleep = 50 * time.Millisecond - p := new(Provisioner) - p.config.StartRetryTimeout = 155 * time.Millisecond - err := p.Prepare(config) - err = p.retryable(retryMe) - if err != nil { - t.Fatalf("should not have error retrying function") - } - - count = 0 - p.config.StartRetryTimeout = 10 * time.Millisecond - err = p.Prepare(config) - err = p.retryable(retryMe) - if err == nil { - t.Fatalf("should have error retrying function") - } -} - func TestCancel(t *testing.T) { // Don't actually call Cancel() as it performs an os.Exit(0) // which kills the 'go test' tool diff --git a/provisioner/shell/provisioner.go b/provisioner/shell/provisioner.go index 6f6c0f46d..168c1c0e4 100644 --- a/provisioner/shell/provisioner.go +++ b/provisioner/shell/provisioner.go @@ -16,6 +16,7 @@ import ( "time" "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/common/shell" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" @@ -237,7 +238,7 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C // upload the var file var cmd *packer.RemoteCmd - err = p.retryable(func() error { + err = retry.Config{StartTimeout: p.config.startRetryTimeout}.Run(ctx, func(ctx context.Context) error { if _, err := tf.Seek(0, 0); err != nil { return err } @@ -297,7 +298,7 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C // and then the command is executed but the file doesn't exist // any longer. var cmd *packer.RemoteCmd - err = p.retryable(func() error { + err = retry.Config{StartTimeout: p.config.startRetryTimeout}.Run(ctx, func(ctx context.Context) error { if _, err := f.Seek(0, 0); err != nil { return err } @@ -372,7 +373,7 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C func (p *Provisioner) cleanupRemoteFile(path string, comm packer.Communicator) error { ctx := context.TODO() - err := p.retryable(func() error { + err := retry.Config{StartTimeout: p.config.startRetryTimeout}.Run(ctx, func(ctx context.Context) error { cmd := &packer.RemoteCmd{ Command: fmt.Sprintf("rm -f %s", path), } @@ -401,32 +402,6 @@ func (p *Provisioner) cleanupRemoteFile(path string, comm packer.Communicator) e return nil } -// retryable will retry the given function over and over until a -// non-error is returned. -func (p *Provisioner) retryable(f func() error) error { - startTimeout := time.After(p.config.startRetryTimeout) - for { - var err error - if err = f(); err == nil { - return nil - } - - // Create an error and log it - err = fmt.Errorf("Retryable error: %s", err) - log.Print(err.Error()) - - // Check if we timed out, otherwise we retry. It is safe to - // retry since the only error case above is if the command - // failed to START. - select { - case <-startTimeout: - return err - default: - time.Sleep(2 * time.Second) - } - } -} - func (p *Provisioner) escapeEnvVars() ([]string, map[string]string) { envVars := make(map[string]string) diff --git a/provisioner/windows-restart/provisioner.go b/provisioner/windows-restart/provisioner.go index 85d3bb15b..fba55f8cb 100644 --- a/provisioner/windows-restart/provisioner.go +++ b/provisioner/windows-restart/provisioner.go @@ -12,6 +12,7 @@ import ( "time" "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -104,7 +105,7 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C var cmd *packer.RemoteCmd command := p.config.RestartCommand - err := p.retryable(func() error { + err := retry.Config{StartTimeout: p.config.RestartTimeout}.Run(ctx, func(context.Context) error { cmd = &packer.RemoteCmd{Command: command} return cmd.RunWithUi(ctx, comm, ui) }) @@ -286,29 +287,3 @@ var waitForCommunicator = func(ctx context.Context, p *Provisioner) error { return nil } - -// retryable will retry the given function over and over until a -// non-error is returned. -func (p *Provisioner) retryable(f func() error) error { - startTimeout := time.After(p.config.RestartTimeout) - for { - var err error - if err = f(); err == nil { - return nil - } - - // Create an error and log it - err = fmt.Errorf("Retryable error: %s", err) - log.Print(err.Error()) - - // Check if we timed out, otherwise we retry. It is safe to - // retry since the only error case above is if the command - // failed to START. - select { - case <-startTimeout: - return err - default: - time.Sleep(retryableSleep) - } - } -} diff --git a/provisioner/windows-restart/provisioner_test.go b/provisioner/windows-restart/provisioner_test.go index 035f5e246..ffe9c48a2 100644 --- a/provisioner/windows-restart/provisioner_test.go +++ b/provisioner/windows-restart/provisioner_test.go @@ -3,7 +3,6 @@ package restart import ( "bytes" "context" - "errors" "fmt" "testing" "time" @@ -305,36 +304,6 @@ func TestProvision_waitForCommunicatorWithCancel(t *testing.T) { } } -func TestRetryable(t *testing.T) { - config := testConfig() - - count := 0 - retryMe := func() error { - t.Logf("RetryMe, attempt number %d", count) - if count == 2 { - return nil - } - count++ - return errors.New(fmt.Sprintf("Still waiting %d more times...", 2-count)) - } - retryableSleep = 50 * time.Millisecond - p := new(Provisioner) - p.config.RestartTimeout = 155 * time.Millisecond - err := p.Prepare(config) - err = p.retryable(retryMe) - if err != nil { - t.Fatalf("should not have error retrying function") - } - - count = 0 - p.config.RestartTimeout = 10 * time.Millisecond - err = p.Prepare(config) - err = p.retryable(retryMe) - if err == nil { - t.Fatalf("should have error retrying function") - } -} - func TestProvision_Cancel(t *testing.T) { config := testConfig() diff --git a/provisioner/windows-shell/provisioner.go b/provisioner/windows-shell/provisioner.go index 4eb8482b7..e3496fbd1 100644 --- a/provisioner/windows-shell/provisioner.go +++ b/provisioner/windows-shell/provisioner.go @@ -14,6 +14,7 @@ import ( "time" "github.com/hashicorp/packer/common" + "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/common/shell" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" @@ -204,7 +205,7 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C // and then the command is executed but the file doesn't exist // any longer. var cmd *packer.RemoteCmd - err = p.retryable(func() error { + err = retry.Config{StartTimeout: p.config.StartRetryTimeout}.Run(ctx, func(ctx context.Context) error { if _, err := f.Seek(0, 0); err != nil { return err } @@ -231,32 +232,6 @@ func (p *Provisioner) Provision(ctx context.Context, ui packer.Ui, comm packer.C return nil } -// retryable will retry the given function over and over until a -// non-error is returned. -func (p *Provisioner) retryable(f func() error) error { - startTimeout := time.After(p.config.StartRetryTimeout) - for { - var err error - if err = f(); err == nil { - return nil - } - - // Create an error and log it - err = fmt.Errorf("Retryable error: %s", err) - log.Print(err.Error()) - - // Check if we timed out, otherwise we retry. It is safe to - // retry since the only error case above is if the command - // failed to START. - select { - case <-startTimeout: - return err - default: - time.Sleep(retryableSleep) - } - } -} - func (p *Provisioner) createFlattenedEnvVars() (flattened string) { flattened = "" envVars := make(map[string]string) diff --git a/provisioner/windows-shell/provisioner_test.go b/provisioner/windows-shell/provisioner_test.go index d5500d522..32ec528e1 100644 --- a/provisioner/windows-shell/provisioner_test.go +++ b/provisioner/windows-shell/provisioner_test.go @@ -3,14 +3,11 @@ package shell import ( "bytes" "context" - "errors" - "fmt" "io/ioutil" "log" "os" "strings" "testing" - "time" "github.com/hashicorp/packer/packer" ) @@ -431,37 +428,6 @@ func TestProvisioner_createFlattenedEnvVars_windows(t *testing.T) { } } } - -func TestRetryable(t *testing.T) { - config := testConfig() - - count := 0 - retryMe := func() error { - log.Printf("RetryMe, attempt number %d", count) - if count == 2 { - return nil - } - count++ - return errors.New(fmt.Sprintf("Still waiting %d more times...", 2-count)) - } - retryableSleep = 50 * time.Millisecond - p := new(Provisioner) - p.config.StartRetryTimeout = 155 * time.Millisecond - err := p.Prepare(config) - err = p.retryable(retryMe) - if err != nil { - t.Fatalf("should not have error retrying function") - } - - count = 0 - p.config.StartRetryTimeout = 10 * time.Millisecond - err = p.Prepare(config) - err = p.retryable(retryMe) - if err == nil { - t.Fatalf("should have error retrying function") - } -} - func TestCancel(t *testing.T) { // Don't actually call Cancel() as it performs an os.Exit(0) // which kills the 'go test' tool