diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e233b44f..8dc7cadfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ IMPROVEMENTS: * core: Plugins communicate over a single TCP connection per plugin now, instead of sometimes dozens. Performance around plugin communication dramatically increased. +* builder/amazon/all: Copying AMIs to multiple regions now happens + in parallel. [GH-495] * provisioner/puppet-masterless: Can now specify a `manifest_dir` to upload manifests to the remote machine for imports. [GH-655] diff --git a/builder/amazon/common/step_ami_region_copy.go b/builder/amazon/common/step_ami_region_copy.go index 5ee486522..2f5442c3c 100644 --- a/builder/amazon/common/step_ami_region_copy.go +++ b/builder/amazon/common/step_ami_region_copy.go @@ -6,6 +6,7 @@ import ( "github.com/mitchellh/goamz/ec2" "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" + "sync" ) type StepAMIRegionCopy struct { @@ -23,41 +24,37 @@ func (s *StepAMIRegionCopy) Run(state multistep.StateBag) multistep.StepAction { } ui.Say(fmt.Sprintf("Copying AMI (%s) to other regions...", ami)) + + var lock sync.Mutex + var wg sync.WaitGroup + errs := new(packer.MultiError) for _, region := range s.Regions { + wg.Add(1) ui.Message(fmt.Sprintf("Copying to: %s", region)) - // Connect to the region where the AMI will be copied to - regionconn := ec2.New(ec2conn.Auth, aws.Regions[region]) - resp, err := regionconn.CopyImage(&ec2.CopyImage{ - SourceRegion: ec2conn.Region.Name, - SourceImageId: ami, - }) + go func(region string) { + defer wg.Done() + id, err := amiRegionCopy(state, ec2conn.Auth, ami, + aws.Regions[region], ec2conn.Region) - if err != nil { - err := fmt.Errorf("Error Copying AMI (%s) to region (%s): %s", ami, region, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } + lock.Lock() + defer lock.Unlock() + amis[region] = id + if err != nil { + errs = packer.MultiErrorAppend(errs, err) + } + }(region) + } - stateChange := StateChangeConf{ - Conn: regionconn, - Pending: []string{"pending"}, - Target: "available", - Refresh: AMIStateRefreshFunc(regionconn, resp.ImageId), - StepState: state, - } + // TODO(mitchellh): Wait but also allow for cancels to go through... + ui.Message("Waiting for all copies to complete...") + wg.Wait() - ui.Say(fmt.Sprintf("Waiting for AMI (%s) in region (%s) to become ready...", - resp.ImageId, region)) - if _, err := WaitForState(&stateChange); err != nil { - err := fmt.Errorf("Error waiting for AMI (%s) in region (%s): %s", resp.ImageId, region, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } - - amis[region] = resp.ImageId + // If there were errors, show them + if len(errs.Errors) > 0 { + state.Put("error", errs) + ui.Error(errs.Error()) + return multistep.ActionHalt } state.Put("amis", amis) @@ -67,3 +64,36 @@ func (s *StepAMIRegionCopy) Run(state multistep.StateBag) multistep.StepAction { func (s *StepAMIRegionCopy) Cleanup(state multistep.StateBag) { // No cleanup... } + +// amiRegionCopy does a copy for the given AMI to the target region and +// returns the resulting ID or error. +func amiRegionCopy(state multistep.StateBag, auth aws.Auth, imageId string, + target aws.Region, source aws.Region) (string, error) { + + // Connect to the region where the AMI will be copied to + regionconn := ec2.New(auth, target) + resp, err := regionconn.CopyImage(&ec2.CopyImage{ + SourceRegion: source.Name, + SourceImageId: imageId, + }) + + if err != nil { + return "", fmt.Errorf("Error Copying AMI (%s) to region (%s): %s", + imageId, target, err) + } + + stateChange := StateChangeConf{ + Conn: regionconn, + Pending: []string{"pending"}, + Target: "available", + Refresh: AMIStateRefreshFunc(regionconn, resp.ImageId), + StepState: state, + } + + if _, err := WaitForState(&stateChange); err != nil { + return "", fmt.Errorf("Error waiting for AMI (%s) in region (%s): %s", + resp.ImageId, target, err) + } + + return resp.ImageId, nil +} diff --git a/test/builder_amazon_ebs.bats b/test/builder_amazon_ebs.bats index e5f4fb45d..7dd17acbd 100755 --- a/test/builder_amazon_ebs.bats +++ b/test/builder_amazon_ebs.bats @@ -7,11 +7,30 @@ load test_helper fixtures amazon-ebs +# This counts how many AMIs were copied to another region +aws_ami_region_copy_count() { + aws ec2 describe-images --region $1 --owners self --output text \ + --filters 'Name=tag:packer-id,Values=ami_region_copy' \ + --query "Images[*].ImageId" \ + | wc -l +} + teardown() { - aws_ami_cleanup + aws_ami_cleanup 'us-east-1' + aws_ami_cleanup 'us-west-1' + aws_ami_cleanup 'us-west-2' } @test "amazon-ebs: build minimal.json" { run packer build $FIXTURE_ROOT/minimal.json [ "$status" -eq 0 ] } + +# @unit-testable +@test "amazon-ebs: AMI region copy" { + run packer build $FIXTURE_ROOT/ami_region_copy.json + [ "$status" -eq 0 ] + [ "$(aws_ami_region_copy_count 'us-east-1')" -eq "1" ] + [ "$(aws_ami_region_copy_count 'us-west-1')" -eq "1" ] + [ "$(aws_ami_region_copy_count 'us-west-2')" -eq "1" ] +} diff --git a/test/fixtures/amazon-ebs/ami_region_copy.json b/test/fixtures/amazon-ebs/ami_region_copy.json new file mode 100644 index 000000000..6f591b6bd --- /dev/null +++ b/test/fixtures/amazon-ebs/ami_region_copy.json @@ -0,0 +1,15 @@ +{ + "builders": [{ + "type": "amazon-ebs", + "ami_name": "packer-test {{timestamp}}", + "instance_type": "m1.small", + "region": "us-east-1", + "ssh_username": "ubuntu", + "source_ami": "ami-0568456c", + "tags": { + "packer-test": "true", + "packer-id": "ami_region_copy" + }, + "ami_regions": ["us-west-1", "us-west-2"] + }] +} diff --git a/test/test_helper.bash b/test/test_helper.bash index c16c737c1..bb6466ac8 100644 --- a/test/test_helper.bash +++ b/test/test_helper.bash @@ -39,7 +39,9 @@ fixtures() { # This deletes any AMIs with a tag "packer-test" of "true" aws_ami_cleanup() { - aws ec2 describe-images --owners self --output json --filters 'Name=tag:packer-test,Values=true' \ + local region=${1:-us-east-1} + aws ec2 describe-images --region ${region} --owners self --output json \ + --filters 'Name=tag:packer-test,Values=true' \ | jq -r -M '.Images[]["ImageId"]' \ - | xargs -n1 aws ec2 deregister-image --image-id + | xargs -n1 aws ec2 deregister-image --region ${region} --image-id }