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: Correct present NIC device calculation #280

Merged
merged 1 commit into from
Dec 6, 2017
Merged

r/virtual_machine: Correct present NIC device calculation #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion vsphere/internal/virtualdevice/virtual_machine_network_interface_subresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ func (r *NetworkInterfaceSubresource) assignEthernetCard(l object.VirtualDeviceL

// Now that we know which units are used, we can pick one
newUnit := int32(r.Index) + pciDeviceOffset
if units[newUnit] {
if units[newUnit-pciDeviceOffset] {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add pciDeviceOffset if we are just subtracting it 2 lines later? Line 872 for reference

Copy link
Contributor Author

@vancluever vancluever Dec 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, units is a []bool of length 10 that we use to get a "map" of what PCI devices in the available network device PCI range (7-16) are currently taken to make sure we are not going to add/assign a device in an already occupied slot. This obviously has an index range of 0-9, and is not reflective of the final unit number. This was actually an error as the slice is being correctly populated here, but not being subsequently referenced correctly.

This is why only 3 devices can be added right now as per the referenced issue:

network_interface.0 7
network_interface.1 8
network_interface.2 9
network_interface.3 10 (out of range here)

Copy link
Member

Choose a reason for hiding this comment

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

Ah. Yep! Makes sense now. Thank you

return fmt.Errorf("device unit at %d is currently in use on the PCI bus", newUnit)
}

Expand Down
142 changes: 142 additions & 0 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 @@ -280,6 +280,26 @@ func TestAccResourceVSphereVirtualMachine(t *testing.T) {
},
},
},
{
"maximum number of nics",
resource.TestCase{
PreCheck: func() {
testAccPreCheck(tp)
testAccResourceVSphereVirtualMachinePreCheck(tp)
},
Providers: testAccProviders,
CheckDestroy: testAccResourceVSphereVirtualMachineCheckExists(false),
Steps: []resource.TestStep{
{
Config: testAccResourceVSphereVirtualMachineConfigMaxNIC(),
Check: resource.ComposeTestCheckFunc(
testAccResourceVSphereVirtualMachineCheckExists(true),
testAccResourceVSphereVirtualMachineCheckNICCount(10),
),
},
},
},
},
{
"upgrade cpu and ram",
resource.TestCase{
Expand Down Expand Up @@ -1808,6 +1828,28 @@ func testAccResourceVSphereVirtualMachineCheckVmdkDatastore(name, expected strin
}
}

// testAccResourceVSphereVirtualMachineCheckNICCount checks the number of NICs
// on the virtual machine.
func testAccResourceVSphereVirtualMachineCheckNICCount(expected int) resource.TestCheckFunc {
return func(s *terraform.State) error {
props, err := testGetVirtualMachineProperties(s, "vm")
if err != nil {
return err
}

var actual int
for _, dev := range props.Config.Hardware.Device {
if _, ok := dev.(types.BaseVirtualEthernetCard); ok {
actual++
}
}
if expected != actual {
return fmt.Errorf("expected %d number of NICs, got %d", expected, actual)
}
return nil
}
}

func testAccResourceVSphereVirtualMachineConfigBasic() string {
return fmt.Sprintf(`
variable "datacenter" {
Expand Down Expand Up @@ -2391,6 +2433,105 @@ resource "vsphere_virtual_machine" "vm" {
)
}

func testAccResourceVSphereVirtualMachineConfigMaxNIC() string {
return fmt.Sprintf(`
variable "datacenter" {
default = "%s"
}

variable "resource_pool" {
default = "%s"
}

variable "network_label" {
default = "%s"
}

variable "datastore" {
default = "%s"
}

data "vsphere_datacenter" "dc" {
name = "${var.datacenter}"
}

data "vsphere_datastore" "datastore" {
name = "${var.datastore}"
datacenter_id = "${data.vsphere_datacenter.dc.id}"
}

data "vsphere_resource_pool" "pool" {
name = "${var.resource_pool}"
datacenter_id = "${data.vsphere_datacenter.dc.id}"
}

data "vsphere_network" "network" {
name = "${var.network_label}"
datacenter_id = "${data.vsphere_datacenter.dc.id}"
}

resource "vsphere_virtual_machine" "vm" {
name = "terraform-test"
resource_pool_id = "${data.vsphere_resource_pool.pool.id}"
datastore_id = "${data.vsphere_datastore.datastore.id}"

num_cpus = 2
memory = 2048
guest_id = "other3xLinux64Guest"

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

network_interface {
network_id = "${data.vsphere_network.network.id}"
}

disk {
name = "terraform-test.vmdk"
size = 20
}
}
`,
os.Getenv("VSPHERE_DATACENTER"),
os.Getenv("VSPHERE_RESOURCE_POOL"),
os.Getenv("VSPHERE_NETWORK_LABEL_PXE"),
os.Getenv("VSPHERE_DATASTORE"),
)
}

func testAccResourceVSphereVirtualMachineConfigBasicAnnotation() string {
return fmt.Sprintf(`
variable "datacenter" {
Expand Down Expand Up @@ -3295,6 +3436,7 @@ resource "vsphere_virtual_machine" "vm" {
os.Getenv("VSPHERE_USE_LINKED_CLONE"),
)
}

func testAccResourceVSphereVirtualMachineConfigCloneTimeZone(zone string) string {
return fmt.Sprintf(`
variable "datacenter" {
Expand Down