Fix pinning of machine_type #2

[dimisjim]

continuation of: #149

what

Achieve pinning of COS image by fixing behavior of machine_image var and input to downstream container module

why

• Be able to pin image and not always use latest one

references

• Closes Instance being replaced even when the machine_image is pinned #114 (which needs to be reopened to be closed again with this)

[bschaatsbergen]

LGTM! Thanks a lot for your work on this pull request, and apologies for the delay in merging.

I do have a question regarding the format “To pin to one, use the following format: projects/cos-cloud/global/images/cos-stable-109-17800-147-54.” Is this different from the format we currently use when relying on the default behavior?

[dimisjim]

Thanks for the review!

Is this different from the format we currently use when relying on the default behavior?

I am not sure what you mean by this exactly.

What this PR does, is that it adds the possibility to pin the vm image to a particular one, so that terraform does not output any drift if a new one is released (they correspond to these OS versions and milestones of COS). This is done by setting the machine_image variable. If one does not set it, the previous behavior is retained. I am not aware of which format you are referring to.

[bschaatsbergen]

Thanks for the review!

Is this different from the format we currently use when relying on the default behavior?

I am not sure what you mean by this exactly.

What this PR does, is that it adds the possibility to pin the vm image to a particular one, so that terraform does not output any drift if a new one is released (they correspond to these OS versions and milestones of COS). This is done by setting the machine_image variable. If one does not set it, the previous behavior is retained. I am not aware of which format you are referring to.

Thanks! I think I misinterpreted something while reading through the previous comments. LGTM! We’ll just need to test whether this is a breaking change. Could you check on this, @cblkwell, @DanielRieske, or @d-costa? I currently don’t have access to a Google Cloud project where I can roll this out.

After reviewing the source code of terraform-google-modules/container-vm/google, it seems that this simply sets an argument into an already executed data source. Therefore, I don’t expect this to be a breaking change. I’ll go ahead and merge it.

[bschaatsbergen]

Thank you for your contribution, @dimisjim 👏🏼! Apologies for the delay in getting this merged.

[bschaatsbergen]

This has been released in v4.2.0.