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.

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

feat: add usb controller #2280

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

burnsjared0415
Copy link
Collaborator

Description

Add the ability to add usb controller on build and modification of virtual machine.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

Running tool: /usr/local/bin/go test -timeout 30s -run ^TestAccResourceVSphereVirtualMachine_basic$ github.com/hashicorp/terraform-provider-vsphere/vsphere

ok  	github.com/hashicorp/terraform-provider-vsphere/vsphere	0.885s
$ make testacc TESTARGS='-run=TestAccXXX'

...

Release Note

Release note for CHANGELOG:

...

References

Closes #620

@burnsjared0415 burnsjared0415 added this to the v2.10.0 milestone Oct 9, 2024
@burnsjared0415 burnsjared0415 self-assigned this Oct 9, 2024
@github-actions github-actions bot added documentation Type: Documentation provider Type: Provider needs-review Status: Pull Request Needs Review labels Oct 9, 2024
@burnsjared0415 burnsjared0415 force-pushed the feat-r/vsphere_virtual_machine-add-usb-controller branch from 1420065 to 2ca7685 Compare October 9, 2024 18:42
CHANGELOG.md Outdated Show resolved Hide resolved
@burnsjared0415 burnsjared0415 force-pushed the feat-r/vsphere_virtual_machine-add-usb-controller branch from 2ca7685 to 647a6a0 Compare October 9, 2024 19:06
Add the ability to add usb controller on build and modification of virtual machine.

Signed-off-by: Jared Burns <jared.burns@broadcom.com>
@burnsjared0415 burnsjared0415 force-pushed the feat-r/vsphere_virtual_machine-add-usb-controller branch from 647a6a0 to 6feba9c Compare October 9, 2024 19:35
@burnsjared0415 burnsjared0415 added the enhancement Type: Enhancement label Oct 10, 2024
@tenthirtyam tenthirtyam marked this pull request as ready for review October 10, 2024 17:18
@tenthirtyam tenthirtyam requested a review from a team as a code owner October 10, 2024 17:18
@tenthirtyam tenthirtyam self-requested a review October 11, 2024 18:01
@@ -283,6 +283,23 @@ func resourceVSphereVirtualMachine() *schema.Resource {
Computed: true,
Description: "The power state of the virtual machine.",
},
"usb_controller": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on the vSphere configuration maximums for v7 and v8m USB 1.x, 2.x, and 3.x is supported; however, only one USB host controller of each version 1.x, 2.x, or 3.x can be added at the same time.

Optional: true,
Default: "2.0",
Description: "The version of the USB controller.",
ValidateFunc: validation.StringInSlice([]string{"2.0", "3.1"}, false),
Copy link
Collaborator

@tenthirtyam tenthirtyam Oct 11, 2024

Choose a reason for hiding this comment

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

Maybe something like

var usbControllerVersions = []string{"2.0", "3.1"}

But account for the 1.x (more than one 1.x version?)

And then:

ValidateFunc: validation.StringInSlice(usbControllerVersions, false),

Comment on lines +300 to +301
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe:

			},
		},
		ValidateFunc: validateUSBControllerVersions,

And add a function to ensure only one of each can be added. FOr example

func validateUSBControllerVersions(val interface{}, key string) (warns []string, errs []error) {
    usbControllers := val.([]interface{})
    versionCount := make(map[string]int)
    for _, version := range usbControllerVersions {
        versionCount[version] = 0
    }

    for _, controller := range usbControllers {
        controllerMap := controller.(map[string]interface{})
        version := controllerMap["usb_version"].(string)
        if _, exists := versionCount[version]; exists {
            versionCount[version]++
            if versionCount[version] > 1 {
                errs = append(errs, fmt.Errorf("%q cannot have more than one USB controller of version %s", key, version))
            }
        }
    }
    return
}

Comment on lines +614 to +622
// Get USB Controller information
var isUSBPresent bool
for _, dev := range vprops.Config.Hardware.Device {
if _, ok := dev.(*types.VirtualUSBController); ok {
isUSBPresent = true
break
}
}
_ = d.Set("usb_controller", isUSBPresent)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something that would return more like this since more than one controller can be present but only one per version.

[
    {
        "usb_controller": true,
        "usb_version": "2.0"
    },
    {
        "usb_controller": false,
        "usb_version": "3.1"
    }
]

Copy link
Collaborator

@tenthirtyam tenthirtyam Oct 11, 2024

Choose a reason for hiding this comment

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

Maybe something like this?

usbPresence := make(map[string]bool)
for _, version := range usbControllerVersions {
    usbPresence[version] = false
}

for _, dev := range vprops.Config.Hardware.Device {
    switch dev.(type) {
    case *types.VirtualUSBController:
        usbPresence["2.0"] = true
    case *types.VirtualUSBXHCIController:
        usbPresence["3.1"] = true
    }
}

var usbControllers []map[string]interface{}
for version, present := range usbPresence {
    usbControllers = append(usbControllers, map[string]interface{}{
        "usb_controller": present,
        "usb_version":    version,
    })
}

_ = d.Set("usb_controller", usbControllers)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if i can get a lab up i can test this, just not sure i want to chage it until i can test

Copy link
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

Left some thoughts on supporting only a single USB controller for each supported version based on the configuration maximums in v7 and v8.

@tenthirtyam tenthirtyam changed the title feat:r/vsphere_virtual_machine add usb controller feat: add usb controller Oct 11, 2024
@tenthirtyam tenthirtyam modified the milestones: v2.10.0, On Deck Oct 16, 2024
@tenthirtyam tenthirtyam added this to the v.2.11.0 milestone Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Type: Documentation enhancement Type: Enhancement needs-review Status: Pull Request Needs Review provider Type: Provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for USB controllers
2 participants