search

hashicorp / packer-plugin-googlecompute

Packer plugin for Google Compute Builder
https://www.packer.io/docs/builders/googlecompute
Mozilla Public License 2.0
23 stars 51 forks source link

add machine image support #200

Closed lmayorga1980 closed 5 months ago

lmayorga1980 commented 6 months ago

@lbajolet-hashicorp do you know if someone can review this PR?

lmayorga1980 commented 6 months ago

Anyone can check this PR?

nywilken commented 5 months ago

Hi @lmayorga1980 thanks for pushing forward your request for Machine Image support. We've seen a few requests indicating that machine images would be a nice addition. We would like to see that added and if you're still interested it would be nice to support you to add the feature.

That said, we can't accept this PR in its current state. It duplicates a lot of the existing code from the other builder which is not maintainable. I think the best option would be to start with the initial HCL configuration on what a source builder would look like to better understand if this has to be a separate builder or if it is something that we could support with the existing builder using different configuration attributes.

Generally speaking when adding new builders we try to reuse common functionality between builders to simplify the logic around authentication, creation, etc... TBH I haven't looked at the requirements for machine images to understand its intersection and differences with the current builder but I think talking through that would be the next best step to understand the need.

I'm going to close this PR and we can continue discussing this work in issue #188.

lmayorga1980 commented 5 months ago

Hi @lmayorga1980 thanks for pushing forward your request for Machine Image support. We've seen a few requests indicating that machine images would be a nice addition. We would like to see that added and if you're still interested it would be nice to support you to add the feature.

That said, we can't accept this PR in its current state. It duplicates a lot of the existing code from the other builder which is not maintainable. I think the best option would be to start with the initial HCL configuration on what a source builder would look like to better understand if this has to be a separate builder or if it is something that we could support with the existing builder using different configuration attributes.

Generally speaking when adding new builders we try to reuse common functionality between builders to simplify the logic around authentication, creation, etc... TBH I haven't looked at the requirements for machine images to understand its intersection and differences with the current builder but I think talking through that would be the next best step to understand the need.

I'm going to close this PR and we can continue discussing this work in issue #188.

I saw a bit of refactoring a few weeks ago that moves some of the code moving some basic common features out of the builder itself. That said, The machine-image should be baked in inside the same plugin since needs most of the features already supported like (temporal instance) and other basic connectivity features. My testing was successful while testing machine images and also regular images.