Skip to content

amazon/ebssurrogate: Add New Builder#4351

Merged
mwhooker merged 1 commit intohashicorp:masterfrom
jen20:ebs-surrogate-builder
Feb 22, 2017
Merged

amazon/ebssurrogate: Add New Builder#4351
mwhooker merged 1 commit intohashicorp:masterfrom
jen20:ebs-surrogate-builder

Conversation

@jen20
Copy link
Copy Markdown
Contributor

@jen20 jen20 commented Jan 3, 2017 •
edited
Loading

This commit adds a new type of builder which builds an AMI based on a snapshot of an EBS volume which is provisioned on a "surrogate" instance. This can be used to build operating system images from scratch, but unlike the chroot builder does not require running from an AWS EC2 instance.

@rickard-von-essen
Copy link
Copy Markdown
Contributor

rickard-von-essen commented Jan 3, 2017

Interesting 👍

dave2
dave2 reviewed Jan 3, 2017
View reviewed changes
Comment thread command/plugin.go
filebuilder "github.com/mitchellh/packer/builder/file"
googlecomputebuilder "github.com/mitchellh/packer/builder/googlecompute"
hypervbuilder "github.com/mitchellh/packer/builder/hyperv/iso"
hypervisobuilder "github.com/mitchellh/packer/builder/hyperv/iso"
Copy link
Copy Markdown
Contributor

@dave2 dave2 Jan 3, 2017

Choose a reason for hiding this comment

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

Was this intentional in the commit?

Copy link
Copy Markdown
Contributor Author

@jen20 jen20 Jan 3, 2017

Choose a reason for hiding this comment

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

Yes - the version in this pull request was generated.

@jen20 jen20 changed the title [WIP]: amazon/ebssurrogate: Add New Builder amazon/ebssurrogate: Add New Builder Feb 6, 2017
@jen20
Copy link
Copy Markdown
Contributor Author

jen20 commented Feb 6, 2017

This is now ready for review! cc @mwhooker, @sean-, @cbednarski, @phinze.

sean-
sean- approved these changes Feb 12, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@sean- sean- left a comment

Choose a reason for hiding this comment

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

I'm excited to see this get some use.

@sean- sean- mentioned this pull request Feb 13, 2017
Closed
@mwhooker mwhooker added this to the v0.12.3 milestone Feb 13, 2017
@mwhooker mwhooker mentioned this pull request Feb 13, 2017
Closed
@mwhooker
Copy link
Copy Markdown
Contributor

mwhooker commented Feb 13, 2017

will need further discussion around pkg/errors vs hashicorp/errwrap when we get to #4097

@mwhooker
Copy link
Copy Markdown
Contributor

mwhooker commented Feb 13, 2017 •
edited
Loading

I've got some thoughts around variable interpolation and tagging.

It seems like we'll want exclude run_tags from the config decoder https://github.com/mitchellh/packer/blob/master/builder/amazon/ebs/builder.go#L50 since we reinterpolate them in StepRunSourceInstance

I think we'll probably want the rest of the steps from e.g. builder/amazon/instance/builder.go that deal with putting the AMI in the right place https://github.com/mitchellh/packer/blob/master/builder/amazon/instance/builder.go#L262-L280

Those deal with tagging the AMI, copying it across regions, and tagging it. Does that sound right?

We'll also want to exclude the following if we add those steps

				"ami_description",
				"run_tags",
				"run_volume_tags",
				"snapshot_tags",
				"tags",

@jen20
Copy link
Copy Markdown
Contributor Author

jen20 commented Feb 13, 2017

@mwhooker re putting the AMI in the right place: it's very common that for scratch built AMIs there are region specific things built in - for example the sources list on Ubuntu. I deliberately left them out for that reason. Tagging is a reasonable thing to add though.

I'll take a look re: the other points here and follow up tomorrow.

@mwhooker
Copy link
Copy Markdown
Contributor

mwhooker commented Feb 13, 2017 •
edited
Loading

@mwhooker re putting the AMI in the right place: it's very common that for scratch built AMIs there are region specific things built in - for example the sources list on Ubuntu. I deliberately left them out for that reason.

good call

@sodabrew
Copy link
Copy Markdown
Contributor

sodabrew commented Feb 14, 2017

Per the comment at #4312 (comment) does this builder not currently support ENA, but this is where you're suggesting that ENA support should be added for building ENA images?

@mwhooker
Copy link
Copy Markdown
Contributor

mwhooker commented Feb 14, 2017

@sodabrew I haven't really thought fully if this builder needs to support ENA, but just wanted to make a note that this builder is one where we might want to add ENA support when we add it to the other builders. I think the answer to your question is yes.

@sodabrew
Copy link
Copy Markdown
Contributor

sodabrew commented Feb 14, 2017

@mwhooker I'm looking to build ENA-enabled images, and as that looks like a bleeding-edge feature request, I'm reading into which PRs here I might adapt for a hacky local build temporarily. I didn't get why 4312 was closed - were you suggesting that only this proposed new builder would be the only ENA builder?

@mwhooker
Copy link
Copy Markdown
Contributor

mwhooker commented Feb 14, 2017

@SODA let's take this to #4312

@jen20
amazon/ebssurrogate: Add New Builder
635aeb7 
This commit adds a new type of builder which builds an AMI based on a
snapshot of an EBS volume which is provisioned on a "surrogate"
instance. This can be used to build operating system images from
scratch, but unlike the `chroot` builder does not require running from
an AWS EC2 instance.
@mwhooker mwhooker merged commit c4008bc into hashicorp:master Feb 22, 2017
@sodabrew sodabrew mentioned this pull request Feb 22, 2017
Merged
@mwhooker mwhooker mentioned this pull request Feb 23, 2017
Closed
@jen20 jen20 deleted the ebs-surrogate-builder branch February 23, 2017 23:16
jen20 added a commit that referenced this pull request Feb 27, 2017
@jen20
build/amazon-ebssurrogate: Add region copy, attributes, tags steps
f440301 
As pointed out in the initial code review of #4351, some of the steps
from the standard EBS builder were (intetionally) omitted. It turns out
that these actually are useful, and the original rationale for the
omission was wrong. Consequently, this commit adds in the following
steps:

- `StepPrevalidate`
- `StepTagEBSVolumes`
- `StepDeregisterAMI`
- `StepCreateEncryptedAMICopy`
- `StepAMIRegionCopy`
- `StepModifyAMIAttribute`
- `StepCreateTags`

We also fix the interpolation filter and documentation to reflect these
additions, though the majority were already documented and just not
functional.
@jen20 jen20 mentioned this pull request Feb 27, 2017
Merged
jen20 added a commit that referenced this pull request Feb 27, 2017
@jen20
build/amazon-ebssurrogate: Add region copy, attributes, tags steps
e856339 
As pointed out in the initial code review of #4351, some of the steps
from the standard EBS builder were (intetionally) omitted. It turns out
that these actually are useful, and the original rationale for the
omission was wrong. Consequently, this commit adds in the following
steps:

- `StepPrevalidate`
- `StepTagEBSVolumes`
- `StepDeregisterAMI`
- `StepCreateEncryptedAMICopy`
- `StepAMIRegionCopy`
- `StepModifyAMIAttribute`
- `StepCreateTags`

We also fix the interpolation filter and documentation to reflect these
additions, though the majority were already documented and just not
functional.
mwhooker pushed a commit that referenced this pull request Feb 27, 2017
@jen20 @mwhooker
build/amazon-ebssurrogate: Add region copy, attributes, tags steps
a25b4e6 
As pointed out in the initial code review of #4351, some of the steps
from the standard EBS builder were (intetionally) omitted. It turns out
that these actually are useful, and the original rationale for the
omission was wrong. Consequently, this commit adds in the following
steps:

- `StepPrevalidate`
- `StepTagEBSVolumes`
- `StepDeregisterAMI`
- `StepCreateEncryptedAMICopy`
- `StepAMIRegionCopy`
- `StepModifyAMIAttribute`
- `StepCreateTags`

We also fix the interpolation filter and documentation to reflect these
additions, though the majority were already documented and just not
functional.
@ghost ghost locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@dave2 dave2 dave2 left review comments

@sean- sean- sean- approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

builder/amazon new-plugin-contribution

Projects

None yet

Milestone

v0.12.3

Development

Successfully merging this pull request may close these issues.

6 participants

@jen20 @rickard-von-essen @mwhooker @sodabrew @sean- @dave2