-
Notifications
You must be signed in to change notification settings - Fork 115
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.
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
ebs: add fast-launch support with AMI copies #451
Conversation
@tivanov-qb: if possible, could I ask you to test this fix on your configuration? I'd like to have a confirmation that this does fix your problem, even if some acceptance tests point in the right direction, we can't be too careful :) |
cd474a6
to
b562aa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM minus a minor test nit
builder/ebs/builder_acc_test.go
Outdated
amiNameWithoutLT := fmt.Sprintf("packer-ebs-windows-fastlaunch-with-copies-%d", time.Now().Unix()) | ||
amiNameWithLT := fmt.Sprintf("packer-ebs-windows-fastlaunch-with-copies-and-launch-templates-%d", time.Now().Unix()) | ||
|
||
fastlaunchwithcopiesamis := []amazon_acc.AMIHelper{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fastlaunchwithcopiesamis and fastlaunchwithcopiesamisandLTs should be camel cased, the latter especially is a bit of a long name but I don't know if I have a better suggestion, maybe abbreviating fastlaunch to FL?
@lbajolet-hashicorp looking forward for this to be merged, so I can kick off a test. Thank you! |
@tivanov-qb any chance you can build the plugin and test it locally on one of your configurations? I'd prefer if you could test it before we release it since we'd have to crank out another release quickly after if that change doesn't fix your issue |
@lbajolet-hashicorp absolutely, could you please share how to point to branch/tag ? packer { |
At the moment unfortunately you won't be able to use a I would advise you to remove the If you need more guidance I suggest referring to the docs on installing plugins, especially under the |
sorry.. but I think I will need help with that.. the document says replace the binary, but how do I get the testing binary to replace it with the actual one ? |
In this case you'll have to build it; if you clone the repository and checkout the branch in which I made those changes, you'll be able to build the plugin and move it where needed. You'll need the Go toolchain for that, and possibly |
@lbajolet-hashicorp i already wasted couple of hours and this just does not work. if the plugin is called packer-plugin-amazon is not being picked by packer.. if the name is packer-plugin-amazon_v1_linux_amd64 it fails because it cannot find a checksum file.. |
|
|
I give up, i don't know how to workaround/fix this error
|
possibly related to hashicorp/packer#12414 ? |
This does look weird. Is this the binary you built? I'm surprised it cannot start it at all, can you run it from your shell at least? If you try to run Let's try to figure out the source of the problem if possible, there could be some other bug lurking around that we can maybe fix by the next release. In the meantime, I'll address @JenGoldstrich's comment and merge this PR so we can release the plugin, hopefully the problem's fixed and you can roll with it then. |
Prior to this commit, enabling fast-launch was done before copying the AMI to other regions, if the option was selected. This was problematic, as the pre-provisioned snapshots produced by enabling fast-launch weren't copied over to other regions, so only the source image was supporting fast-launch. To remedy this problem, we now enable fast-launch after the image is copies to other regions. This fixes the problem, but takes more time to complete the build, since each enabling takes time. Also, this requirement means that each region may have its own launch template to use for enabling fast-launch, so we introduce a new block in the configuration to specify which region should use which template. This option does not directly supersede the original `fast_launch_template*' options, but instead assumes that if specified, will only apply to the region the AMI is built in, and other regions will default if unspecified.
Enabling fast-launch for an AMI is an operation that takes time. Prior to the patch that mandates running the operation on the source AMI, parallelising would have had no effect, as the operation was only run once per build. After the patch though, we need to run this operation on each AMI that was copied, in which case, running in parallel may result in a significant reduction in build times. Example on the copy + fast-launch acceptance tests: - Before this patch: 2436s, i.e. 40 minutes - After this patch: 2062s, i.e. 34 minutes
b562aa3
to
ab47d8c
Compare
@lbajolet-hashicorp seems you fixed it! AMIs are now created with fast launch enabled and same goes for the copied AMIs. Great work! Thank you! |
@lbajolet-hashicorp i just noticed that AMIs shared via org arn are not actually having the fast launch enabled. so in my source ci account all looks good, but fast launch is not enabled in the destination accounts :( |
might not be a surprise for you, but aws confirmed fast launch setting is not automatically carried over to the destination accounts |
Hi @tivanov-qb, This doesn't surprise me that much since fast-launch basically pre-provisions snapshots for deploying the AMI with, which AFAIK are account-bound, so that checks out that for other accounts, I don't think this is something we can support in Packer tbh. |
Prior to this commit, enabling fast-launch was done before copying the AMI to other regions, if the option was selected.
This was problematic, as the pre-provisioned snapshots produced by enabling fast-launch weren't copied over to other regions, so only the source image was supporting fast-launch.
To remedy this problem, we now enable fast-launch after the image is copies to other regions. This fixes the problem, but takes more time to complete the build, since each enabling takes time.
Also, this requirement means that each region may have its own launch template to use for enabling fast-launch, so we introduce a new block in the configuration to specify which region should use which template.
This option does not directly supersede the original
fast_launch_template*
options, but instead assumes that if specified,will only apply to the region the AMI is built in, and other regions will default if unspecified.
Closes: #445