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: support skipping image creation #186

Merged
merged 1 commit into from
May 20, 2022

Conversation

jkoelker
Copy link
Contributor

Adds the config option skip_create_image to the builders to support
skipping image creation.

Fixes: #180

@jkoelker jkoelker requested a review from a team as a code owner February 20, 2022 04:08
@hashicorp-cla
Copy link

hashicorp-cla commented Feb 20, 2022

CLA assistant check
All committers have signed the CLA.

Adds the config option `skip_create_image` to the builders to support
skipping image creation.

Fixes: hashicorp#180
Copy link

@msdolbey msdolbey left a comment

Choose a reason for hiding this comment

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

👍

@jkoelker
Copy link
Contributor Author

Please let me know if this shared step approach is desirable. For CI purposes, I really only need something for the azure-arm builder, and if desired I can narrow the scope to only skipping the image capture for that builder.

@jkoelker
Copy link
Contributor Author

I looked into the CI failures and it looks like they are existing in the repo due to commented out code. Please let me know if you want me to fix them. I took a stab at it briefly, but realized that the commented code and related issues with it may be part of a larger plan that I'm unaware of.

@nywilken
Copy link
Contributor

nywilken commented Apr 29, 2022

Thanks for opening up this PR. We'll work on getting this reviewed as soon as we can to get you feedback on the changes.

@garimakhulbe02
Copy link
Contributor

@nywilken any update? We need this merge too :)

@garimakhulbe02
Copy link
Contributor

garimakhulbe02 commented May 19, 2022

@nywilken @sylviamoss Can we prioritize this PR? We have customers block on this. Sorry for pinging again.

@JenGoldstrich JenGoldstrich self-assigned this May 20, 2022
Copy link
Contributor

@JenGoldstrich JenGoldstrich left a comment

Choose a reason for hiding this comment

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

Hi @jkoelker and all,

I've spent a bit of time today getting up to speed on this PR, testing it for myself, and thinking about the change.

Overall it's a very good, kudos @jkoelker! I pointed out a place where we could add another unit test, and I think it would be good to include some example packer template files for this field, but I do not want to block merging of this PR on either of those issues.

I will look into back filling those two things soon, but would of course appreciate a PR adding them.

Merging.

"github.com/hashicorp/packer-plugin-azure/builder/azure/common"
)

func TestSkipCreateImage(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add a test case here where we pass in false for SkipCreateImage, but this is good for now.

@JenGoldstrich JenGoldstrich merged commit a1a5275 into hashicorp:main May 20, 2022
@jkoelker jkoelker deleted the skip_image_capture branch May 20, 2022 20:23
@garimakhulbe02
Copy link
Contributor

@jkoelker @JenGoldstrich Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

skip_create_image option for cloud platforms other than Google
6 participants