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.

Sign up for GitHub

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

r/virtual_machine: Fix broken acceptance tests and CD drives #427

Merged
merged 2 commits into from
Mar 9, 2018
Merged

r/virtual_machine: Fix broken acceptance tests and CD drives #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41ccb75
r/virtual_machine: Fix broken acceptance tests and CD drives
vancluever Mar 9, 2018
2abc771
r/virtual_machine: CDROM formatting and extra info
vancluever Mar 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion vsphere/internal/virtualdevice/virtual_machine_cdrom_subresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,16 @@ func (r *CdromSubresource) Read(l object.VirtualDeviceList) error {
r.Set("datastore_id", backing.Datastore.Value)
r.Set("path", dp.Path)
default:
return fmt.Errorf("%s: Unknown CDROM type %s", r, backing)
// This is an unsupported entry, so we clear all attributes in the
// subresource (except for the device address and key, of course). In
// addition to making sure correct diffs get created for these kinds of
// devices, this ensures we don't fail on CDROM device types we don't
// support right now, such as passthrough devices. We might support these
// later.
log.Printf("%s: [DEBUG] Unknown CDROM type %T, clearing all attributes", r, backing)
Copy link

Choose a reason for hiding this comment

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

this behaviour should probably also be specified in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Added an extra blurb on this one in the docs.

r.Set("datastore_id", "")
r.Set("path", "")
r.Set("client_device", false)
}
// Save the device key and address data
ctlr, err := findControllerForDevice(l, d)
Expand Down
30 changes: 24 additions & 6 deletions vsphere/resource_vsphere_virtual_machine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ func TestAccResourceVSphereVirtualMachine_addDevices(t *testing.T) {
}

func TestAccResourceVSphereVirtualMachine_removeMiddleDevices(t *testing.T) {
var state *terraform.State
resource.Test(t, resource.TestCase{
PreCheck: func() {
testAccPreCheck(t)
Expand All @@ -200,11 +201,22 @@ func TestAccResourceVSphereVirtualMachine_removeMiddleDevices(t *testing.T) {
{
Config: testAccResourceVSphereVirtualMachineConfigMultiDevice(),
Check: resource.ComposeTestCheckFunc(
copyStatePtr(&state),
testAccResourceVSphereVirtualMachineCheckExists(true),
testAccResourceVSphereVirtualMachineCheckMultiDevice([]bool{true, true, true}, []bool{true, true, true}),
),
},
{
PreConfig: func() {
// As sometimes the OS image that we are using to test "bare metal"
// changes in how well it integrates VMware tools, we power down the
// VM for this operation. This is not necessarily checking that a
// hot-remove operation happened so it's not essential it's powered
// on.
if err := testPowerOffVM(state, "vm"); err != nil {
panic(err)
}
},
Config: testAccResourceVSphereVirtualMachineConfigRemoveMiddle(),
Check: resource.ComposeTestCheckFunc(
testAccResourceVSphereVirtualMachineCheckExists(true),
Expand Down Expand Up @@ -7860,8 +7872,10 @@ resource "vsphere_virtual_machine" "vm" {
}

disk {
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
thin_provisioned = "${data.vsphere_virtual_machine.template.disks.0.thin_provisioned}"
eagerly_scrub = "${data.vsphere_virtual_machine.template.disks.0.eagerly_scrub}"
}

vapp {
Expand Down Expand Up @@ -7975,8 +7989,10 @@ resource "vsphere_virtual_machine" "vm" {
}

disk {
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
thin_provisioned = "${data.vsphere_virtual_machine.template.disks.0.thin_provisioned}"
eagerly_scrub = "${data.vsphere_virtual_machine.template.disks.0.eagerly_scrub}"
}

vapp {
Expand Down Expand Up @@ -8091,8 +8107,10 @@ resource "vsphere_virtual_machine" "vm" {
}

disk {
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
thin_provisioned = "${data.vsphere_virtual_machine.template.disks.0.thin_provisioned}"
eagerly_scrub = "${data.vsphere_virtual_machine.template.disks.0.eagerly_scrub}"
}

vapp {
Expand Down
13 changes: 9 additions & 4 deletions website/docs/r/virtual_machine.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,15 +773,20 @@ resource "vsphere_virtual_machine" "vm" {
The options are:

* `client_device` - (Optional) Indicates whether the device should be backed by
remote client device. Conflicts with datastore_id and path.
remote client device. Conflicts with `datastore_id` and `path`.
* `datastore_id` - (Optional) The datastore ID that the ISO is located in.
Requried for using a datastore ISO. Conflicts with client_device.
Requried for using a datastore ISO. Conflicts with `client_device`.
* `path` - (Optional) The path to the ISO file. Requried for using a datastore
ISO. Conflicts with client_device.
ISO. Conflicts with `client_device`.

-> **NOTE:** Either client_device (for a remote backed CDROM) or datastore_id
~> **NOTE:** Either `client_device` (for a remote backed CDROM) or `datastore_id`
and path (for a datastore ISO backed CDROM) are required.

~> **NOTE:** Some CDROM drive types are currently unsupported by this resource,
such as pass-through devices. If these drives are present in a cloned template,
or added outside of Terraform, they will have their configurations corrected to
that of the defined device, or removed if no `cdrom` sub-resource is present.

### Virtual device computed options

Virtual device resources (`disk`, `network_interface`, and `cdrom`) all export
Expand Down