fix copy logic and tests

builder/amazon/common/step_ami_region_copy.go

@@ -33,19 +33,25 @@ func (s *StepAMIRegionCopy) Run(ctx context.Context, state multistep.StateBag) m
 
 	ami := amis[s.OriginalRegion]
 	// Always copy back into original region to preserve the ami name
-	s.toDelete = ami
-	if !s.AMISkipBuildRegion {
-		s.Regions = append(s.Regions, s.OriginalRegion)
+	if s.EncryptBootVolume != nil || s.AMISkipBuildRegion {
+		// if we haven't specificed encryption and we aren't skipping the save
+		// to the build region, we don't have anything to delete
+		s.toDelete = ami
 	}
 
-	if s.EncryptBootVolume != nil && *s.EncryptBootVolume {
-		// encrypt_boot is true, so we have to copy the temporary
-		// AMI with required encryption setting.
-		// temp image was created by stepCreateAMI.
-		if s.RegionKeyIds == nil {
-			s.RegionKeyIds = make(map[string]string)
+	if s.EncryptBootVolume != nil {
+		if !s.AMISkipBuildRegion {
+			s.Regions = append(s.Regions, s.OriginalRegion)
+		}
+		if *s.EncryptBootVolume {
+			// encrypt_boot is true, so we have to copy the temporary
+			// AMI with required encryption setting.
+			// temp image was created by stepCreateAMI.
+			if s.RegionKeyIds == nil {
+				s.RegionKeyIds = make(map[string]string)
+			}
+			s.RegionKeyIds[s.OriginalRegion] = s.AMIKmsKeyId
 		}
-		s.RegionKeyIds[s.OriginalRegion] = s.AMIKmsKeyId
 	}
 
 	if len(s.Regions) == 0 {
@@ -99,6 +105,10 @@ func (s *StepAMIRegionCopy) Cleanup(state multistep.StateBag) {
 	ec2conn := state.Get("ec2").(*ec2.EC2)
 	ui := state.Get("ui").(packer.Ui)
 
+	if len(s.toDelete) == 0 {
+		return
+	}
+
 	// Delete the unencrypted amis and snapshots
 	ui.Say("Deregistering the AMI and deleting unencrypted temporary " +
 		"AMIs and snapshots")

builder/amazon/common/step_ami_region_copy_test.go

@@ -101,11 +101,11 @@ func TestStepAmiRegionCopy_nil_encryption(t *testing.T) {
 	state := tState()
 	stepAMIRegionCopy.Run(context.Background(), state)
 
-	if stepAMIRegionCopy.toDelete != "ami-12345" {
-		t.Fatalf("Should delete original intermediary ami even if not encrypted")
+	if stepAMIRegionCopy.toDelete != "" {
+		t.Fatalf("Shouldn't have an intermediary ami if encrypt is nil")
 	}
-	if len(stepAMIRegionCopy.Regions) == 0 {
-		t.Fatalf("Should have added original ami to original region")
+	if len(stepAMIRegionCopy.Regions) != 0 {
+		t.Fatalf("Should not have added original ami to original region")
 	}
 }
 
@@ -179,7 +179,7 @@ func TestStepAmiRegionCopy_true_AMISkipBuildRegion(t *testing.T) {
 	if stepAMIRegionCopy.toDelete == "" {
 		t.Fatalf("Should delete original AMI if skip_save_build_region=true")
 	}
-	if len(stepAMIRegionCopy.Regions) == 0 {
+	if len(stepAMIRegionCopy.Regions) != 0 {
 		t.Fatalf("Should not have added original ami to Regions")
 	}
 }