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.

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

feat: add platform flag for ec2 image import task #434

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

GastroGee
Copy link
Contributor

Setting platform for the image-import task fixes a bug where the process gets stuck as mentioned in #433

If your PR resolves any open issue(s), please indicate them like this so they will be closed when your PR is merged:

Closes #433

@GastroGee GastroGee requested a review from a team as a code owner November 15, 2023 02:03
@hashicorp-cla
Copy link

hashicorp-cla commented Nov 15, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Hi @GastroGee,

Thanks for opening this PR! I left a couple suggestions regarding the default value for the attribute, I propose leaving it empty by default, unless this is required (i.e. if boot_mode is set).

Other than that, the implementation makes sense.
Out of curiosity, have you tested this on a template? Does that fix the problem?

Thanks again for addressing this bug!

post-processor/import/post-processor.go Outdated Show resolved Hide resolved
post-processor/import/post-processor.go Outdated Show resolved Hide resolved
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Hi @GastroGee,

The code looks good to me, there's some errors at build-time though, could you fix those and re-submit?

Once that's done, we can merge this, thanks!

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution @GastroGee!

@lbajolet-hashicorp lbajolet-hashicorp merged commit a25da9b into hashicorp:main Nov 17, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Amazon import-image task stuck on "updating" when platform is not specified
3 participants