From 9850c7a5ef0fa0d5b1d53ef3b833ddc6f0ee3a40 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 22 Jan 2024 16:10:21 -0500 Subject: [PATCH 01/19] Add swap volume ebssurrogate option when using create image method, otherwise snapshot and register --- builder/ebssurrogate/builder.go | 66 ++++--- builder/ebssurrogate/builder.hcl2spec.go | 2 + builder/ebssurrogate/builder_acc_test.go | 155 ++++++++++++++++ builder/ebssurrogate/root_block_device.go | 6 + builder/ebssurrogate/step_create_ami.go | 213 ++++++++++++++++++++++ builder/ebssurrogate/step_swap_volumes.go | 125 +++++++++++++ 6 files changed, 546 insertions(+), 21 deletions(-) create mode 100644 builder/ebssurrogate/step_create_ami.go create mode 100644 builder/ebssurrogate/step_swap_volumes.go diff --git a/builder/ebssurrogate/builder.go b/builder/ebssurrogate/builder.go index dc7455c37..79703249c 100644 --- a/builder/ebssurrogate/builder.go +++ b/builder/ebssurrogate/builder.go @@ -318,6 +318,49 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) amiDevices := b.config.AMIMappings.BuildEC2BlockDeviceMappings() launchDevices := b.config.LaunchMappings.BuildEC2BlockDeviceMappings() + var imageMethod multistep.Step + var volumeMethod multistep.Step + + if b.config.RootDevice.ImageMethod != "create" { + volumeMethod = &StepSnapshotVolumes{ + PollingConfig: b.config.PollingConfig, + LaunchDevices: launchDevices, + SnapshotOmitMap: b.config.LaunchMappings.GetOmissions(), + SnapshotTags: b.config.SnapshotTags, + Ctx: b.config.ctx, + } + imageMethod = &StepRegisterAMI{ + RootDevice: b.config.RootDevice, + AMIDevices: amiDevices, + LaunchDevices: launchDevices, + EnableAMISriovNetSupport: b.config.AMISriovNetSupport, + EnableAMIENASupport: b.config.AMIENASupport, + Architecture: b.config.Architecture, + LaunchOmitMap: b.config.LaunchMappings.GetOmissions(), + AMISkipBuildRegion: b.config.AMISkipBuildRegion, + PollingConfig: b.config.PollingConfig, + BootMode: b.config.BootMode, + UefiData: b.config.UefiData, + TpmSupport: b.config.TpmSupport, + } + } else { + volumeMethod = &StepSwapVolumes{ + PollingConfig: b.config.PollingConfig, + RootDevice: b.config.RootDevice, + LaunchDevices: launchDevices, + LaunchOmitMap: b.config.LaunchMappings.GetOmissions(), + Ctx: b.config.ctx, + } + + imageMethod = &StepCreateAMI{ + AMISkipBuildRegion: b.config.AMISkipBuildRegion, + PollingConfig: b.config.PollingConfig, + IsRestricted: b.config.IsChinaCloud() || b.config.IsGovCloud(), + Tags: b.config.RunTags, + Ctx: b.config.ctx, + } + } + // Build the steps steps := []multistep.Step{ &awscommon.StepPreValidate{ @@ -420,13 +463,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) EnableAMISriovNetSupport: b.config.AMISriovNetSupport, EnableAMIENASupport: b.config.AMIENASupport, }, - &StepSnapshotVolumes{ - PollingConfig: b.config.PollingConfig, - LaunchDevices: launchDevices, - SnapshotOmitMap: b.config.LaunchMappings.GetOmissions(), - SnapshotTags: b.config.SnapshotTags, - Ctx: b.config.ctx, - }, + volumeMethod, &awscommon.StepDeregisterAMI{ AccessConfig: &b.config.AccessConfig, ForceDeregister: b.config.AMIForceDeregister, @@ -434,20 +471,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) AMIName: b.config.AMIName, Regions: b.config.AMIRegions, }, - &StepRegisterAMI{ - RootDevice: b.config.RootDevice, - AMIDevices: amiDevices, - LaunchDevices: launchDevices, - EnableAMISriovNetSupport: b.config.AMISriovNetSupport, - EnableAMIENASupport: b.config.AMIENASupport, - Architecture: b.config.Architecture, - LaunchOmitMap: b.config.LaunchMappings.GetOmissions(), - AMISkipBuildRegion: b.config.AMISkipBuildRegion, - PollingConfig: b.config.PollingConfig, - BootMode: b.config.BootMode, - UefiData: b.config.UefiData, - TpmSupport: b.config.TpmSupport, - }, + imageMethod, &awscommon.StepAMIRegionCopy{ AccessConfig: &b.config.AccessConfig, Regions: b.config.AMIRegions, diff --git a/builder/ebssurrogate/builder.hcl2spec.go b/builder/ebssurrogate/builder.hcl2spec.go index 3cda9cd1a..8dc8bc8ca 100644 --- a/builder/ebssurrogate/builder.hcl2spec.go +++ b/builder/ebssurrogate/builder.hcl2spec.go @@ -390,6 +390,7 @@ type FlatRootBlockDevice struct { IOPS *int64 `mapstructure:"iops" required:"false" cty:"iops" hcl:"iops"` VolumeType *string `mapstructure:"volume_type" required:"false" cty:"volume_type" hcl:"volume_type"` VolumeSize *int64 `mapstructure:"volume_size" required:"false" cty:"volume_size" hcl:"volume_size"` + ImageMethod *string `mapstructure:"image_method" required:"false" cty:"image_method" hcl:"image_method"` } // FlatMapstructure returns a new FlatRootBlockDevice. @@ -410,6 +411,7 @@ func (*FlatRootBlockDevice) HCL2Spec() map[string]hcldec.Spec { "iops": &hcldec.AttrSpec{Name: "iops", Type: cty.Number, Required: false}, "volume_type": &hcldec.AttrSpec{Name: "volume_type", Type: cty.String, Required: false}, "volume_size": &hcldec.AttrSpec{Name: "volume_size", Type: cty.Number, Required: false}, + "image_method": &hcldec.AttrSpec{Name: "image_method", Type: cty.String, Required: false}, } return s } diff --git a/builder/ebssurrogate/builder_acc_test.go b/builder/ebssurrogate/builder_acc_test.go index fa547a15d..8bef9d925 100644 --- a/builder/ebssurrogate/builder_acc_test.go +++ b/builder/ebssurrogate/builder_acc_test.go @@ -109,6 +109,75 @@ func TestAccBuilder_Ebssurrogate_SSHPrivateKeyFile_SSM(t *testing.T) { acctest.TestPlugin(t, testcase) } +func TestAccBuilder_EbssurrogateImageMethodCreate(t *testing.T) { + ami := amazon_acc.AMIHelper{ + Region: "us-east-1", + Name: "ebs-image-method-create-acc-test", + } + testCase := &acctest.PluginTestCase{ + Name: "amazon-ebssurrogate_image_method_create_test", + Template: fmt.Sprintf(testBuilderAccImageMethodCreate, ami.Name), + Teardown: func() error { + return ami.CleanUpAmi() + }, + Check: func(buildCommand *exec.Cmd, logfile string) error { + if buildCommand.ProcessState != nil { + if buildCommand.ProcessState.ExitCode() != 0 { + return fmt.Errorf("Bad exit code. Logfile: %s", logfile) + } + } + return nil + }, + } + acctest.TestPlugin(t, testCase) +} + +func TestAccBuilder_EbssurrogateImageMethodRegister(t *testing.T) { + ami := amazon_acc.AMIHelper{ + Region: "us-east-1", + Name: "ebs-image-method-register-acc-test", + } + testCase := &acctest.PluginTestCase{ + Name: "amazon-ebssurrogate_image_method_register_test", + Template: fmt.Sprintf(testBuilderAccImageMethodRegister, ami.Name), + Teardown: func() error { + return ami.CleanUpAmi() + }, + Check: func(buildCommand *exec.Cmd, logfile string) error { + if buildCommand.ProcessState != nil { + if buildCommand.ProcessState.ExitCode() != 0 { + return fmt.Errorf("Bad exit code. Logfile: %s", logfile) + } + } + return nil + }, + } + acctest.TestPlugin(t, testCase) +} + +func TestAccBuilder_EbssurrogateImageMethodEmpty(t *testing.T) { + ami := amazon_acc.AMIHelper{ + Region: "us-east-1", + Name: "ebs-image-method-empty-acc-test", + } + testCase := &acctest.PluginTestCase{ + Name: "amazon-ebssurrogate_image_method_empty_test", + Template: fmt.Sprintf(testBuilderAccImageMethodRegister, ami.Name), + Teardown: func() error { + return ami.CleanUpAmi() + }, + Check: func(buildCommand *exec.Cmd, logfile string) error { + if buildCommand.ProcessState != nil { + if buildCommand.ProcessState.ExitCode() != 0 { + return fmt.Errorf("Bad exit code. Logfile: %s", logfile) + } + } + return nil + }, + } + acctest.TestPlugin(t, testCase) +} + const testBuilderAccBasic = ` source "amazon-ebssurrogate" "test" { ami_name = "%s" @@ -197,3 +266,89 @@ build { sources = ["amazon-ebssurrogate.test"] } ` + +const testBuilderAccImageMethodCreate = ` +source "amazon-ebssurrogate" "test" { + ami_name = "%s" + region = "us-east-1" + instance_type = "m3.medium" + source_ami = "ami-76b2a71e" + ssh_username = "ubuntu" + launch_block_device_mappings { + device_name = "/dev/xvda" + delete_on_termination = true + volume_size = 8 + volume_type = "gp2" + } + ami_virtualization_type = "hvm" + ami_root_device { + source_device_name = "/dev/xvda" + device_name = "/dev/sda1" + delete_on_termination = true + volume_size = 8 + volume_type = "gp2" + image_method = "create" + } +} + +build { + sources = ["amazon-ebssurrogate.test"] +} +` + +const testBuilderAccImageMethodRegister = ` +source "amazon-ebssurrogate" "test" { + ami_name = "%s" + region = "us-east-1" + instance_type = "m3.medium" + source_ami = "ami-76b2a71e" + ssh_username = "ubuntu" + launch_block_device_mappings { + device_name = "/dev/xvda" + delete_on_termination = true + volume_size = 8 + volume_type = "gp2" + } + ami_virtualization_type = "hvm" + ami_root_device { + source_device_name = "/dev/xvda" + device_name = "/dev/sda1" + delete_on_termination = true + volume_size = 8 + volume_type = "gp2" + image_method = "create" + } +} + +build { + sources = ["amazon-ebssurrogate.test"] +} +` + +const testBuilderAccImageMethodEmpty = ` +source "amazon-ebssurrogate" "test" { + ami_name = "%s" + region = "us-east-1" + instance_type = "m3.medium" + source_ami = "ami-76b2a71e" + ssh_username = "ubuntu" + launch_block_device_mappings { + device_name = "/dev/xvda" + delete_on_termination = true + volume_size = 8 + volume_type = "gp2" + } + ami_virtualization_type = "hvm" + ami_root_device { + source_device_name = "/dev/xvda" + device_name = "/dev/sda1" + delete_on_termination = true + volume_size = 8 + volume_type = "gp2" + } +} + +build { + sources = ["amazon-ebssurrogate.test"] +} +` diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index 0373dfbf1..a39ba8ad8 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -36,6 +36,8 @@ type RootBlockDevice struct { // The size of the volume, in GiB. Required if // not specifying a snapshot_id. VolumeSize int64 `mapstructure:"volume_size" required:"false"` + //Method used for image, create or register + ImageMethod string `mapstructure:"image_method" required:"false"` } func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { @@ -61,6 +63,10 @@ func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { errs = append(errs, errors.New("volume_size must be greater than 0")) } + if c.ImageMethod != "" && c.ImageMethod != "create" && c.ImageMethod != "register" { + errs = append(errs, errors.New("image_method must be empty string or 'create' or 'register'")) + } + if len(errs) > 0 { return errs } diff --git a/builder/ebssurrogate/step_create_ami.go b/builder/ebssurrogate/step_create_ami.go new file mode 100644 index 000000000..1a60299cd --- /dev/null +++ b/builder/ebssurrogate/step_create_ami.go @@ -0,0 +1,213 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ebssurrogate + +import ( + "context" + "fmt" + "log" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + awscommon "github.com/hashicorp/packer-plugin-amazon/builder/common" + "github.com/hashicorp/packer-plugin-amazon/builder/common/awserrors" + "github.com/hashicorp/packer-plugin-sdk/multistep" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer-plugin-sdk/random" + "github.com/hashicorp/packer-plugin-sdk/retry" + "github.com/hashicorp/packer-plugin-sdk/template/interpolate" +) + +type StepCreateAMI struct { + PollingConfig *awscommon.AWSPollingConfig + image *ec2.Image + AMISkipBuildRegion bool + IsRestricted bool + Ctx interpolate.Context + Tags map[string]string +} + +func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + config := state.Get("config").(*Config) + ec2conn := state.Get("ec2").(*ec2.EC2) + instance := state.Get("instance").(*ec2.Instance) + ui := state.Get("ui").(packersdk.Ui) + + // Create the image + amiName := config.AMIName + state.Put("intermediary_image", false) + if config.AMIEncryptBootVolume.True() || s.AMISkipBuildRegion { + state.Put("intermediary_image", true) + + // From AWS SDK docs: You can encrypt a copy of an unencrypted snapshot, + // but you cannot use it to create an unencrypted copy of an encrypted + // snapshot. Your default CMK for EBS is used unless you specify a + // non-default key using KmsKeyId. + + // If encrypt_boot is nil or true, we need to create a temporary image + // so that in step_region_copy, we can copy it with the correct + // encryption + amiName = random.AlphaNum(7) + } + + ui.Say(fmt.Sprintf("Creating AMI %s from instance %s", amiName, *instance.InstanceId)) + createOpts := &ec2.CreateImageInput{ + InstanceId: instance.InstanceId, + Name: &amiName, + BlockDeviceMappings: config.AMIMappings.BuildEC2BlockDeviceMappings(), + } + + if !s.IsRestricted { + ec2Tags, err := awscommon.TagMap(s.Tags).EC2Tags(s.Ctx, *ec2conn.Config.Region, state) + if err != nil { + err := fmt.Errorf("Error tagging AMI: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + createOpts.TagSpecifications = ec2Tags.TagSpecifications(ec2.ResourceTypeImage, ec2.ResourceTypeSnapshot) + } + + var createResp *ec2.CreateImageOutput + + // Create a timeout for the CreateImage call. + timeoutCtx, cancel := context.WithTimeout(ctx, time.Minute*15) + defer cancel() + + err := retry.Config{ + Tries: 0, + ShouldRetry: func(err error) bool { + if awserrors.Matches(err, "InvalidParameterValue", "Instance is not in state") { + return true + } + return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(timeoutCtx, func(ctx context.Context) error { + var err error + createResp, err = ec2conn.CreateImage(createOpts) + return err + }) + if err != nil { + err := fmt.Errorf("Error creating AMI: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + // Set the AMI ID in the state + ui.Message(fmt.Sprintf("AMI: %s", *createResp.ImageId)) + amis := make(map[string]string) + amis[*ec2conn.Config.Region] = *createResp.ImageId + state.Put("amis", amis) + + // Wait for the image to become ready + ui.Say("Waiting for AMI to become ready...") + if waitErr := s.PollingConfig.WaitUntilAMIAvailable(ctx, ec2conn, *createResp.ImageId); waitErr != nil { + // waitErr should get bubbled up if the issue is a wait timeout + err := fmt.Errorf("Error waiting for AMI: %s", waitErr) + imResp, imerr := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ImageIds: []*string{createResp.ImageId}}) + if imerr != nil { + // If there's a failure describing images, bubble that error up too, but don't erase the waitErr. + log.Printf("DescribeImages call was unable to determine reason waiting for AMI failed: %s", imerr) + err = fmt.Errorf("Unknown error waiting for AMI; %s. DescribeImages returned an error: %s", waitErr, imerr) + } + if imResp != nil && len(imResp.Images) > 0 { + // Finally, if there's a stateReason, store that with the wait err + image := imResp.Images[0] + if image != nil { + stateReason := image.StateReason + if stateReason != nil { + err = fmt.Errorf("Error waiting for AMI: %s. DescribeImages returned the state reason: %s", waitErr, stateReason) + } + } + } + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + imagesResp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ImageIds: []*string{createResp.ImageId}}) + if err != nil { + err := fmt.Errorf("Error searching for AMI: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + s.image = imagesResp.Images[0] + + snapshots := make(map[string][]string) + for _, blockDeviceMapping := range imagesResp.Images[0].BlockDeviceMappings { + if blockDeviceMapping.Ebs != nil && blockDeviceMapping.Ebs.SnapshotId != nil { + + snapshots[*ec2conn.Config.Region] = append(snapshots[*ec2conn.Config.Region], *blockDeviceMapping.Ebs.SnapshotId) + } + } + state.Put("snapshots", snapshots) + + return multistep.ActionContinue +} + +func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { + if s.image == nil { + return + } + + _, cancelled := state.GetOk(multistep.StateCancelled) + _, halted := state.GetOk(multistep.StateHalted) + if !cancelled && !halted { + return + } + + ec2conn := state.Get("ec2").(*ec2.EC2) + ui := state.Get("ui").(packersdk.Ui) + + ui.Say("Deregistering the AMI and deleting associated snapshots because " + + "of cancellation, or error...") + + resp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ + ImageIds: []*string{s.image.ImageId}, + }) + + if err != nil { + err := fmt.Errorf("Error describing AMI: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return + } + + // Deregister image by name. + for _, i := range resp.Images { + _, err := ec2conn.DeregisterImage(&ec2.DeregisterImageInput{ + ImageId: i.ImageId, + }) + + if err != nil { + err := fmt.Errorf("Error deregistering existing AMI: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return + } + ui.Say(fmt.Sprintf("Deregistered AMI id: %s", *i.ImageId)) + + // Delete snapshot(s) by image + for _, b := range i.BlockDeviceMappings { + if b.Ebs != nil && aws.StringValue(b.Ebs.SnapshotId) != "" { + _, err := ec2conn.DeleteSnapshot(&ec2.DeleteSnapshotInput{ + SnapshotId: b.Ebs.SnapshotId, + }) + + if err != nil { + err := fmt.Errorf("Error deleting existing snapshot: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return + } + ui.Say(fmt.Sprintf("Deleted snapshot: %s", *b.Ebs.SnapshotId)) + } + } + } +} diff --git a/builder/ebssurrogate/step_swap_volumes.go b/builder/ebssurrogate/step_swap_volumes.go new file mode 100644 index 000000000..4cd50f561 --- /dev/null +++ b/builder/ebssurrogate/step_swap_volumes.go @@ -0,0 +1,125 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package ebssurrogate + +import ( + "context" + "fmt" + "strings" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + awscommon "github.com/hashicorp/packer-plugin-amazon/builder/common" + "github.com/hashicorp/packer-plugin-sdk/multistep" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer-plugin-sdk/template/interpolate" +) + +// StepSnapshotVolumes creates snapshots of the created volumes. +// +// Produces: +// +// snapshot_ids map[string]string - IDs of the created snapshots +type StepSwapVolumes struct { + PollingConfig *awscommon.AWSPollingConfig + RootDevice RootBlockDevice + LaunchDevices []*ec2.BlockDeviceMapping + LaunchOmitMap map[string]bool + Ctx interpolate.Context +} + +func (s *StepSwapVolumes) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + ec2conn := state.Get("ec2").(*ec2.EC2) + ui := state.Get("ui").(packersdk.Ui) + instance := state.Get("instance").(*ec2.Instance) + + // Describe the instance + input := &ec2.DescribeInstancesInput{ + InstanceIds: []*string{ + aws.String(*instance.InstanceId), + }, + } + + result, err := ec2conn.DescribeInstances(input) + if err != nil { + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + deviceToVolumeMap := make(map[string]string) + + // Iterate through block device mappings and populate the map + for _, reservation := range result.Reservations { + for _, instance := range reservation.Instances { + for _, blockDevice := range instance.BlockDeviceMappings { + deviceToVolumeMap[*blockDevice.DeviceName] = *blockDevice.Ebs.VolumeId + } + } + } + + for deviceName, volumeID := range deviceToVolumeMap { + ui.Say(fmt.Sprintf("Detaching EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) + + _, err := ec2conn.DetachVolume(&ec2.DetachVolumeInput{VolumeId: &volumeID}) + if err == nil { + err = s.PollingConfig.WaitUntilVolumeDetached(ctx, ec2conn, volumeID) + } + + if err != nil { + err := fmt.Errorf("error detaching volume: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + } + + for deviceName, volumeID := range deviceToVolumeMap { + omit, ok := s.LaunchOmitMap[deviceName] + if ok && omit { + ui.Say(fmt.Sprintf("Skip Attaching Ommitted EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) + continue + } + + if deviceName == s.RootDevice.DeviceName { + ui.Say(fmt.Sprintf("Skip Attaching Root EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) + continue + } + ui.Say(fmt.Sprintf("Attaching EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) + + attachVolume := deviceName + if deviceName == s.RootDevice.SourceDeviceName { + rootDeviceName := *aws.String(s.RootDevice.DeviceName) + ui.Say(fmt.Sprintf("Rename EBS Device Name: %s as the root device name %s\n", deviceName, rootDeviceName)) + // For the API call, it expects "sd" prefixed devices. + attachVolume = strings.Replace(rootDeviceName, "/xvd", "/sd", 1) + } + + ui.Say(fmt.Sprintf("Attaching the volume to %s", attachVolume)) + _, err := ec2conn.AttachVolume(&ec2.AttachVolumeInput{ + InstanceId: instance.InstanceId, + VolumeId: &volumeID, + Device: &attachVolume, + }) + if err != nil { + err := fmt.Errorf("error attaching volume: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + // Wait for the volume to become attached + err = s.PollingConfig.WaitUntilVolumeAttached(ctx, ec2conn, volumeID) + if err != nil { + err := fmt.Errorf("error waiting for volume: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + } + + return multistep.ActionContinue +} + +func (s *StepSwapVolumes) Cleanup(state multistep.StateBag) {} From f3efbaaca27f274710becae01c48b29f31a0843c Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 29 Jan 2024 22:35:33 -0500 Subject: [PATCH 02/19] Detach omitted and root devices, pass in all block devices and combine --- builder/ebssurrogate/builder.go | 3 + builder/ebssurrogate/step_create_ami.go | 65 +++++++++++++---- builder/ebssurrogate/step_swap_volumes.go | 85 +++++++++++------------ 3 files changed, 94 insertions(+), 59 deletions(-) diff --git a/builder/ebssurrogate/builder.go b/builder/ebssurrogate/builder.go index 79703249c..38bfd86c0 100644 --- a/builder/ebssurrogate/builder.go +++ b/builder/ebssurrogate/builder.go @@ -354,6 +354,9 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) imageMethod = &StepCreateAMI{ AMISkipBuildRegion: b.config.AMISkipBuildRegion, + RootDevice: b.config.RootDevice, + AMIDevices: amiDevices, + LaunchDevices: launchDevices, PollingConfig: b.config.PollingConfig, IsRestricted: b.config.IsChinaCloud() || b.config.IsGovCloud(), Tags: b.config.RunTags, diff --git a/builder/ebssurrogate/step_create_ami.go b/builder/ebssurrogate/step_create_ami.go index 1a60299cd..466109b34 100644 --- a/builder/ebssurrogate/step_create_ami.go +++ b/builder/ebssurrogate/step_create_ami.go @@ -22,6 +22,10 @@ import ( type StepCreateAMI struct { PollingConfig *awscommon.AWSPollingConfig + RootDevice RootBlockDevice + AMIDevices []*ec2.BlockDeviceMapping + LaunchDevices []*ec2.BlockDeviceMapping + LaunchOmitMap map[string]bool image *ec2.Image AMISkipBuildRegion bool IsRestricted bool @@ -35,6 +39,8 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi instance := state.Get("instance").(*ec2.Instance) ui := state.Get("ui").(packersdk.Ui) + blockDevices := s.combineDevices() + // Create the image amiName := config.AMIName state.Put("intermediary_image", false) @@ -56,13 +62,13 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi createOpts := &ec2.CreateImageInput{ InstanceId: instance.InstanceId, Name: &amiName, - BlockDeviceMappings: config.AMIMappings.BuildEC2BlockDeviceMappings(), + BlockDeviceMappings: blockDevices, } if !s.IsRestricted { ec2Tags, err := awscommon.TagMap(s.Tags).EC2Tags(s.Ctx, *ec2conn.Config.Region, state) if err != nil { - err := fmt.Errorf("Error tagging AMI: %s", err) + err := fmt.Errorf("error tagging AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -80,10 +86,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi err := retry.Config{ Tries: 0, ShouldRetry: func(err error) bool { - if awserrors.Matches(err, "InvalidParameterValue", "Instance is not in state") { - return true - } - return false + return awserrors.Matches(err, "InvalidParameterValue", "Instance is not in state") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(timeoutCtx, func(ctx context.Context) error { @@ -92,7 +95,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi return err }) if err != nil { - err := fmt.Errorf("Error creating AMI: %s", err) + err := fmt.Errorf("error creating AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -108,12 +111,12 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi ui.Say("Waiting for AMI to become ready...") if waitErr := s.PollingConfig.WaitUntilAMIAvailable(ctx, ec2conn, *createResp.ImageId); waitErr != nil { // waitErr should get bubbled up if the issue is a wait timeout - err := fmt.Errorf("Error waiting for AMI: %s", waitErr) + err := fmt.Errorf("error waiting for AMI: %s", waitErr) imResp, imerr := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ImageIds: []*string{createResp.ImageId}}) if imerr != nil { // If there's a failure describing images, bubble that error up too, but don't erase the waitErr. log.Printf("DescribeImages call was unable to determine reason waiting for AMI failed: %s", imerr) - err = fmt.Errorf("Unknown error waiting for AMI; %s. DescribeImages returned an error: %s", waitErr, imerr) + err = fmt.Errorf("unknown error waiting for AMI; %s. DescribeImages returned an error: %s", waitErr, imerr) } if imResp != nil && len(imResp.Images) > 0 { // Finally, if there's a stateReason, store that with the wait err @@ -121,7 +124,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi if image != nil { stateReason := image.StateReason if stateReason != nil { - err = fmt.Errorf("Error waiting for AMI: %s. DescribeImages returned the state reason: %s", waitErr, stateReason) + err = fmt.Errorf("error waiting for AMI: %s. DescribeImages returned the state reason: %s", waitErr, stateReason) } } } @@ -132,7 +135,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi imagesResp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ImageIds: []*string{createResp.ImageId}}) if err != nil { - err := fmt.Errorf("Error searching for AMI: %s", err) + err := fmt.Errorf("error searching for AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -151,6 +154,40 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi return multistep.ActionContinue } +func (s *StepCreateAMI) combineDevices() []*ec2.BlockDeviceMapping { + devices := map[string]*ec2.BlockDeviceMapping{} + + for _, device := range s.AMIDevices { + devices[*device.DeviceName] = device + } + + // Devices in launch_block_device_mappings override any with + // the same name in ami_block_device_mappings + // If launch device name is equal to Root SourceDeviceName + // then set the device's deviceName to RootDevice.DeviceName from root ami_root_device + // and overwrite / add the device in the devices map + for _, device := range s.LaunchDevices { + // Skip devices we've flagged for omission + omit, ok := s.LaunchOmitMap[*device.DeviceName] + if ok && omit { + continue + } + + // Use root device name for the new source root device + if *device.DeviceName == s.RootDevice.SourceDeviceName { + device.DeviceName = aws.String(s.RootDevice.DeviceName) + } + devices[*device.DeviceName] = device + } + + blockDevices := []*ec2.BlockDeviceMapping{} + for _, device := range devices { + + blockDevices = append(blockDevices, device) + } + return blockDevices +} + func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { if s.image == nil { return @@ -173,7 +210,7 @@ func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { }) if err != nil { - err := fmt.Errorf("Error describing AMI: %s", err) + err := fmt.Errorf("error describing AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return @@ -186,7 +223,7 @@ func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { }) if err != nil { - err := fmt.Errorf("Error deregistering existing AMI: %s", err) + err := fmt.Errorf("error deregistering existing AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return @@ -201,7 +238,7 @@ func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { }) if err != nil { - err := fmt.Errorf("Error deleting existing snapshot: %s", err) + err := fmt.Errorf("error deleting existing snapshot: %s", err) state.Put("error", err) ui.Error(err.Error()) return diff --git a/builder/ebssurrogate/step_swap_volumes.go b/builder/ebssurrogate/step_swap_volumes.go index 4cd50f561..511312802 100644 --- a/builder/ebssurrogate/step_swap_volumes.go +++ b/builder/ebssurrogate/step_swap_volumes.go @@ -6,7 +6,6 @@ package ebssurrogate import ( "context" "fmt" - "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -60,11 +59,15 @@ func (s *StepSwapVolumes) Run(ctx context.Context, state multistep.StateBag) mul } for deviceName, volumeID := range deviceToVolumeMap { - ui.Say(fmt.Sprintf("Detaching EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) - - _, err := ec2conn.DetachVolume(&ec2.DetachVolumeInput{VolumeId: &volumeID}) - if err == nil { - err = s.PollingConfig.WaitUntilVolumeDetached(ctx, ec2conn, volumeID) + omit, ok := s.LaunchOmitMap[deviceName] + if ok && omit { + ui.Say(fmt.Sprintf("Detaching Ommitted EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) + err = s.detach_volume(ctx, ec2conn, deviceName, volumeID) + } else if deviceName == s.RootDevice.DeviceName || deviceName == s.RootDevice.SourceDeviceName || deviceName == "/dev/sda1" { + ui.Say(fmt.Sprintf("Detaching Root EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) + err = s.detach_volume(ctx, ec2conn, deviceName, volumeID) + } else { + ui.Say(fmt.Sprintf("Skip Detach of EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) } if err != nil { @@ -73,53 +76,45 @@ func (s *StepSwapVolumes) Run(ctx context.Context, state multistep.StateBag) mul ui.Error(err.Error()) return multistep.ActionHalt } + } - for deviceName, volumeID := range deviceToVolumeMap { - omit, ok := s.LaunchOmitMap[deviceName] - if ok && omit { - ui.Say(fmt.Sprintf("Skip Attaching Ommitted EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) - continue - } + rootVolumeId := aws.String(deviceToVolumeMap[s.RootDevice.SourceDeviceName]) + rootDeviceName := aws.String(s.RootDevice.DeviceName) + ui.Say(fmt.Sprintf("Attaching Root EBS Device Name %s, Volume ID: %s", *rootDeviceName, *rootVolumeId)) - if deviceName == s.RootDevice.DeviceName { - ui.Say(fmt.Sprintf("Skip Attaching Root EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) - continue - } - ui.Say(fmt.Sprintf("Attaching EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) - - attachVolume := deviceName - if deviceName == s.RootDevice.SourceDeviceName { - rootDeviceName := *aws.String(s.RootDevice.DeviceName) - ui.Say(fmt.Sprintf("Rename EBS Device Name: %s as the root device name %s\n", deviceName, rootDeviceName)) - // For the API call, it expects "sd" prefixed devices. - attachVolume = strings.Replace(rootDeviceName, "/xvd", "/sd", 1) - } + _, err = ec2conn.AttachVolume(&ec2.AttachVolumeInput{ + InstanceId: instance.InstanceId, + VolumeId: rootVolumeId, + Device: rootDeviceName, + }) - ui.Say(fmt.Sprintf("Attaching the volume to %s", attachVolume)) - _, err := ec2conn.AttachVolume(&ec2.AttachVolumeInput{ - InstanceId: instance.InstanceId, - VolumeId: &volumeID, - Device: &attachVolume, - }) - if err != nil { - err := fmt.Errorf("error attaching volume: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } + if err != nil { + err := fmt.Errorf("error attaching volume: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } - // Wait for the volume to become attached - err = s.PollingConfig.WaitUntilVolumeAttached(ctx, ec2conn, volumeID) - if err != nil { - err := fmt.Errorf("error waiting for volume: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } + // Wait for the volume to become attached + err = s.PollingConfig.WaitUntilVolumeAttached(ctx, ec2conn, *rootVolumeId) + if err != nil { + err := fmt.Errorf("error waiting for volume: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt } return multistep.ActionContinue } +func (s *StepSwapVolumes) detach_volume(ctx context.Context, ec2conn *ec2.EC2, deviceName string, volumeId string) error { + _, err := ec2conn.DetachVolume(&ec2.DetachVolumeInput{VolumeId: &volumeId}) + if err == nil { + return s.PollingConfig.WaitUntilVolumeDetached(ctx, ec2conn, volumeId) + } + + return err +} + func (s *StepSwapVolumes) Cleanup(state multistep.StateBag) {} From 3646823cae719fadeef207d37de016f97fcb8b03 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Tue, 30 Jan 2024 09:24:30 -0500 Subject: [PATCH 03/19] Normalize case of error messages to be consistent with rest of project --- builder/ebssurrogate/step_create_ami.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builder/ebssurrogate/step_create_ami.go b/builder/ebssurrogate/step_create_ami.go index 466109b34..36a1b37d9 100644 --- a/builder/ebssurrogate/step_create_ami.go +++ b/builder/ebssurrogate/step_create_ami.go @@ -68,7 +68,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi if !s.IsRestricted { ec2Tags, err := awscommon.TagMap(s.Tags).EC2Tags(s.Ctx, *ec2conn.Config.Region, state) if err != nil { - err := fmt.Errorf("error tagging AMI: %s", err) + err := fmt.Errorf("Error tagging AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -95,7 +95,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi return err }) if err != nil { - err := fmt.Errorf("error creating AMI: %s", err) + err := fmt.Errorf("Error creating AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -111,12 +111,12 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi ui.Say("Waiting for AMI to become ready...") if waitErr := s.PollingConfig.WaitUntilAMIAvailable(ctx, ec2conn, *createResp.ImageId); waitErr != nil { // waitErr should get bubbled up if the issue is a wait timeout - err := fmt.Errorf("error waiting for AMI: %s", waitErr) + err := fmt.Errorf("Error waiting for AMI: %s", waitErr) imResp, imerr := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ImageIds: []*string{createResp.ImageId}}) if imerr != nil { // If there's a failure describing images, bubble that error up too, but don't erase the waitErr. log.Printf("DescribeImages call was unable to determine reason waiting for AMI failed: %s", imerr) - err = fmt.Errorf("unknown error waiting for AMI; %s. DescribeImages returned an error: %s", waitErr, imerr) + err = fmt.Errorf("Unknown error waiting for AMI; %s. DescribeImages returned an error: %s", waitErr, imerr) } if imResp != nil && len(imResp.Images) > 0 { // Finally, if there's a stateReason, store that with the wait err @@ -124,7 +124,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi if image != nil { stateReason := image.StateReason if stateReason != nil { - err = fmt.Errorf("error waiting for AMI: %s. DescribeImages returned the state reason: %s", waitErr, stateReason) + err = fmt.Errorf("Error waiting for AMI: %s. DescribeImages returned the state reason: %s", waitErr, stateReason) } } } @@ -135,7 +135,7 @@ func (s *StepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi imagesResp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ImageIds: []*string{createResp.ImageId}}) if err != nil { - err := fmt.Errorf("error searching for AMI: %s", err) + err := fmt.Errorf("Error searching for AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return multistep.ActionHalt @@ -210,7 +210,7 @@ func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { }) if err != nil { - err := fmt.Errorf("error describing AMI: %s", err) + err := fmt.Errorf("Error describing AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return @@ -223,7 +223,7 @@ func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { }) if err != nil { - err := fmt.Errorf("error deregistering existing AMI: %s", err) + err := fmt.Errorf("Error deregistering existing AMI: %s", err) state.Put("error", err) ui.Error(err.Error()) return @@ -238,7 +238,7 @@ func (s *StepCreateAMI) Cleanup(state multistep.StateBag) { }) if err != nil { - err := fmt.Errorf("error deleting existing snapshot: %s", err) + err := fmt.Errorf("Error deleting existing snapshot: %s", err) state.Put("error", err) ui.Error(err.Error()) return From 647aa00c9f67548e1063f5c2961f12b310baac3d Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Tue, 30 Jan 2024 09:33:50 -0500 Subject: [PATCH 04/19] Fix reference to empty image_method test case --- builder/ebssurrogate/builder_acc_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builder/ebssurrogate/builder_acc_test.go b/builder/ebssurrogate/builder_acc_test.go index 8bef9d925..721d4f211 100644 --- a/builder/ebssurrogate/builder_acc_test.go +++ b/builder/ebssurrogate/builder_acc_test.go @@ -162,7 +162,7 @@ func TestAccBuilder_EbssurrogateImageMethodEmpty(t *testing.T) { } testCase := &acctest.PluginTestCase{ Name: "amazon-ebssurrogate_image_method_empty_test", - Template: fmt.Sprintf(testBuilderAccImageMethodRegister, ami.Name), + Template: fmt.Sprintf(testBuilderAccImageMethodEmpty, ami.Name), Teardown: func() error { return ami.CleanUpAmi() }, @@ -316,7 +316,7 @@ source "amazon-ebssurrogate" "test" { delete_on_termination = true volume_size = 8 volume_type = "gp2" - image_method = "create" + image_method = "register" } } From 9c69ec60744bdb61b7dc3860e26df6538d803489 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Tue, 30 Jan 2024 09:53:43 -0500 Subject: [PATCH 05/19] Run go fmt ./... --- builder/ebssurrogate/builder.hcl2spec.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builder/ebssurrogate/builder.hcl2spec.go b/builder/ebssurrogate/builder.hcl2spec.go index 8dc8bc8ca..d9c152eef 100644 --- a/builder/ebssurrogate/builder.hcl2spec.go +++ b/builder/ebssurrogate/builder.hcl2spec.go @@ -390,7 +390,7 @@ type FlatRootBlockDevice struct { IOPS *int64 `mapstructure:"iops" required:"false" cty:"iops" hcl:"iops"` VolumeType *string `mapstructure:"volume_type" required:"false" cty:"volume_type" hcl:"volume_type"` VolumeSize *int64 `mapstructure:"volume_size" required:"false" cty:"volume_size" hcl:"volume_size"` - ImageMethod *string `mapstructure:"image_method" required:"false" cty:"image_method" hcl:"image_method"` + ImageMethod *string `mapstructure:"image_method" required:"false" cty:"image_method" hcl:"image_method"` } // FlatMapstructure returns a new FlatRootBlockDevice. @@ -411,7 +411,7 @@ func (*FlatRootBlockDevice) HCL2Spec() map[string]hcldec.Spec { "iops": &hcldec.AttrSpec{Name: "iops", Type: cty.Number, Required: false}, "volume_type": &hcldec.AttrSpec{Name: "volume_type", Type: cty.String, Required: false}, "volume_size": &hcldec.AttrSpec{Name: "volume_size", Type: cty.Number, Required: false}, - "image_method": &hcldec.AttrSpec{Name: "image_method", Type: cty.String, Required: false}, + "image_method": &hcldec.AttrSpec{Name: "image_method", Type: cty.String, Required: false}, } return s } From a6ccbc3c7c351d40ecccdc17cefd5499de8d4d89 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Tue, 30 Jan 2024 09:54:38 -0500 Subject: [PATCH 06/19] Generate docs --- .../builder/ebssurrogate/RootBlockDevice-not-required.mdx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx index c8d6e7be5..72e4b4788 100644 --- a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx @@ -25,4 +25,6 @@ - `volume_size` (int64) - The size of the volume, in GiB. Required if not specifying a snapshot_id. +- `image_method` (string) - Method used for image, create or register + From 00abc4aa2381e18cc40c8fbf1493ca09ae13ab08 Mon Sep 17 00:00:00 2001 From: dwc0011 <53784917+dwc0011@users.noreply.github.com> Date: Wed, 21 Feb 2024 21:05:35 -0500 Subject: [PATCH 07/19] Update builder/ebssurrogate/root_block_device.go Set the default ImageMethod to "register" Co-authored-by: Wilken Rivera --- builder/ebssurrogate/root_block_device.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index a39ba8ad8..2b29fa40d 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -63,10 +63,12 @@ func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { errs = append(errs, errors.New("volume_size must be greater than 0")) } - if c.ImageMethod != "" && c.ImageMethod != "create" && c.ImageMethod != "register" { + if c.ImageMethod != "" c.ImageMethod != "create" && c.ImageMethod != "register" { errs = append(errs, errors.New("image_method must be empty string or 'create' or 'register'")) } - + if c.ImageMethod == "" { + c.ImageMethod = "register" + } if len(errs) > 0 { return errs } From 608b103a1f7f80a15f79aabbac99081235ac8ece Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 21 Feb 2024 21:03:22 -0500 Subject: [PATCH 08/19] Fix the comment, remove copy pasta and add what the swap volumes step does. --- builder/ebssurrogate/step_swap_volumes.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/builder/ebssurrogate/step_swap_volumes.go b/builder/ebssurrogate/step_swap_volumes.go index 511312802..6674bd41b 100644 --- a/builder/ebssurrogate/step_swap_volumes.go +++ b/builder/ebssurrogate/step_swap_volumes.go @@ -15,11 +15,8 @@ import ( "github.com/hashicorp/packer-plugin-sdk/template/interpolate" ) -// StepSnapshotVolumes creates snapshots of the created volumes. -// -// Produces: -// -// snapshot_ids map[string]string - IDs of the created snapshots +// StepSwapVolumes detaches omitted volumes and original root volume and reattaches +// the new root volume specified by ami_root_device.source_device_name. type StepSwapVolumes struct { PollingConfig *awscommon.AWSPollingConfig RootDevice RootBlockDevice From bf65fa041b1068dc7b635fdf20206528a4c56439 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 21 Feb 2024 21:17:32 -0500 Subject: [PATCH 09/19] Add more in-depth comment about what actually happens when using Create or Register --- builder/ebssurrogate/root_block_device.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index 2b29fa40d..74ceabb3f 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -36,7 +36,11 @@ type RootBlockDevice struct { // The size of the volume, in GiB. Required if // not specifying a snapshot_id. VolumeSize int64 `mapstructure:"volume_size" required:"false"` - //Method used for image, create or register + //Whether to create or register the AMI image. When set to create + //the root volume is detached and swapped with the volume specified + //by SourceDeviceName before the image is created. All volume metadata is retained. + //When set to register, the root volume is snapshotted and used when registering the image. + //Volume metadata is not retained. Both methods detach any ommitted volumes. ImageMethod string `mapstructure:"image_method" required:"false"` } From febde1cc4c87ff52237eb4a629109a1472ccfd43 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 21 Feb 2024 21:27:58 -0500 Subject: [PATCH 10/19] Refactor to prevent the duplicate check of the empty string --- builder/ebssurrogate/root_block_device.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index 74ceabb3f..178b230f6 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -67,12 +67,12 @@ func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { errs = append(errs, errors.New("volume_size must be greater than 0")) } - if c.ImageMethod != "" c.ImageMethod != "create" && c.ImageMethod != "register" { - errs = append(errs, errors.New("image_method must be empty string or 'create' or 'register'")) - } if c.ImageMethod == "" { - c.ImageMethod = "register" - } + c.ImageMethod = "register" + } else if c.ImageMethod != "create" && c.ImageMethod != "register" { + errs = append(errs, errors.New("image_method must be 'create', 'register' or an empty string' ")) + } + if len(errs) > 0 { return errs } From ee97e5b5165d5b35b79c355ed4573399f5ba9075 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Wed, 21 Feb 2024 21:50:41 -0500 Subject: [PATCH 11/19] Generate docs --- .../builder/ebssurrogate/RootBlockDevice-not-required.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx index 72e4b4788..02d11474c 100644 --- a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx @@ -25,6 +25,10 @@ - `volume_size` (int64) - The size of the volume, in GiB. Required if not specifying a snapshot_id. -- `image_method` (string) - Method used for image, create or register +- `image_method` (string) - Whether to create or register the AMI image. When set to create + the root volume is detached and swapped with the volume specified + by SourceDeviceName before the image is created. All volume metadata is retained. + When set to register, the root volume is snapshotted and used when registering the image. + Volume metadata is not retained. Both methods detach any ommitted volumes. From 182506d8f5f92abb83b063990a787cda194f1d29 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Thu, 22 Feb 2024 19:29:53 -0500 Subject: [PATCH 12/19] Based on feedback from @lorengordon, Update comment to be more explicit and reference the APIs it utilizes. --- builder/ebssurrogate/root_block_device.go | 12 +++++++----- .../ebssurrogate/RootBlockDevice-not-required.mdx | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index 178b230f6..faced6c54 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -36,11 +36,13 @@ type RootBlockDevice struct { // The size of the volume, in GiB. Required if // not specifying a snapshot_id. VolumeSize int64 `mapstructure:"volume_size" required:"false"` - //Whether to create or register the AMI image. When set to create - //the root volume is detached and swapped with the volume specified - //by SourceDeviceName before the image is created. All volume metadata is retained. - //When set to register, the root volume is snapshotted and used when registering the image. - //Volume metadata is not retained. Both methods detach any ommitted volumes. + //Whether to use the CreateImage or RegisterImage API when creating the AMI. + //When set to `create`, CreateImage creates the image from the instance itself, + //and inherits properties from the instance. When set to `register`, the image + //is created from a snapshot of the specified EBS volume, and no properties + //are inherited from the instance. + //Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html + // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html ImageMethod string `mapstructure:"image_method" required:"false"` } diff --git a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx index 02d11474c..636c7136c 100644 --- a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx @@ -25,10 +25,12 @@ - `volume_size` (int64) - The size of the volume, in GiB. Required if not specifying a snapshot_id. -- `image_method` (string) - Whether to create or register the AMI image. When set to create - the root volume is detached and swapped with the volume specified - by SourceDeviceName before the image is created. All volume metadata is retained. - When set to register, the root volume is snapshotted and used when registering the image. - Volume metadata is not retained. Both methods detach any ommitted volumes. +- `image_method` (string) - Whether to use the CreateImage or RegisterImage API when creating the AMI. + When set to `create`, CreateImage creates the image from the instance itself, + and inherits properties from the instance. When set to `register`, the image + is created from a snapshot of the specified EBS volume, and no properties + are inherited from the instance. + Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html + https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html From 68d14442447a78b16259658c704e133f38e36558 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Thu, 29 Feb 2024 13:58:30 -0500 Subject: [PATCH 13/19] Rename option and change to a boolean --- builder/ebssurrogate/builder.go | 52 +++++++++++------------ builder/ebssurrogate/builder_acc_test.go | 20 ++++----- builder/ebssurrogate/root_block_device.go | 16 +++---- 3 files changed, 41 insertions(+), 47 deletions(-) diff --git a/builder/ebssurrogate/builder.go b/builder/ebssurrogate/builder.go index 38bfd86c0..5393b3774 100644 --- a/builder/ebssurrogate/builder.go +++ b/builder/ebssurrogate/builder.go @@ -318,18 +318,37 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) amiDevices := b.config.AMIMappings.BuildEC2BlockDeviceMappings() launchDevices := b.config.LaunchMappings.BuildEC2BlockDeviceMappings() - var imageMethod multistep.Step - var volumeMethod multistep.Step + var buildAmiStep multistep.Step + var volumeStep multistep.Step - if b.config.RootDevice.ImageMethod != "create" { - volumeMethod = &StepSnapshotVolumes{ + if b.config.RootDevice.UseCreateImage { + volumeStep = &StepSwapVolumes{ + PollingConfig: b.config.PollingConfig, + RootDevice: b.config.RootDevice, + LaunchDevices: launchDevices, + LaunchOmitMap: b.config.LaunchMappings.GetOmissions(), + Ctx: b.config.ctx, + } + + buildAmiStep = &StepCreateAMI{ + AMISkipBuildRegion: b.config.AMISkipBuildRegion, + RootDevice: b.config.RootDevice, + AMIDevices: amiDevices, + LaunchDevices: launchDevices, + PollingConfig: b.config.PollingConfig, + IsRestricted: b.config.IsChinaCloud() || b.config.IsGovCloud(), + Tags: b.config.RunTags, + Ctx: b.config.ctx, + } + } else { + volumeStep = &StepSnapshotVolumes{ PollingConfig: b.config.PollingConfig, LaunchDevices: launchDevices, SnapshotOmitMap: b.config.LaunchMappings.GetOmissions(), SnapshotTags: b.config.SnapshotTags, Ctx: b.config.ctx, } - imageMethod = &StepRegisterAMI{ + buildAmiStep = &StepRegisterAMI{ RootDevice: b.config.RootDevice, AMIDevices: amiDevices, LaunchDevices: launchDevices, @@ -343,25 +362,6 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) UefiData: b.config.UefiData, TpmSupport: b.config.TpmSupport, } - } else { - volumeMethod = &StepSwapVolumes{ - PollingConfig: b.config.PollingConfig, - RootDevice: b.config.RootDevice, - LaunchDevices: launchDevices, - LaunchOmitMap: b.config.LaunchMappings.GetOmissions(), - Ctx: b.config.ctx, - } - - imageMethod = &StepCreateAMI{ - AMISkipBuildRegion: b.config.AMISkipBuildRegion, - RootDevice: b.config.RootDevice, - AMIDevices: amiDevices, - LaunchDevices: launchDevices, - PollingConfig: b.config.PollingConfig, - IsRestricted: b.config.IsChinaCloud() || b.config.IsGovCloud(), - Tags: b.config.RunTags, - Ctx: b.config.ctx, - } } // Build the steps @@ -466,7 +466,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) EnableAMISriovNetSupport: b.config.AMISriovNetSupport, EnableAMIENASupport: b.config.AMIENASupport, }, - volumeMethod, + volumeStep, &awscommon.StepDeregisterAMI{ AccessConfig: &b.config.AccessConfig, ForceDeregister: b.config.AMIForceDeregister, @@ -474,7 +474,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) AMIName: b.config.AMIName, Regions: b.config.AMIRegions, }, - imageMethod, + buildAmiStep, &awscommon.StepAMIRegionCopy{ AccessConfig: &b.config.AccessConfig, Regions: b.config.AMIRegions, diff --git a/builder/ebssurrogate/builder_acc_test.go b/builder/ebssurrogate/builder_acc_test.go index 721d4f211..0e6b76259 100644 --- a/builder/ebssurrogate/builder_acc_test.go +++ b/builder/ebssurrogate/builder_acc_test.go @@ -109,14 +109,14 @@ func TestAccBuilder_Ebssurrogate_SSHPrivateKeyFile_SSM(t *testing.T) { acctest.TestPlugin(t, testcase) } -func TestAccBuilder_EbssurrogateImageMethodCreate(t *testing.T) { +func TestAccBuilder_EbssurrogateUseCreateImageTrue(t *testing.T) { ami := amazon_acc.AMIHelper{ Region: "us-east-1", Name: "ebs-image-method-create-acc-test", } testCase := &acctest.PluginTestCase{ Name: "amazon-ebssurrogate_image_method_create_test", - Template: fmt.Sprintf(testBuilderAccImageMethodCreate, ami.Name), + Template: fmt.Sprintf(testBuilderAccUseCreateImageTrue, ami.Name), Teardown: func() error { return ami.CleanUpAmi() }, @@ -132,14 +132,14 @@ func TestAccBuilder_EbssurrogateImageMethodCreate(t *testing.T) { acctest.TestPlugin(t, testCase) } -func TestAccBuilder_EbssurrogateImageMethodRegister(t *testing.T) { +func TestAccBuilder_EbssurrogateUseCreateImageFalse(t *testing.T) { ami := amazon_acc.AMIHelper{ Region: "us-east-1", Name: "ebs-image-method-register-acc-test", } testCase := &acctest.PluginTestCase{ Name: "amazon-ebssurrogate_image_method_register_test", - Template: fmt.Sprintf(testBuilderAccImageMethodRegister, ami.Name), + Template: fmt.Sprintf(testBuilderAccUseCreateImageFalse, ami.Name), Teardown: func() error { return ami.CleanUpAmi() }, @@ -155,14 +155,14 @@ func TestAccBuilder_EbssurrogateImageMethodRegister(t *testing.T) { acctest.TestPlugin(t, testCase) } -func TestAccBuilder_EbssurrogateImageMethodEmpty(t *testing.T) { +func TestAccBuilder_EbssurrogateUseCreateImageOptional(t *testing.T) { ami := amazon_acc.AMIHelper{ Region: "us-east-1", Name: "ebs-image-method-empty-acc-test", } testCase := &acctest.PluginTestCase{ Name: "amazon-ebssurrogate_image_method_empty_test", - Template: fmt.Sprintf(testBuilderAccImageMethodEmpty, ami.Name), + Template: fmt.Sprintf(testBuilderAccUseCreateImageOptional, ami.Name), Teardown: func() error { return ami.CleanUpAmi() }, @@ -267,7 +267,7 @@ build { } ` -const testBuilderAccImageMethodCreate = ` +const testBuilderAccUseCreateImageTrue = ` source "amazon-ebssurrogate" "test" { ami_name = "%s" region = "us-east-1" @@ -296,7 +296,7 @@ build { } ` -const testBuilderAccImageMethodRegister = ` +const testBuilderAccUseCreateImageFalse = ` source "amazon-ebssurrogate" "test" { ami_name = "%s" region = "us-east-1" @@ -316,7 +316,7 @@ source "amazon-ebssurrogate" "test" { delete_on_termination = true volume_size = 8 volume_type = "gp2" - image_method = "register" + use_create_image = false } } @@ -325,7 +325,7 @@ build { } ` -const testBuilderAccImageMethodEmpty = ` +const testBuilderAccUseCreateImageOptional = ` source "amazon-ebssurrogate" "test" { ami_name = "%s" region = "us-east-1" diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index faced6c54..d60ef7dea 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -37,13 +37,13 @@ type RootBlockDevice struct { // not specifying a snapshot_id. VolumeSize int64 `mapstructure:"volume_size" required:"false"` //Whether to use the CreateImage or RegisterImage API when creating the AMI. - //When set to `create`, CreateImage creates the image from the instance itself, - //and inherits properties from the instance. When set to `register`, the image - //is created from a snapshot of the specified EBS volume, and no properties - //are inherited from the instance. + //When set to `true`, the CreateImage API is used and will create the image + //from the instance itself, and inherit properties from the instance. + //When set to `false`, the RegisterImage API is used and the image is created using + // a snapshot of the specified EBS volume, and no properties are inherited from the instance. //Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html - ImageMethod string `mapstructure:"image_method" required:"false"` + UseCreateImage bool `mapstructure:"use_create_image" required:"false"` } func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { @@ -69,12 +69,6 @@ func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { errs = append(errs, errors.New("volume_size must be greater than 0")) } - if c.ImageMethod == "" { - c.ImageMethod = "register" - } else if c.ImageMethod != "create" && c.ImageMethod != "register" { - errs = append(errs, errors.New("image_method must be 'create', 'register' or an empty string' ")) - } - if len(errs) > 0 { return errs } From 502cca143423a20ca053918327d6fc095de56842 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Thu, 29 Feb 2024 14:15:31 -0500 Subject: [PATCH 14/19] Generate docs --- builder/ebssurrogate/builder.hcl2spec.go | 4 ++-- .../ebssurrogate/RootBlockDevice-not-required.mdx | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/builder/ebssurrogate/builder.hcl2spec.go b/builder/ebssurrogate/builder.hcl2spec.go index d9c152eef..83985f6aa 100644 --- a/builder/ebssurrogate/builder.hcl2spec.go +++ b/builder/ebssurrogate/builder.hcl2spec.go @@ -390,7 +390,7 @@ type FlatRootBlockDevice struct { IOPS *int64 `mapstructure:"iops" required:"false" cty:"iops" hcl:"iops"` VolumeType *string `mapstructure:"volume_type" required:"false" cty:"volume_type" hcl:"volume_type"` VolumeSize *int64 `mapstructure:"volume_size" required:"false" cty:"volume_size" hcl:"volume_size"` - ImageMethod *string `mapstructure:"image_method" required:"false" cty:"image_method" hcl:"image_method"` + UseCreateImage *bool `mapstructure:"use_create_image" required:"false" cty:"use_create_image" hcl:"use_create_image"` } // FlatMapstructure returns a new FlatRootBlockDevice. @@ -411,7 +411,7 @@ func (*FlatRootBlockDevice) HCL2Spec() map[string]hcldec.Spec { "iops": &hcldec.AttrSpec{Name: "iops", Type: cty.Number, Required: false}, "volume_type": &hcldec.AttrSpec{Name: "volume_type", Type: cty.String, Required: false}, "volume_size": &hcldec.AttrSpec{Name: "volume_size", Type: cty.Number, Required: false}, - "image_method": &hcldec.AttrSpec{Name: "image_method", Type: cty.String, Required: false}, + "use_create_image": &hcldec.AttrSpec{Name: "use_create_image", Type: cty.Bool, Required: false}, } return s } diff --git a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx index 636c7136c..ac2a79484 100644 --- a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx @@ -25,11 +25,11 @@ - `volume_size` (int64) - The size of the volume, in GiB. Required if not specifying a snapshot_id. -- `image_method` (string) - Whether to use the CreateImage or RegisterImage API when creating the AMI. - When set to `create`, CreateImage creates the image from the instance itself, - and inherits properties from the instance. When set to `register`, the image - is created from a snapshot of the specified EBS volume, and no properties - are inherited from the instance. +- `use_create_image` (bool) - Whether to use the CreateImage or RegisterImage API when creating the AMI. + When set to `true`, the CreateImage API is used and will create the image + from the instance itself, and inherit properties from the instance. + When set to `false`, the RegisterImage API is used and the image is created using + a snapshot of the specified EBS volume, and no properties are inherited from the instance. Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html From df8f2edccf1bb305ba4fa450bb925c535851f8d2 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Thu, 29 Feb 2024 15:07:46 -0500 Subject: [PATCH 15/19] Add default value comment --- builder/ebssurrogate/root_block_device.go | 9 +++++---- .../ebssurrogate/RootBlockDevice-not-required.mdx | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index d60ef7dea..0b7a29d13 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -36,11 +36,12 @@ type RootBlockDevice struct { // The size of the volume, in GiB. Required if // not specifying a snapshot_id. VolumeSize int64 `mapstructure:"volume_size" required:"false"` - //Whether to use the CreateImage or RegisterImage API when creating the AMI. - //When set to `true`, the CreateImage API is used and will create the image - //from the instance itself, and inherit properties from the instance. - //When set to `false`, the RegisterImage API is used and the image is created using + // Whether to use the CreateImage or RegisterImage API when creating the AMI. + // When set to `true`, the CreateImage API is used and will create the image + // from the instance itself, and inherit properties from the instance. + // When set to `false`, the RegisterImage API is used and the image is created using // a snapshot of the specified EBS volume, and no properties are inherited from the instance. + // Defaults to `false`. //Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html UseCreateImage bool `mapstructure:"use_create_image" required:"false"` diff --git a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx index ac2a79484..ea40c904e 100644 --- a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx @@ -30,6 +30,7 @@ from the instance itself, and inherit properties from the instance. When set to `false`, the RegisterImage API is used and the image is created using a snapshot of the specified EBS volume, and no properties are inherited from the instance. + Defaults to `false`. Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html From 4c7c715a509502b0d8c6d3f56b65c4f86505bb52 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Thu, 29 Feb 2024 16:34:54 -0500 Subject: [PATCH 16/19] Rename method to match go standards --- builder/ebssurrogate/step_swap_volumes.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builder/ebssurrogate/step_swap_volumes.go b/builder/ebssurrogate/step_swap_volumes.go index 6674bd41b..7a19c02b2 100644 --- a/builder/ebssurrogate/step_swap_volumes.go +++ b/builder/ebssurrogate/step_swap_volumes.go @@ -59,10 +59,10 @@ func (s *StepSwapVolumes) Run(ctx context.Context, state multistep.StateBag) mul omit, ok := s.LaunchOmitMap[deviceName] if ok && omit { ui.Say(fmt.Sprintf("Detaching Ommitted EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) - err = s.detach_volume(ctx, ec2conn, deviceName, volumeID) + err = s.detachVolume(ctx, ec2conn, deviceName, volumeID) } else if deviceName == s.RootDevice.DeviceName || deviceName == s.RootDevice.SourceDeviceName || deviceName == "/dev/sda1" { ui.Say(fmt.Sprintf("Detaching Root EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) - err = s.detach_volume(ctx, ec2conn, deviceName, volumeID) + err = s.detachVolume(ctx, ec2conn, deviceName, volumeID) } else { ui.Say(fmt.Sprintf("Skip Detach of EBS Device Name: %s, Volume ID: %s\n", deviceName, volumeID)) } @@ -105,7 +105,7 @@ func (s *StepSwapVolumes) Run(ctx context.Context, state multistep.StateBag) mul return multistep.ActionContinue } -func (s *StepSwapVolumes) detach_volume(ctx context.Context, ec2conn *ec2.EC2, deviceName string, volumeId string) error { +func (s *StepSwapVolumes) detachVolume(ctx context.Context, ec2conn *ec2.EC2, deviceName string, volumeId string) error { _, err := ec2conn.DetachVolume(&ec2.DetachVolumeInput{VolumeId: &volumeId}) if err == nil { return s.PollingConfig.WaitUntilVolumeDetached(ctx, ec2conn, volumeId) From 647c83ab39e0247862107125bcf571e08d7d3d96 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 4 Mar 2024 09:14:47 -0500 Subject: [PATCH 17/19] Refactor to move use_create_image to ebssurrogate --- builder/ebssurrogate/builder.go | 11 ++++++++++- builder/ebssurrogate/builder.hcl2spec.go | 2 -- builder/ebssurrogate/builder_acc_test.go | 4 ++-- builder/ebssurrogate/root_block_device.go | 9 --------- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/builder/ebssurrogate/builder.go b/builder/ebssurrogate/builder.go index 5393b3774..c35945def 100644 --- a/builder/ebssurrogate/builder.go +++ b/builder/ebssurrogate/builder.go @@ -85,6 +85,15 @@ type Config struct { // [NitroTPM Support](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enable-nitrotpm-support-on-ami.html) for // more information. Only enabled if a valid option is provided, otherwise ignored. TpmSupport string `mapstructure:"tpm_support" required:"false"` + // Whether to use the CreateImage or RegisterImage API when creating the AMI. + // When set to `true`, the CreateImage API is used and will create the image + // from the instance itself, and inherit properties from the instance. + // When set to `false`, the RegisterImage API is used and the image is created using + // a snapshot of the specified EBS volume, and no properties are inherited from the instance. + // Defaults to `false`. + //Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html + // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html + UseCreateImage bool `mapstructure:"use_create_image" required:"false"` ctx interpolate.Context } @@ -321,7 +330,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) var buildAmiStep multistep.Step var volumeStep multistep.Step - if b.config.RootDevice.UseCreateImage { + if b.config.UseCreateImage { volumeStep = &StepSwapVolumes{ PollingConfig: b.config.PollingConfig, RootDevice: b.config.RootDevice, diff --git a/builder/ebssurrogate/builder.hcl2spec.go b/builder/ebssurrogate/builder.hcl2spec.go index 83985f6aa..3cda9cd1a 100644 --- a/builder/ebssurrogate/builder.hcl2spec.go +++ b/builder/ebssurrogate/builder.hcl2spec.go @@ -390,7 +390,6 @@ type FlatRootBlockDevice struct { IOPS *int64 `mapstructure:"iops" required:"false" cty:"iops" hcl:"iops"` VolumeType *string `mapstructure:"volume_type" required:"false" cty:"volume_type" hcl:"volume_type"` VolumeSize *int64 `mapstructure:"volume_size" required:"false" cty:"volume_size" hcl:"volume_size"` - UseCreateImage *bool `mapstructure:"use_create_image" required:"false" cty:"use_create_image" hcl:"use_create_image"` } // FlatMapstructure returns a new FlatRootBlockDevice. @@ -411,7 +410,6 @@ func (*FlatRootBlockDevice) HCL2Spec() map[string]hcldec.Spec { "iops": &hcldec.AttrSpec{Name: "iops", Type: cty.Number, Required: false}, "volume_type": &hcldec.AttrSpec{Name: "volume_type", Type: cty.String, Required: false}, "volume_size": &hcldec.AttrSpec{Name: "volume_size", Type: cty.Number, Required: false}, - "use_create_image": &hcldec.AttrSpec{Name: "use_create_image", Type: cty.Bool, Required: false}, } return s } diff --git a/builder/ebssurrogate/builder_acc_test.go b/builder/ebssurrogate/builder_acc_test.go index 0e6b76259..e277726ce 100644 --- a/builder/ebssurrogate/builder_acc_test.go +++ b/builder/ebssurrogate/builder_acc_test.go @@ -274,6 +274,7 @@ source "amazon-ebssurrogate" "test" { instance_type = "m3.medium" source_ami = "ami-76b2a71e" ssh_username = "ubuntu" + use_create_image = true launch_block_device_mappings { device_name = "/dev/xvda" delete_on_termination = true @@ -287,7 +288,6 @@ source "amazon-ebssurrogate" "test" { delete_on_termination = true volume_size = 8 volume_type = "gp2" - image_method = "create" } } @@ -303,6 +303,7 @@ source "amazon-ebssurrogate" "test" { instance_type = "m3.medium" source_ami = "ami-76b2a71e" ssh_username = "ubuntu" + use_create_image = false launch_block_device_mappings { device_name = "/dev/xvda" delete_on_termination = true @@ -316,7 +317,6 @@ source "amazon-ebssurrogate" "test" { delete_on_termination = true volume_size = 8 volume_type = "gp2" - use_create_image = false } } diff --git a/builder/ebssurrogate/root_block_device.go b/builder/ebssurrogate/root_block_device.go index 0b7a29d13..0373dfbf1 100644 --- a/builder/ebssurrogate/root_block_device.go +++ b/builder/ebssurrogate/root_block_device.go @@ -36,15 +36,6 @@ type RootBlockDevice struct { // The size of the volume, in GiB. Required if // not specifying a snapshot_id. VolumeSize int64 `mapstructure:"volume_size" required:"false"` - // Whether to use the CreateImage or RegisterImage API when creating the AMI. - // When set to `true`, the CreateImage API is used and will create the image - // from the instance itself, and inherit properties from the instance. - // When set to `false`, the RegisterImage API is used and the image is created using - // a snapshot of the specified EBS volume, and no properties are inherited from the instance. - // Defaults to `false`. - //Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html - // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html - UseCreateImage bool `mapstructure:"use_create_image" required:"false"` } func (c *RootBlockDevice) Prepare(ctx *interpolate.Context) []error { From 0bc3e263bfa4a6e54b0bfbe106161bfa64074e7a Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 4 Mar 2024 09:16:59 -0500 Subject: [PATCH 18/19] Generate docs --- builder/ebssurrogate/builder.hcl2spec.go | 2 ++ .../builder/ebssurrogate/Config-not-required.mdx | 9 +++++++++ .../ebssurrogate/RootBlockDevice-not-required.mdx | 9 --------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/builder/ebssurrogate/builder.hcl2spec.go b/builder/ebssurrogate/builder.hcl2spec.go index 3cda9cd1a..3031fbb49 100644 --- a/builder/ebssurrogate/builder.hcl2spec.go +++ b/builder/ebssurrogate/builder.hcl2spec.go @@ -210,6 +210,7 @@ type FlatConfig struct { BootMode *string `mapstructure:"boot_mode" required:"false" cty:"boot_mode" hcl:"boot_mode"` UefiData *string `mapstructure:"uefi_data" required:"false" cty:"uefi_data" hcl:"uefi_data"` TpmSupport *string `mapstructure:"tpm_support" required:"false" cty:"tpm_support" hcl:"tpm_support"` + UseCreateImage *bool `mapstructure:"use_create_image" required:"false" cty:"use_create_image" hcl:"use_create_image"` } // FlatMapstructure returns a new FlatConfig. @@ -377,6 +378,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "boot_mode": &hcldec.AttrSpec{Name: "boot_mode", Type: cty.String, Required: false}, "uefi_data": &hcldec.AttrSpec{Name: "uefi_data", Type: cty.String, Required: false}, "tpm_support": &hcldec.AttrSpec{Name: "tpm_support", Type: cty.String, Required: false}, + "use_create_image": &hcldec.AttrSpec{Name: "use_create_image", Type: cty.Bool, Required: false}, } return s } diff --git a/docs-partials/builder/ebssurrogate/Config-not-required.mdx b/docs-partials/builder/ebssurrogate/Config-not-required.mdx index c0c8187b2..77358a1d6 100644 --- a/docs-partials/builder/ebssurrogate/Config-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/Config-not-required.mdx @@ -42,4 +42,13 @@ [NitroTPM Support](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enable-nitrotpm-support-on-ami.html) for more information. Only enabled if a valid option is provided, otherwise ignored. +- `use_create_image` (bool) - Whether to use the CreateImage or RegisterImage API when creating the AMI. + When set to `true`, the CreateImage API is used and will create the image + from the instance itself, and inherit properties from the instance. + When set to `false`, the RegisterImage API is used and the image is created using + a snapshot of the specified EBS volume, and no properties are inherited from the instance. + Defaults to `false`. + Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html + https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html + diff --git a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx index ea40c904e..c8d6e7be5 100644 --- a/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx +++ b/docs-partials/builder/ebssurrogate/RootBlockDevice-not-required.mdx @@ -25,13 +25,4 @@ - `volume_size` (int64) - The size of the volume, in GiB. Required if not specifying a snapshot_id. -- `use_create_image` (bool) - Whether to use the CreateImage or RegisterImage API when creating the AMI. - When set to `true`, the CreateImage API is used and will create the image - from the instance itself, and inherit properties from the instance. - When set to `false`, the RegisterImage API is used and the image is created using - a snapshot of the specified EBS volume, and no properties are inherited from the instance. - Defaults to `false`. - Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html - https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html - From f871d4384c5c15f8939eacce3f1c36e00adeb905 Mon Sep 17 00:00:00 2001 From: Dennis Carey Date: Mon, 4 Mar 2024 09:35:03 -0500 Subject: [PATCH 19/19] Commit hidden dir --- .web-docs/components/builder/ebssurrogate/README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.web-docs/components/builder/ebssurrogate/README.md b/.web-docs/components/builder/ebssurrogate/README.md index c8c131466..fbd29b5e9 100644 --- a/.web-docs/components/builder/ebssurrogate/README.md +++ b/.web-docs/components/builder/ebssurrogate/README.md @@ -86,6 +86,15 @@ necessary for this build to succeed and can be found further down the page. [NitroTPM Support](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/enable-nitrotpm-support-on-ami.html) for more information. Only enabled if a valid option is provided, otherwise ignored. +- `use_create_image` (bool) - Whether to use the CreateImage or RegisterImage API when creating the AMI. + When set to `true`, the CreateImage API is used and will create the image + from the instance itself, and inherit properties from the instance. + When set to `false`, the RegisterImage API is used and the image is created using + a snapshot of the specified EBS volume, and no properties are inherited from the instance. + Defaults to `false`. + Ref: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateImage.html + https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_RegisterImage.html +