From 2e65867cbad75dcae61c64d8b97d1e4a572b1b72 Mon Sep 17 00:00:00 2001 From: Arthur Burkart Date: Sun, 16 Oct 2016 22:19:55 -0400 Subject: [PATCH 1/2] Closes #3908: Adds snapshot tag overrides This commit adds the ability to configure unique tags on snapshots that are separate from the tags defined on the AMI. Anything applied to the AMI will also be applied to the snapshots, but `snapshot_tags` will override and append tags to the tags already applied to the snapshots --- builder/amazon/chroot/builder.go | 3 +- builder/amazon/common/ami_config.go | 1 + builder/amazon/common/step_create_tags.go | 48 ++++++++++++++----- builder/amazon/ebs/builder.go | 3 +- builder/amazon/instance/builder.go | 3 +- .../docs/builders/amazon-chroot.html.md | 3 ++ .../source/docs/builders/amazon-ebs.html.md | 5 +- .../docs/builders/amazon-instance.html.md | 7 ++- 8 files changed, 55 insertions(+), 18 deletions(-) diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index 52aee12d9..c9f56289a 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -262,7 +262,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe ProductCodes: b.config.AMIProductCodes, }, &awscommon.StepCreateTags{ - Tags: b.config.AMITags, + Tags: b.config.AMITags, + SnapshotTags: b.config.SnapshotTags, }, ) diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 3bbe8039a..28eafb7fd 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -20,6 +20,7 @@ type AMIConfig struct { AMIEnhancedNetworking bool `mapstructure:"enhanced_networking"` AMIForceDeregister bool `mapstructure:"force_deregister"` AMIEncryptBootVolume bool `mapstructure:"encrypt_boot"` + SnapshotTags map[string]string `mapstructure:"snapshot_tags"` } func (c *AMIConfig) Prepare(ctx *interpolate.Context) []error { diff --git a/builder/amazon/common/step_create_tags.go b/builder/amazon/common/step_create_tags.go index 8764968d7..925bf4362 100644 --- a/builder/amazon/common/step_create_tags.go +++ b/builder/amazon/common/step_create_tags.go @@ -13,7 +13,8 @@ import ( ) type StepCreateTags struct { - Tags map[string]string + Tags map[string]string + SnapshotTags map[string]string } func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { @@ -21,21 +22,15 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packer.Ui) amis := state.Get("amis").(map[string]string) - if len(s.Tags) > 0 { + if len(s.Tags) > 0 || len(s.SnapshotTags) > 0 { for region, ami := range amis { ui.Say(fmt.Sprintf("Adding tags to AMI (%s)...", ami)) - var ec2Tags []*ec2.Tag - for key, value := range s.Tags { - ui.Message(fmt.Sprintf("Adding tag: \"%s\": \"%s\"", key, value)) - ec2Tags = append(ec2Tags, &ec2.Tag{ - Key: aws.String(key), - Value: aws.String(value), - }) - } + // Convert tags to ec2.Tag format + ec2Tags := ConvertToEC2Tags(s.Tags, ui) + snapshotTags := ConvertToEC2Tags(s.SnapshotTags, ui) // Declare list of resources to tag - resourceIds := []*string{&ami} awsConfig := aws.Config{ Credentials: ec2conn.Config.Credentials, Region: aws.String(region), @@ -47,10 +42,10 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { ui.Error(err.Error()) return multistep.ActionHalt } - regionconn := ec2.New(session) // Retrieve image list for given AMI + resourceIds := []*string{&ami} imageResp, err := regionconn.DescribeImages(&ec2.DescribeImagesInput{ ImageIds: resourceIds, }) @@ -70,17 +65,20 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { } image := imageResp.Images[0] + snapshotIds := []*string{} // Add only those with a Snapshot ID, i.e. not Ephemeral for _, device := range image.BlockDeviceMappings { if device.Ebs != nil && device.Ebs.SnapshotId != nil { ui.Say(fmt.Sprintf("Tagging snapshot: %s", *device.Ebs.SnapshotId)) resourceIds = append(resourceIds, device.Ebs.SnapshotId) + snapshotIds = append(snapshotIds, device.Ebs.SnapshotId) } } // Retry creating tags for about 2.5 minutes err = retry.Retry(0.2, 30, 11, func() (bool, error) { + // Tag images and snapshots _, err := regionconn.CreateTags(&ec2.CreateTagsInput{ Resources: resourceIds, Tags: ec2Tags, @@ -94,6 +92,20 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { return false, nil } } + + // Override tags on snapshots + _, err = regionconn.CreateTags(&ec2.CreateTagsInput{ + Resources: snapshotIds, + Tags: snapshotTags, + }) + if err == nil { + return true, nil + } + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "InvalidSnapshot.NotFound" { + return false, nil + } + } return true, err }) @@ -112,3 +124,15 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { func (s *StepCreateTags) Cleanup(state multistep.StateBag) { // No cleanup... } + +func ConvertToEC2Tags(tags map[string]string, ui packer.Ui) []*ec2.Tag { + var ec2Tags []*ec2.Tag + for key, value := range tags { + ui.Message(fmt.Sprintf("Adding tag: \"%s\": \"%s\"", key, value)) + ec2Tags = append(ec2Tags, &ec2.Tag{ + Key: aws.String(key), + Value: aws.String(value), + }) + } + return ec2Tags +} diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index 4c397694c..dd4ff246d 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -182,7 +182,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe ProductCodes: b.config.AMIProductCodes, }, &awscommon.StepCreateTags{ - Tags: b.config.AMITags, + Tags: b.config.AMITags, + SnapshotTags: b.config.SnapshotTags, }, } diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index 319c4f4a3..230dcaf34 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -263,7 +263,8 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe ProductCodes: b.config.AMIProductCodes, }, &awscommon.StepCreateTags{ - Tags: b.config.AMITags, + Tags: b.config.AMITags, + SnapshotTags: b.config.SnapshotTags, }, } diff --git a/website/source/docs/builders/amazon-chroot.html.md b/website/source/docs/builders/amazon-chroot.html.md index 799fdc01a..210fa4c73 100644 --- a/website/source/docs/builders/amazon-chroot.html.md +++ b/website/source/docs/builders/amazon-chroot.html.md @@ -200,6 +200,9 @@ each category, the available configuration keys are alphabetized. - `most_recent` (bool) - Selects the newest created image when true. This is most useful for selecting a daily distro build. +- `snapshot_tags` (object of key/value strings) - Tags to apply to snapshot. + They will override AMI tags if already applied to snapshot. + - `tags` (object of key/value strings) - Tags applied to the AMI. ## Basic Example diff --git a/website/source/docs/builders/amazon-ebs.html.md b/website/source/docs/builders/amazon-ebs.html.md index bc3c560ad..54bf472dd 100644 --- a/website/source/docs/builders/amazon-ebs.html.md +++ b/website/source/docs/builders/amazon-ebs.html.md @@ -172,7 +172,7 @@ builder. described above. Note that if this is specified, you must omit the `security_group_id`. -- `skip_region_validation` (boolean) - Set to true if you want to skip +- `skip_region_validation` (boolean) - Set to true if you want to skip validation of the region configuration option. Defaults to false. - `source_ami_filter` (object) - Filters used to populate the `source_ami` field. @@ -203,6 +203,9 @@ builder. - `most_recent` (bool) - Selects the newest created image when true. This is most useful for selecting a daily distro build. +- `snapshot_tags` (object of key/value strings) - Tags to apply to snapshot. + They will override AMI tags if already applied to snapshot. + - `spot_price` (string) - The maximum hourly price to pay for a spot instance to create the AMI. Spot instances are a type of instance that EC2 starts when the current spot price is less than the maximum price you specify. Spot diff --git a/website/source/docs/builders/amazon-instance.html.md b/website/source/docs/builders/amazon-instance.html.md index 9bc526e52..75e1cb246 100644 --- a/website/source/docs/builders/amazon-instance.html.md +++ b/website/source/docs/builders/amazon-instance.html.md @@ -188,7 +188,7 @@ builder. described above. Note that if this is specified, you must omit the `security_group_id`. -- `skip_region_validation` (boolean) - Set to true if you want to skip +- `skip_region_validation` (boolean) - Set to true if you want to skip validation of the region configuration option. Defaults to false. - `source_ami_filter` (object) - Filters used to populate the `source_ami` field. @@ -219,6 +219,9 @@ builder. - `most_recent` (bool) - Selects the newest created image when true. This is most useful for selecting a daily distro build. +- `snapshot_tags` (object of key/value strings) - Tags to apply to snapshot. + They will override AMI tags if already applied to snapshot. + - `spot_price` (string) - The maximum hourly price to launch a spot instance to create the AMI. It is a type of instances that EC2 starts when the maximum price that you specify exceeds the current spot price. Spot price @@ -247,7 +250,7 @@ builder. AMI, leave the `ssh_keypair_name` blank. To associate an existing key pair in AWS with the source instance, set the `ssh_keypair_name` field to the name of the key pair. - + - `ssh_private_ip` (boolean) - If true, then SSH will always use the private IP if available. From 0c7e73b1cf46e48460c1c2414f51b399297617b9 Mon Sep 17 00:00:00 2001 From: Arthur Burkart Date: Tue, 18 Oct 2016 22:46:41 -0400 Subject: [PATCH 2/2] Implements Snapshot tagging While implementing my acceptance test, I stumbled upon a comment stating that snapshot deletion should also be implemented, so I snuck that in. I can't help but wonder if there is some generic logic that is implemented a few times throughout the packer code base that could maybe better serve us if it were abstracted to the common package. --- builder/amazon/common/artifact.go | 13 ++ builder/amazon/common/step_create_tags.go | 171 +++++++++++----------- builder/amazon/ebs/tags_acc_test.go | 43 ++++-- 3 files changed, 132 insertions(+), 95 deletions(-) diff --git a/builder/amazon/common/artifact.go b/builder/amazon/common/artifact.go index bee5fe604..28331a4ec 100644 --- a/builder/amazon/common/artifact.go +++ b/builder/amazon/common/artifact.go @@ -79,6 +79,19 @@ func (a *Artifact) Destroy() error { } regionConn := ec2.New(session) + // Get image metadata + imageResp, err := regionConn.DescribeImages(&ec2.DescribeImagesInput{ + ImageIds: []*string{&imageId}, + }) + if err != nil { + errors = append(errors, err) + } + if len(imageResp.Images) == 0 { + err := fmt.Errorf("Error retrieving details for AMI (%s), no images found", imageId) + errors = append(errors, err) + } + + // Deregister ami input := &ec2.DeregisterImageInput{ ImageId: &imageId, } diff --git a/builder/amazon/common/step_create_tags.go b/builder/amazon/common/step_create_tags.go index 925bf4362..a8234b5d3 100644 --- a/builder/amazon/common/step_create_tags.go +++ b/builder/amazon/common/step_create_tags.go @@ -22,99 +22,100 @@ func (s *StepCreateTags) Run(state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packer.Ui) amis := state.Get("amis").(map[string]string) - if len(s.Tags) > 0 || len(s.SnapshotTags) > 0 { - for region, ami := range amis { - ui.Say(fmt.Sprintf("Adding tags to AMI (%s)...", ami)) - - // Convert tags to ec2.Tag format - ec2Tags := ConvertToEC2Tags(s.Tags, ui) - snapshotTags := ConvertToEC2Tags(s.SnapshotTags, ui) - - // Declare list of resources to tag - awsConfig := aws.Config{ - Credentials: ec2conn.Config.Credentials, - Region: aws.String(region), - } - session, err := session.NewSession(&awsConfig) - if err != nil { - err := fmt.Errorf("Error creating AWS session: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } - regionconn := ec2.New(session) + if len(s.Tags) == 0 && len(s.SnapshotTags) == 0 { + return multistep.ActionContinue + } - // Retrieve image list for given AMI - resourceIds := []*string{&ami} - imageResp, err := regionconn.DescribeImages(&ec2.DescribeImagesInput{ - ImageIds: resourceIds, - }) + // Adds tags to AMIs and snapshots + for region, ami := range amis { + ui.Say(fmt.Sprintf("Adding tags to AMI (%s)...", ami)) - if err != nil { - err := fmt.Errorf("Error retrieving details for AMI (%s): %s", ami, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } + // Convert tags to ec2.Tag format + amiTags := ConvertToEC2Tags(s.Tags, ui) + ui.Say(fmt.Sprintf("Snapshot tags:")) + snapshotTags := ConvertToEC2Tags(s.SnapshotTags, ui) - if len(imageResp.Images) == 0 { - err := fmt.Errorf("Error retrieving details for AMI (%s), no images found", ami) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt - } + // Declare list of resources to tag + awsConfig := aws.Config{ + Credentials: ec2conn.Config.Credentials, + Region: aws.String(region), + } + session, err := session.NewSession(&awsConfig) + if err != nil { + err := fmt.Errorf("Error creating AWS session: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + regionconn := ec2.New(session) - image := imageResp.Images[0] - snapshotIds := []*string{} + // Retrieve image list for given AMI + resourceIds := []*string{&ami} + imageResp, err := regionconn.DescribeImages(&ec2.DescribeImagesInput{ + ImageIds: resourceIds, + }) - // Add only those with a Snapshot ID, i.e. not Ephemeral - for _, device := range image.BlockDeviceMappings { - if device.Ebs != nil && device.Ebs.SnapshotId != nil { - ui.Say(fmt.Sprintf("Tagging snapshot: %s", *device.Ebs.SnapshotId)) - resourceIds = append(resourceIds, device.Ebs.SnapshotId) - snapshotIds = append(snapshotIds, device.Ebs.SnapshotId) - } + if err != nil { + err := fmt.Errorf("Error retrieving details for AMI (%s): %s", ami, err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + if len(imageResp.Images) == 0 { + err := fmt.Errorf("Error retrieving details for AMI (%s), no images found", ami) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + image := imageResp.Images[0] + snapshotIds := []*string{} + + // Add only those with a Snapshot ID, i.e. not Ephemeral + for _, device := range image.BlockDeviceMappings { + if device.Ebs != nil && device.Ebs.SnapshotId != nil { + ui.Say(fmt.Sprintf("Tagging snapshot: %s", *device.Ebs.SnapshotId)) + resourceIds = append(resourceIds, device.Ebs.SnapshotId) + snapshotIds = append(snapshotIds, device.Ebs.SnapshotId) } + } - // Retry creating tags for about 2.5 minutes - err = retry.Retry(0.2, 30, 11, func() (bool, error) { - // Tag images and snapshots - _, err := regionconn.CreateTags(&ec2.CreateTagsInput{ - Resources: resourceIds, - Tags: ec2Tags, - }) - if err == nil { - return true, nil - } - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidAMIID.NotFound" || - awsErr.Code() == "InvalidSnapshot.NotFound" { - return false, nil - } + // Retry creating tags for about 2.5 minutes + err = retry.Retry(0.2, 30, 11, func() (bool, error) { + // Tag images and snapshots + _, err := regionconn.CreateTags(&ec2.CreateTagsInput{ + Resources: resourceIds, + Tags: amiTags, + }) + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "InvalidAMIID.NotFound" || + awsErr.Code() == "InvalidSnapshot.NotFound" { + return false, nil } + } - // Override tags on snapshots - _, err = regionconn.CreateTags(&ec2.CreateTagsInput{ - Resources: snapshotIds, - Tags: snapshotTags, - }) - if err == nil { - return true, nil - } - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == "InvalidSnapshot.NotFound" { - return false, nil - } - } - return true, err + // Override tags on snapshots + _, err = regionconn.CreateTags(&ec2.CreateTagsInput{ + Resources: snapshotIds, + Tags: snapshotTags, }) - - if err != nil { - err := fmt.Errorf("Error adding tags to Resources (%#v): %s", resourceIds, err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt + if err == nil { + return true, nil + } + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "InvalidSnapshot.NotFound" { + return false, nil + } } + return true, err + }) + + if err != nil { + err := fmt.Errorf("Error adding tags to Resources (%#v): %s", resourceIds, err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt } } @@ -126,13 +127,13 @@ func (s *StepCreateTags) Cleanup(state multistep.StateBag) { } func ConvertToEC2Tags(tags map[string]string, ui packer.Ui) []*ec2.Tag { - var ec2Tags []*ec2.Tag + var amiTags []*ec2.Tag for key, value := range tags { ui.Message(fmt.Sprintf("Adding tag: \"%s\": \"%s\"", key, value)) - ec2Tags = append(ec2Tags, &ec2.Tag{ + amiTags = append(amiTags, &ec2.Tag{ Key: aws.String(key), Value: aws.String(value), }) } - return ec2Tags + return amiTags } diff --git a/builder/amazon/ebs/tags_acc_test.go b/builder/amazon/ebs/tags_acc_test.go index 3027eff7f..5d7866c89 100644 --- a/builder/amazon/ebs/tags_acc_test.go +++ b/builder/amazon/ebs/tags_acc_test.go @@ -1,6 +1,7 @@ package ebs import ( + "encoding/json" "fmt" "testing" @@ -11,6 +12,21 @@ import ( "github.com/mitchellh/packer/packer" ) +type TFBuilder struct { + Type string `json:"type"` + Region string `json:"region"` + SourceAmi string `json:"source_ami"` + InstanceType string `json:"instance_type"` + SshUsername string `json:"ssh_username"` + AmiName string `json:"ami_name"` + Tags map[string]string `json:"tags"` + SnapshotTags map[string]string `json:"snapshot_tags"` +} + +type TFConfig struct { + Builders []TFBuilder `json:"builders"` +} + func TestBuilderTagsAcc_basic(t *testing.T) { builderT.Test(t, builderT.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -26,9 +42,10 @@ func checkTags() builderT.TestCheckFunc { return fmt.Errorf("more than 1 artifact") } - tags := make(map[string]string) - tags["OS_Version"] = "Ubuntu" - tags["Release"] = "Latest" + config := TFConfig{} + json.Unmarshal([]byte(testBuilderTagsAccBasic), &config) + tags := config.Builders[0].Tags + snapshotTags := config.Builders[0].SnapshotTags // Get the actual *Artifact pointer so we can access the AMIs directly artifactRaw := artifacts[0] @@ -37,18 +54,18 @@ func checkTags() builderT.TestCheckFunc { return fmt.Errorf("unknown artifact: %#v", artifactRaw) } - // describe the image, get block devices with a snapshot + // Describe the image, get block devices with a snapshot ec2conn, _ := testEC2Conn() imageResp, err := ec2conn.DescribeImages(&ec2.DescribeImagesInput{ ImageIds: []*string{aws.String(artifact.Amis["us-east-1"])}, }) if err != nil { - return fmt.Errorf("Error retrieving details for AMI Artifcat (%#v) in Tags Test: %s", artifact, err) + return fmt.Errorf("Error retrieving details for AMI Artifact (%#v) in Tags Test: %s", artifact, err) } if len(imageResp.Images) == 0 { - return fmt.Errorf("No images found for AMI Artifcat (%#v) in Tags Test: %s", artifact, err) + return fmt.Errorf("No images found for AMI Artifact (%#v) in Tags Test: %s", artifact, err) } image := imageResp.Images[0] @@ -61,7 +78,7 @@ func checkTags() builderT.TestCheckFunc { } } - // grab matching snapshot info + // Grab matching snapshot info resp, err := ec2conn.DescribeSnapshots(&ec2.DescribeSnapshotsInput{ SnapshotIds: snapshots, }) @@ -74,12 +91,14 @@ func checkTags() builderT.TestCheckFunc { return fmt.Errorf("No Snapshots found for AMI Artifcat (%#v) in Tags Test", artifact) } - // grab the snapshots, check the tags + // Grab the snapshots, check the tags for _, s := range resp.Snapshots { expected := len(tags) for _, t := range s.Tags { for key, value := range tags { - if key == *t.Key && value == *t.Value { + if val, ok := snapshotTags[key]; ok && val == *t.Value { + expected-- + } else if key == *t.Key && value == *t.Value { expected-- } } @@ -106,7 +125,11 @@ const testBuilderTagsAccBasic = ` "ami_name": "packer-tags-testing-{{timestamp}}", "tags": { "OS_Version": "Ubuntu", - "Release": "Latest" + "Release": "Latest", + "Name": "Bleep" + }, + "snapshot_tags": { + "Name": "Foobar" } } ]