Skip to content

Commit

Permalink
Do not throw error when disk path is not known yet
Browse files Browse the repository at this point in the history
If the path to an attached disk is interpolated, then sometimes we may
end up in a situation where the diff for the VM having the disks runs
before the disk itself has been processed. In this case since the path
has no value we throw an error.

This changes the behaviour so we check if the path has been calculated
before we consider it final or not.
  • Loading branch information
Kyriakos Oikonomakos authored and koikonom committed Jan 16, 2020
1 parent c8ab7ed commit 1aa34d7
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 6 deletions.
18 changes: 12 additions & 6 deletions vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,15 +582,21 @@ func DiskDiffOperation(d *schema.ResourceDiff, c *govmomi.Client) error {
return fmt.Errorf("disk: duplicate name %s", name)
}
// If attach is set, we need to validate that there's no other duplicate paths.
curDiskPath := fmt.Sprintf("disk.%d.path", ni)
pathKnown := d.NewValueKnown(curDiskPath)
if nm["attach"].(bool) {
path := diskPathOrName(nm)
if path == "" {
return fmt.Errorf("disk.%d: path or name cannot be empty when using attach", ni)
}
if _, ok := attachments[path]; ok {
return fmt.Errorf("disk: multiple entries trying to attach external disk %s", path)
if pathKnown {
if path == "" {
return fmt.Errorf("disk.%d: path or name cannot be empty when using attach", ni)
}
if _, ok := attachments[path]; ok {
return fmt.Errorf("disk: multiple entries trying to attach external disk %s", path)
}
attachments[path] = struct{}{}
} else {
log.Printf("[DEBUG] Disk path for disk %d is not known yet.", ni)
}
attachments[path] = struct{}{}
}

if _, ok := units[nm["unit_number"].(int)]; ok {
Expand Down
85 changes: 85 additions & 0 deletions vsphere/resource_vsphere_virtual_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2985,6 +2985,26 @@ func TestAccResourceVSphereVirtualMachine_readVappChildResourcePool(t *testing.T
},
})
}

func TestAccResourceVSphereVirtualMachine_interpolatedDisk(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() {
testAccPreCheck(t)
testAccResourceVSphereVirtualMachinePreCheck(t)
},
Providers: testAccProviders,
CheckDestroy: testAccResourceVSphereVirtualMachineCheckExists(false),
Steps: []resource.TestStep{
{
Config: testAccResourceVSphereVirtualMachineTestPathInterpolation(),
Check: resource.ComposeTestCheckFunc(
testAccResourceVSphereVirtualMachineCheckExists(true),
),
},
},
})
}

func testAccResourceVSphereVirtualMachinePreCheck(t *testing.T) {
// Note that VSPHERE_USE_LINKED_CLONE is also a variable and its presence
// speeds up tests greatly, but it's not a necessary variable, so we don't
Expand Down Expand Up @@ -13726,3 +13746,68 @@ resource "vsphere_virtual_machine" "vm" {
os.Getenv("VSPHERE_VAPP_RESOURCE_POOL"),
)
}

func testAccResourceVSphereVirtualMachineTestPathInterpolation() string {
return fmt.Sprintf(`
data "vsphere_datacenter" "dc" {
name = "%s"
}
data "vsphere_datastore" "datastore" {
name = "%s"
datacenter_id = data.vsphere_datacenter.dc.id
}
data "vsphere_compute_cluster" "cluster" {
name = "%s"
datacenter_id = data.vsphere_datacenter.dc.id
}
data "vsphere_network" "n" {
name = "%s"
datacenter_id = data.vsphere_datacenter.dc.id
}
resource "null_resource" "n" {
count = 2
}
resource "vsphere_virtual_disk" "d" {
count = 2
size = 1
vmdk_path = "${null_resource.n[count.index].id}.vmdk"
datastore = data.vsphere_datastore.datastore.name
datacenter = data.vsphere_datacenter.dc.name
}
resource "vsphere_virtual_machine" "vm" {
name = "terraform-test"
resource_pool_id = data.vsphere_compute_cluster.cluster.resource_pool_id
datastore_id = data.vsphere_datastore.datastore.id
guest_id = "ubuntu64Guest"
network_interface {
network_id = data.vsphere_network.n.id
}
disk {
attach = true
label = "disk0"
path = vsphere_virtual_disk.d[0].vmdk_path
unit_number = 0
datastore_id = data.vsphere_datastore.datastore.id
}
disk {
attach = true
label = "disk1"
path = vsphere_virtual_disk.d[1].vmdk_path
unit_number = 1
datastore_id = data.vsphere_datastore.datastore.id
}
}`,
os.Getenv("VSPHERE_DATACENTER"),
os.Getenv("VSPHERE_DATASTORE"),
os.Getenv("VSPHERE_CLUSTER"),
os.Getenv("VSPHERE_NETWORK_LABEL"))
}

0 comments on commit 1aa34d7

Please sign in to comment.