Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added tagging support for amazon/ebs AMIs #233

Merged
merged 8 commits into from
Aug 6, 2013
Merged

Added tagging support for amazon/ebs AMIs #233

merged 8 commits into from
Aug 6, 2013

Conversation

jmassara
Copy link
Contributor

@jmassara jmassara commented Aug 1, 2013

Add tags to an amazon/ebs AMI. Example configuration:

{
  "builders": [{
    "type": "amazon-ebs",
    "region": "us-west-1",
    "source_ami": "ami-12345678",
    "security_group_id": "sg-12345678",
    "iam_instance_profile": "my-ami-role",
    "instance_type": "t1.micro",
    "ssh_username": "ec2-user",
    "ssh_timeout": "5m",
    "ami_name": "packer-test",
    "tags": {
      "Name": "My AMI",
      "OS_Version": "CentOS6"
    }
  }]
}

@rgarcia
Copy link

rgarcia commented Aug 1, 2013

Nice, this is really useful.

mitchellh
mitchellh reviewed Aug 3, 2013
View reviewed changes
builder/amazon/common/run_config.go
@@ -18,6 +19,7 @@ type RunConfig struct {
SecurityGroupId string `mapstructure:"security_group_id"`
SubnetId string `mapstructure:"subnet_id"`
VpcId string `mapstructure:"vpc_id"`
Tags []ec2.Tag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really belong in RunConfig, because it isn't common to all configs necessary to run EC2 instances. This instead belongs in some general EBS config.

@mitchellh
Copy link
Contributor

Very good! Some nitpicks but I don't mind merging as is and CCing you to show you what I would change:

  1. Tags doesn't belong in RunConfig because it is not a common configuration necessary for running EC2 instances.
  2. Tags should probably be a map[string]string that is then turned into []ec2.Tag by the builder itself. I generally prefer to only use Go primitives for configuration since it makes reasoning about config easier from the source.

@jmassara jmassara closed this Aug 4, 2013
@jmassara jmassara reopened this Aug 4, 2013
@jmassara
Copy link
Contributor Author

jmassara commented Aug 4, 2013

Oops, wrong button. :( What I meant to do was say I will start working on those changes and updating the commit. Sorry about that...

@jmassara
Copy link
Contributor Author

jmassara commented Aug 4, 2013

@mitchellh New changes are in. Please let me know if that's better. Sorry about the goof up earlier!

@markpeek
Copy link
Collaborator

markpeek commented Aug 5, 2013

I'm curious what people think about the syntax for the tags being:

"tags": [
    { "key": "Name", "value": "My AMI" },
    { "key": "OS_Version", "value": "CentOS6" },
    { "key": "Release", "value": "Latest" }
]

vs. the possibly more concise:

"tags": {
    "Name": "My AMI",
    "OS_Version": "Ubuntu"
}

BTW, I already have the latter prototyped in a patch on top of your latest pull request.

@jmassara
Copy link
Contributor Author

jmassara commented Aug 5, 2013

I like that a lot better.

@ghost ghost assigned markpeek Aug 6, 2013
kwilczynski
kwilczynski reviewed Aug 6, 2013
View reviewed changes
builder/amazon/ebs/builder.go
@@ -107,7 +135,9 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
},
&common.StepProvision{},
&stepStopInstance{},
&stepCreateAMI{},
&stepCreateAMI{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since type has only one item encapsulated in it, I'd recommend making this more idiomatic:

&stepCreateAMI{b.config.ec2Tags}

@jmassara
Copy link
Contributor Author

jmassara commented Aug 6, 2013

@markpeek, updated Tags per your recommendation.

markpeek added a commit that referenced this pull request Aug 6, 2013
@markpeek
Merge pull request #233 from jmassara/tags
3d46119 
builder/amazon/ebs: Added tagging support for amazon/ebs AMIs
markpeek added a commit that referenced this pull request Aug 6, 2013
@markpeek
builder/amazon/all: refactor ami tags [GH-233]
3bcfeaf 
Refactor the EBS ami tag into a common step and add support
for instance-store ami tags.
/cc @jmassara
markpeek added a commit that referenced this pull request Aug 6, 2013
@markpeek
Update CHANGELOG for Amazon AMI tag support [GH-233]
15e8031
@markpeek markpeek merged commit e6a1fc4 into hashicorp:master Aug 6, 2013
@markpeek
Copy link
Collaborator

markpeek commented Aug 6, 2013

Thank you @jmassara for the AMI tag feature.

markpeek added a commit that referenced this pull request Apr 28, 2014
@markpeek
Merge pull request #233 from jmassara/tags
413fc1b 
builder/amazon/ebs: Added tagging support for amazon/ebs AMIs
markpeek added a commit that referenced this pull request Apr 28, 2014
@markpeek
builder/amazon/all: refactor ami tags [GH-233]
8a63651 
Refactor the EBS ami tag into a common step and add support
for instance-store ami tags.
/cc @jmassara
markpeek added a commit that referenced this pull request Apr 28, 2014
@markpeek
Update CHANGELOG for Amazon AMI tag support [GH-233]
acd0924
rickard-von-essen pushed a commit to rickard-von-essen/packer that referenced this pull request May 1, 2014
@YungSang @rickard-von-essen
Added support for Parallels Desktop for Mac [hashicorpGH-233] in the …
75e26ee 
…vagrant post-processor.

The code originate from https://github.com/YungSang/packer-parallels
rickard-von-essen added a commit to rickard-von-essen/packer that referenced this pull request May 1, 2014
@rickard-von-essen
Added support for Parallels Desktop for Mac [hashicorpGH-233] in the …
d08ee4a 
…vagrant post-processor.

Fixes rickard-von-essen/packer-parallels#3
@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@markpeek markpeek

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jmassara @rgarcia @mitchellh @markpeek @kwilczynski