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

amazon/ebssurrogate: Add New Builder #4351

Merged
merged 1 commit into from
Feb 22, 2017
Merged

amazon/ebssurrogate: Add New Builder #4351

merged 1 commit into from
Feb 22, 2017

Conversation

jen20
Copy link
Collaborator

@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
Collaborator

Interesting 👍

dave2
dave2 reviewed Jan 3, 2017
View reviewed changes
command/plugin.go Outdated
@@ -23,7 +24,7 @@ import (
dockerbuilder "github.com/mitchellh/packer/builder/docker"
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
Contributor

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
Collaborator Author

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
Collaborator 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
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
Contributor

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

@mwhooker
Copy link
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
Collaborator 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
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
Contributor

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
Contributor

@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
Contributor

@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
Contributor

@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

@dave2 dave2 dave2 left review comments

@sean- sean- sean- approved these changes

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