r/virtual_machine: Re-written resource, adding several features and data sources

[vancluever]

This is a very large PR that comprises all of the work necessary as part of the vsphere_virtual_machine rewrite.

There are several new features and bug fixes in this resource. The major highlights include:

VM resource highlights

• Better separation of duties between VM creation and cloning.
• More customization options.
• Removal of the guest network and network_interface coupling, which was a large source of crashes.
• More virtual device options and better control of virtual device lifecycles across both virtual disks and network interfaces.
• Support for CPU and memory hot-plug, guest OS permitting.
• Support for both host and storage vMotion.
• Support for importing.

New data sources

New data sources have been created as part of this work to support the new resource. These include the vsphere_resource_pool, vsphere_datastore, and vsphere_virtual_machine resource, the latter fetching information from an existing VM or template to be used as data during clone operations, mainly.

---

The documentation has been completely re-written and contains a lot more detail about the new options and what working with the new resource looks like, so if you are looking for details, please see the file located here.

Also as part of this, TF has been re-vendored at version v0.11.0 to allow us to take advantage of the new diff customization features, that we use to help normalize sub-resource options, and validate configuration in ways that can't be done in the traditional validation and the refresh/plan phases.

[mjrider]

shouldn't that be ipv6_netmask ?

[vancluever]

Thanks @mjrider - corrected now!

[mjrider]

helo @vancluever it looks like a very big change, and the upgrading is ust like importing without the import, and the warnings on importing and the confusing diffs for disks, it might be usefull to add an extra document which walks thru the upgrade with examples of diffs plan wil give, and the important parts to look for highlighted.

with what i read just now, i would not upgrade or delay that as long as possible. which is counter productive because it fixes so many issues and cornercases.

[vancluever]

Hey @mjrider, sorry that this is seeming unworkable for you.

Hopefully I can address some of your concerns:

• First off, I agree that the disk diffs are confusing and am personally not happy about having to ship it as is. This is something that I want to look at after we release version 1.0.0. This is inherently due to how Terraform handles diffs for sets. I attempted to try and move the codebase for disks away from sets to lists a couple of times, however could not get it work given some of the sensitive parts of handling virtual disk lifecycles versus network devices and CDROM drives. Given that we want to ship 1.0.0 soon, and there are other things to work on with the provider as well, I've made the decision to go forward (for now) with keeping disks as a set for the time being. It's worth saying I think that this is not really a change from how the resource currently handles disks in configuration (they have always been sets), so hopefully the value in the incremental improvement here is visible.

• I think a more in-depth upgrade guide is a good idea! We will be doing some more announcements around this coming up to version 1.0.0, which I think would be a good place to fit that in. With that said, keep in mind that there is a lot of info surrounding what will change during an import (including the only attributes that should change on disk sub-resources), and the import path is the same as the migration path. This will remain in the documentation probably well into 1.0.0 - basically until the information is dated enough that it seems out of place to have in there.

Lastly, hopefully this is the last major fundamental change to how the resource operates for the most part - this means that future schema upgrades will be much less drastic and we will be able to carry settings over in state much easier. I really don't think that this kind of drastic change was avoidable, with the coupling of of a good majority of the resource to a single workflow (cloning and customization), the large amount of issues associated with the current implementation of resoruce - as you mentioned - and the high degree of "choose your own adventure", if you will, with virtual machine management in VMware as it is.

I hope that you will at least give it a shot! If anything, you can spin up a lab stack from your old configuration and just give upgrading a try. You can also version lock some of your current configurations to 0.4.2 and just use 1.0.0 going forward for new work. Thanks to how we version providers now, there's nothing forcing you to upgrade - unless you need the new features, of course - but you could even connect the two using remote state.

Hope this helps!

[vancluever]

@mjrider - just an update here, I've re-visited moving disks to lists again and have had success.

The implementation is the same as far as the user is concerned, however the much more important effect is that the diff UX is much better now, only showing fields that have changed (so that if you have just changed the size, all you will see is the size), with no confusing hashes or empty left/right side values.

I have also added defaults to the import and state migration functions, which removes those from the next diff, unless you are modifying settings, of course.

I'm working to try and target a situation where an import or state migration causes an apply that only amounts to a update of settings in Terraform state - hopefully, just a move of the keep_on_remove setting for known disks from true to false, a safeguard that will remain in place. This is a no-op update on the vSphere side.

We will see if we can get there - I will be pushing the updates soon.

[mjrider]

@vancluever that sounds great, i we are currently on 0.2.2 (iirc) and i hold back updating to 0.4 because of the amount of issues reported here. With all the restructuring, i guess i have to update to keep getting new great features. But i will be delaying that for a while for production, which we start phasing in next tuesday .

PS: for my wishlist (S)DRS rule support to keep multiple identical machines on different hosts. :)

[mjrider]

do you still need to update the documentation for the change in how disks work?

[surajsub]

Q1) is there support in this change to set the diskMode when creating a disk to "persistent" , "independent_persistent" etc ?
Q2 ) Is there an option to create new scsi controllers when new disks are added ( purely) for sap workloads that are running on vmware?

[surajsub]

@vancluever - no need to respond - I did look at your commits - looks like the diskMode capability is added to this release . Thanks

[vancluever]

Hey @tombuildsstuff and @mbfrahry, thanks for the reviews. I've answered all comments inline and applied recommendations where necessary.

Re: the goto/continue debate: as mentioned OOB this morning, I don't necessarily have a problem myself with these when used to do things like break out of loops/etc. There has probably been much said about this over the ages, so I don't necessarily want to beat that horse, but I think it's worth noting the Go spec is very specific as to where both can be used (see continue and goto in the spec). I think that these safeguards allow for a very pragmatic application of this functionality while preventing its abuse.

For the part of this PR - the one place there were gotos have been factored out as there was a much cleaner solution in general. The continues have been left in, as I think that honestly replacing them should be part of a larger refactoring effort to de-duplicate some of the device workflow code if at all possible, versus just to remove them on principle. This could be revisited, and I'm not going to be unnecessarily stubborn about it - I think there is merit to both arguments, and I also think there is merit to making sure everyone feels comfortable working with the code - but right now I want to focus on the release.

I want to say though that I value the feedback, and really appreciate the two of you bearing with me through all of this, given the scale of this PR. It's been a time, but I think the provider is in a much better place for the effort.

Thank you VERY much! 👍 🎉 🌮 ❤️

I am just running the test suite on this (we have also added another import test to test the SCSI controller count inference that passed) and then I will be merging!!!

[jason-azze]

@vancluever Awesome work! I'm trying to wrap my head around some of the changes. Would you consider some limited office hours in the hashicorp-terraform Gitter? Or, where is the best place to ask questions about the new features where everyone can benefit from the answers?

[vancluever]

@jason-azze for sure! I am around on the Gitter now - feel free to @ me or DM with your questions!