diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index 3b1b4609e..aa9e7ddd7 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -167,11 +167,20 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend( errs, errors.New("source_ami or source_ami_filter is required.")) } - if len(b.config.AMIMappings) != 0 { - warns = append(warns, "ami_block_device_mappings are unused when from_scratch is false") - } - if b.config.RootDeviceName != "" { - warns = append(warns, "root_device_name is unused when from_scratch is false") + if len(b.config.AMIMappings) > 0 && b.config.RootDeviceName != "" { + if b.config.RootVolumeSize == 0 { + // Although, they can specify the device size in the block device mapping, it's easier to + // be specific here. + errs = packer.MultiErrorAppend( + errs, errors.New("root_volume_size is required if ami_block_device_mappings is specified")) + } + warns = append(warns, "ami_block_device_mappings from source image will be completely overwritten") + } else if len(b.config.AMIMappings) > 0 { + errs = packer.MultiErrorAppend( + errs, errors.New("If ami_block_device_mappings is specified, root_device_name must be specified")) + } else if b.config.RootDeviceName != "" { + errs = packer.MultiErrorAppend( + errs, errors.New("If root_device_name is specified, ami_block_device_mappings must be specified")) } } diff --git a/builder/amazon/chroot/builder_test.go b/builder/amazon/chroot/builder_test.go index b4bb0ab76..1fcc905f1 100644 --- a/builder/amazon/chroot/builder_test.go +++ b/builder/amazon/chroot/builder_test.go @@ -162,3 +162,49 @@ func TestBuilderPrepare_CopyFilesNoDefault(t *testing.T) { t.Errorf("Was expecting no default value for copy_files.") } } + +func TestBuilderPrepare_RootDeviceNameAndAMIMappings(t *testing.T) { + var b Builder + config := testConfig() + + config["root_device_name"] = "/dev/sda" + config["ami_block_device_mappings"] = []interface{}{map[string]string{}} + config["root_volume_size"] = 15 + warnings, err := b.Prepare(config) + if len(warnings) == 0 { + t.Fatal("Missing warning, stating block device mappings will be overwritten") + } else if len(warnings) > 1 { + t.Fatalf("excessive warnings: %#v", warnings) + } + if err != nil { + t.Fatalf("should not have error: %s", err) + } +} + +func TestBuilderPrepare_AMIMappingsNoRootDeviceName(t *testing.T) { + var b Builder + config := testConfig() + + config["ami_block_device_mappings"] = []interface{}{map[string]string{}} + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } + if err == nil { + t.Fatalf("should have error") + } +} + +func TestBuilderPrepare_RootDeviceNameNoAMIMappings(t *testing.T) { + var b Builder + config := testConfig() + + config["root_device_name"] = "/dev/sda" + warnings, err := b.Prepare(config) + if len(warnings) > 0 { + t.Fatalf("bad: %#v", warnings) + } + if err == nil { + t.Fatalf("should have error") + } +} diff --git a/builder/amazon/chroot/step_register_ami.go b/builder/amazon/chroot/step_register_ami.go index c3eac6c14..da60e917a 100644 --- a/builder/amazon/chroot/step_register_ami.go +++ b/builder/amazon/chroot/step_register_ami.go @@ -80,7 +80,8 @@ func buildBaseRegisterOpts(config *Config, sourceImage *ec2.Image, rootVolumeSiz rootDeviceName string ) - if config.FromScratch { + generatingNewBlockDeviceMappings := config.FromScratch || len(config.AMIMappings) > 0 + if generatingNewBlockDeviceMappings { mappings = config.AMIBlockDevices.BuildAMIDevices() rootDeviceName = config.RootDeviceName } else { @@ -99,7 +100,7 @@ func buildBaseRegisterOpts(config *Config, sourceImage *ec2.Image, rootVolumeSiz newDevice.Ebs = &ec2.EbsBlockDevice{SnapshotId: aws.String(snapshotID)} } - if config.FromScratch || rootVolumeSize > *newDevice.Ebs.VolumeSize { + if generatingNewBlockDeviceMappings || rootVolumeSize > *newDevice.Ebs.VolumeSize { newDevice.Ebs.VolumeSize = aws.Int64(rootVolumeSize) } } @@ -123,14 +124,14 @@ func buildBaseRegisterOpts(config *Config, sourceImage *ec2.Image, rootVolumeSiz } } - return buildRegisterOpts(config, sourceImage, newMappings) + return buildRegisterOptsFromExistingImage(config, sourceImage, newMappings, rootDeviceName) } -func buildRegisterOpts(config *Config, image *ec2.Image, mappings []*ec2.BlockDeviceMapping) *ec2.RegisterImageInput { +func buildRegisterOptsFromExistingImage(config *Config, image *ec2.Image, mappings []*ec2.BlockDeviceMapping, rootDeviceName string) *ec2.RegisterImageInput { registerOpts := &ec2.RegisterImageInput{ Name: &config.AMIName, Architecture: image.Architecture, - RootDeviceName: image.RootDeviceName, + RootDeviceName: &rootDeviceName, BlockDeviceMappings: mappings, VirtualizationType: image.VirtualizationType, } diff --git a/builder/amazon/chroot/step_register_ami_test.go b/builder/amazon/chroot/step_register_ami_test.go index 7187ff674..8c686b4bc 100644 --- a/builder/amazon/chroot/step_register_ami_test.go +++ b/builder/amazon/chroot/step_register_ami_test.go @@ -23,12 +23,13 @@ func TestStepRegisterAmi_buildRegisterOpts_pv(t *testing.T) { config.AMIName = "test_ami_name" config.AMIDescription = "test_ami_description" config.AMIVirtType = "paravirtual" + rootDeviceName := "foo" image := testImage() blockDevices := []*ec2.BlockDeviceMapping{} - opts := buildRegisterOpts(&config, &image, blockDevices) + opts := buildRegisterOptsFromExistingImage(&config, &image, blockDevices, rootDeviceName) expected := config.AMIVirtType if *opts.VirtualizationType != expected { @@ -45,6 +46,10 @@ func TestStepRegisterAmi_buildRegisterOpts_pv(t *testing.T) { t.Fatalf("Unexpected KernelId value: expected %s got %s\n", expected, *opts.KernelId) } + expected = rootDeviceName + if *opts.RootDeviceName != expected { + t.Fatalf("Unexpected RootDeviceName value: expected %s got %s\n", expected, *opts.RootDeviceName) + } } func TestStepRegisterAmi_buildRegisterOpts_hvm(t *testing.T) { @@ -52,12 +57,13 @@ func TestStepRegisterAmi_buildRegisterOpts_hvm(t *testing.T) { config.AMIName = "test_ami_name" config.AMIDescription = "test_ami_description" config.AMIVirtType = "hvm" + rootDeviceName := "foo" image := testImage() blockDevices := []*ec2.BlockDeviceMapping{} - opts := buildRegisterOpts(&config, &image, blockDevices) + opts := buildRegisterOptsFromExistingImage(&config, &image, blockDevices, rootDeviceName) expected := config.AMIVirtType if *opts.VirtualizationType != expected { @@ -72,6 +78,11 @@ func TestStepRegisterAmi_buildRegisterOpts_hvm(t *testing.T) { if opts.KernelId != nil { t.Fatalf("Unexpected KernelId value: expected nil got %s\n", *opts.KernelId) } + + expected = rootDeviceName + if *opts.RootDeviceName != expected { + t.Fatalf("Unexpected RootDeviceName value: expected %s got %s\n", expected, *opts.RootDeviceName) + } } func TestStepRegisterAmi_buildRegisterOptsFromScratch(t *testing.T) { @@ -146,3 +157,63 @@ func TestStepRegisterAmi_buildRegisterOptFromExistingImage(t *testing.T) { } t.Fatalf("Could not find device with snapshot ID %s", snapshotID) } + +func TestStepRegisterAmi_buildRegisterOptFromExistingImageWithBlockDeviceMappings(t *testing.T) { + const ( + rootDeviceName = "/dev/xvda" + oldRootDevice = "/dev/sda1" + ) + snapshotId := "foo" + + config := Config{ + FromScratch: false, + PackerConfig: common.PackerConfig{}, + AMIBlockDevices: amazon.AMIBlockDevices{ + AMIMappings: []amazon.BlockDevice{ + { + DeviceName: rootDeviceName, + }, + }, + }, + RootDeviceName: rootDeviceName, + } + + // Intentionally try to use a different root devicename + sourceImage := ec2.Image{ + RootDeviceName: aws.String(oldRootDevice), + BlockDeviceMappings: []*ec2.BlockDeviceMapping{ + { + DeviceName: aws.String(oldRootDevice), + Ebs: &ec2.EbsBlockDevice{ + VolumeSize: aws.Int64(10), + }, + }, + // Throw in an ephemeral device, it seems like all devices in the return struct in a source AMI have + // a size, even if it's for ephemeral + { + DeviceName: aws.String("/dev/sdb"), + VirtualName: aws.String("ephemeral0"), + Ebs: &ec2.EbsBlockDevice{ + VolumeSize: aws.Int64(0), + }, + }, + }, + } + registerOpts := buildBaseRegisterOpts(&config, &sourceImage, 15, snapshotId) + + if len(registerOpts.BlockDeviceMappings) != 1 { + t.Fatal("Expected block device mapping of length 1") + } + + if *registerOpts.BlockDeviceMappings[0].Ebs.SnapshotId != snapshotId { + t.Fatalf("Snapshot ID of root disk set to '%s' expected '%s'", *registerOpts.BlockDeviceMappings[0].Ebs.SnapshotId, rootDeviceName) + } + + if *registerOpts.RootDeviceName != rootDeviceName { + t.Fatalf("Root device set to '%s' expected %s", *registerOpts.RootDeviceName, rootDeviceName) + } + + if *registerOpts.BlockDeviceMappings[0].Ebs.VolumeSize != 15 { + t.Fatalf("Size of root disk not set to 15 GB, instead %d", *registerOpts.BlockDeviceMappings[0].Ebs.VolumeSize) + } +}