hashicorp / packer-plugin-amazon

Packer plugin for Amazon AMI Builder
https://www.packer.io/docs/builders/amazon
Mozilla Public License 2.0
75 stars 112 forks source link

Allow Customizing Spot Allocation Strategy #427

Open djmetzle opened 1 year ago

djmetzle commented 1 year ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request. If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Description

EC2 allows choosing multiple allocation strategies for spot requests. It would be great to be able to specify the Allocation strategy for packer builds on Spot.

Use Case(s)

In our use case (building AMIs in CI/CD) we are seeing frequent evictions of spot instances mid-build, or even before launching the build instances. It could help to choose a strategy like capacity-optimized or price-capacity-optimized, in order to see less frequent evictions and failures due to lack of capacity.

Potential configuration

An option that exposes spot_allocation_strategy on the AWS builder would do the trick. That option could still default to lowest-price for backwards behavioral compatibility. But price-capacity-optimized is the recommended value.

Potential References

It looks like the AWS SDK exposes this configuration point as an Enum (sorry, this file is to big to permalink to): https://github.com/aws/aws-sdk-go/blob/main/service/ec2/api.go

I think it would only be a matter of adding that option to the spot fleet request here? https://github.com/hashicorp/packer-plugin-amazon/blob/e8a4526665223b6cf2830f47f0fb31baab180075/builder/common/step_run_spot_instance.go#L368-L384

From there, a configuration option would need to be exposed from input and threaded to the fleet request.


Apologies if this is too vague, as i'm not very well versed in Go.

lbajolet-hashicorp commented 1 year ago

Hi @djmetzle,

This may be a good addition indeed! This wouldn't be too hard to implement as well, so I figure this would be well suited for a first contribution.

If you're interested in this, please let me know, and we can provide guidance on that.

I'll mark the issue as such, and place it in our backlog in the meantime, we can revisit it later.

Thanks for bringing this up!

PS: for reference, the allocation strategy is not exposed as an enum but as a *string, but there's a couple of constants we can use to support them, they're exposed as the ec2.AllocationStrategy_Values() function, which we can rely on for validation.

djmetzle commented 1 year ago

Hello @lbajolet-hashicorp! Thanks for the fast feedback.

I dont think i will personally be able to contribute a solution here. I'm relatively familiar with the EC2 SDK (from other languages) which is how i searched out the link from the description. But i do not think i will have the time to learn all the input validation and testing paradigms needed to make the feature add here.

But i do think this would be an easy to add, and very useful feature. Mostly as the lowest-price strategy is not the recommended default. Even just switching to price-capacity-optimized as the default strategy could be a worthwhile change.