search

hashicorp / packer-plugin-amazon

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

Add ability to EBS Surrogate builder to persist properties from Marketplace source image, such as product codes #455

Closed dwc0011 closed 3 months ago

dwc0011 commented 4 months ago

Add option to swap the root device to an EBS surrogate and create a new AMI in addition to being able to register an AMI. This allows billing data to be retained when creating AMI's.

Closes #440

dwc0011 commented 4 months ago

Wanted to bump this to see if we can someone has time to review this. Thanks

lorengordon commented 4 months ago

@nywilken Sorry for the ping, but just wanted to check and see if we could get a review on this pr, or an idea whether we're on an acceptable track. What I really need to know is if there's no possibility of merging this, so we can refocus on trying to find some other way to manage the workflow. Thanks!

nywilken commented 4 months ago

@nywilken Sorry for the ping, but just wanted to check and see if we could get a review on this pr, or an idea whether we're on an acceptable track. What I really need to know is if there's no possibility of merging this, so we can refocus on trying to find some other way to manage the workflow. Thanks!

@lorengordon @dwc0011 apologies I've been out sick for the last few weeks and circling back to this PR. We have plans to review and merge.

nywilken commented 4 months ago

Hi @dwc0011 @lorengordon thanks again for pushing this change forward and for you patience with the review. Speaking about this change internally we found ourselves a bit confused on the approach. To be clear, we understand why and what is happening. But we do wonder if there is a different approach on using create or register that we can take to make this clearer to not confuse users who may already be relying on ebssurrogate.

To help push this forward @lbajolet-hashicorp and I would like to setup a call so we can chat a bit more. Would you be up for a video call this week? We are both based in EST timezone.

If you are up for it you can send an email to <packer at hashicorp dot com>

lorengordon commented 4 months ago

Sure, @nywilken, we can hop on a call. I'm on Pacific Time, @dwc0011 is Eastern Time. Though, it looks like the email address was cutoff? If it's easier, I'm in the hangops slack as well, as @loren.

lbajolet-hashicorp commented 4 months ago

Hi @lorengordon,

Thanks for the quick update; the email address was cutoff since Markdown interpreted it as raw html because of the braces, I've encased it in code markup, you should be able to see it now.

dwc0011 commented 4 months ago

I am definitely open to a call, I am not available from 0800-1200 on Wednesday and I am out of office on Friday. Other than those times I am available!

Thanks, Dennis

From: Loren Gordon @.> Sent: Monday, February 26, 2024 11:40 AM To: hashicorp/packer-plugin-amazon @.> Cc: Dennis Carey @.>; Mention @.> Subject: Re: [hashicorp/packer-plugin-amazon] Ebssurrogate swap volumes (PR #455)

Sure, @nywilkenhttps://github.com/nywilken, we can hop on a call. I'm on Pacific Time, @dwc0011https://github.com/dwc0011 is Eastern Time. Though, it looks like the email address was cutoff? If it's easier, I'm in the hangops slack as well, as @loren.

— Reply to this email directly, view it on GitHubhttps://github.com/hashicorp/packer-plugin-amazon/pull/455#issuecomment-1964603782, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AM2LCVPARALPWFD4OYULKQ3YVS3IFAVCNFSM6AAAAABCRLZK7WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRUGYYDGNZYGI. You are receiving this because you were mentioned.Message ID: @.***>

lorengordon commented 4 months ago

Hi @lorengordon,

Thanks for the quick update; the email address was cutoff since Markdown interpreted it as raw html because of the braces, I've encased it in code markup, you should be able to see it now.

Roger that, email sent!

lorengordon commented 3 months ago

@nywilken @lbajolet-hashicorp Per the offline discussion, Dennis renamed the new argument as a bool, use_create_image and updated the argument description. Please let us know if you need anything else!

dwc0011 commented 3 months ago

Thank you for your time today, look forward to this being incorporated into packer!

lorengordon commented 3 months ago

Thanks again for the syncing and re-rolling this change. Overall it looks good. I left a couple of suggested changes to have the configuration attribute on the builder config and not within the ami_root_device config.

I'm running the acceptance test now and will report back if I run into any issues.

Dennis is out today, but will tackle the requested change next week. Or feel free to make the changes yourself if you want to close it sooner.

dwc0011 commented 3 months ago

@nywilken I have addressed all the moves and fixed the tests as well. I appreciate the feedback and pointing us in the right direction. I am new to go and packer in general and expected change requests, hopefully they are all complete and we are set!

Thank you again!

nywilken commented 3 months ago

We release every Tuesday so this will go out tomorrow. If I can get it released today I will trigger things earlier. Thanks again for making this change and for working with @lbajolet-hashicorp and I to better understand the needed.