Add support for "High with Hyperthreading" option for `latency_sensitivity`

Guerckan commented 7 months ago

Community Guidelines

[X] I have read and agree to the HashiCorp Community Guidelines .
[X] Vote on this issue by adding a 👍 reaction to the original issue initial description to help the maintainers prioritize.
[x] Do not leave "+1" or other comments that do not add relevant information or questions.
[x] If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Description

As introduced by vSphere 8.0, the "latency sensitivity" virtual machine setting now has a new possible value named "High with Hyperthreading". See VMWare documentation supporting this statement : https://docs.vmware.com/en/VMware-vSphere/8.0/vsphere-vm-administration/GUID-F5DAC793-7B13-418F-949E-0CD80CEC5D1E.html

Current provider implementation (2.7.0 at the time of writing) of the resource vsphere_virtual_machine does not support this new value as specified here : https://registry.terraform.io/providers/hashicorp/vsphere/latest/docs/resources/virtual_machine#latency_sensitivity

Use Case(s)

This setting is particularly interesting for network appliances virtual machines and the new functionality of "high with hyperthreading" offers a way to reduce the computing power footprint of a latency sensitivity "high" setting on the host without sacrificing much of its advantages.

Potential Terraform Provider Configuration

No response

References

No response

github-actions[bot] commented 7 months ago

Hello, Guerckan! 🖐

Thank you for submitting an issue for this provider. The issue will now enter into the issue lifecycle.

If you want to contribute to this project, please review the contributing guidelines and information on submitting pull requests.

tenthirtyam commented 5 months ago

This item has an upstream dependency on [vmware/govmomi](https://github.com/vmware/govmomi/blob/bf28314d9e3287ad1674f80d8a80d2245c0159ae/vim25/types/enum.go#L4846-L4881) which would need to support values other than Low, Normal, Medium, and High. Once available and the dependency is updated, the virtualMachineLatencySensitivityAllowedValues could be updated to support the new value.

For example:

https://github.com/hashicorp/terraform-provider-vsphere/blob/413892c42a358438ad4bb258935b531d3d6e3bbe/vsphere/virtual_machine_config_structure.go#L54-L59

to...

var virtualMachineLatencySensitivityAllowedValues = []string{
    string(types.LatencySensitivitySensitivityLevelLow),
    string(types.LatencySensitivitySensitivityLevelNormal),
    string(types.LatencySensitivitySensitivityLevelMedium),
    string(types.LatencySensitivitySensitivityLevelHigh),
        string(types.LatencySensitivitySensitivityLevelHighHyperthreading), # Example
}

Please consider opening an upstream enhancement in vmware/govmomi.

Ryan Johnson Distinguished Engineer, VMware by Broadcom

Guerckan commented 5 months ago

Thank you for your help @tenthirtyam ! Upstream enhancement request has been opened as requested.

Guerckan commented 2 months ago

Hello @tenthirtyam ,

So I compared the two JSON VM objects returned by the vsphere web UI for a same VM when configured first on latency sensitivity "High" and then "High with HyperThreading" and it seems that @dougm was right over at (https://github.com/vmware/govmomi/issues/3468). In both cases the value for latency sensitivity was set to "high".

"latencySensitivity": {
        "_type": "com.vmware.vim.binding.vim.LatencySensitivity",
        "level": "high",
        "sensitivity": null
}

However the difference between "high" and "high with HT" is made elsewhere on a property called "SimultaneousThreads". In the first case it is set to 1 and on the second it is set to 2.

https://dp-downloads.broadcom.com/api-content/apis/API_VWSA_001/8.0U3/html/ReferenceGuides/vim.vm.VirtualHardware.html

"hardware": {
                "_type": "com.vmware.vim.binding.vim.vm.VirtualHardware",
                "numCPU": 2,
                "numCoresPerSocket": 1,
                "autoCoresPerSocket": true,
                "memoryMB": 2048,
                "virtualICH7MPresent": false,
                "virtualSMCPresent": false,
                "device": [
                    ...
                ],
                "motherboardLayout": "i440bxHostBridge",
                "simultaneousThreads": 2
}

So I guess this parameter should be updated by the vSphere terraform provider as well as the latency sensitivity when switching from "normal" to "high with HT". The value for simultaneousThreads is probably to adapt depending on the number of vCPUs configured on the VM.

Guerckan commented 3 weeks ago

Anybody to confirm or reject my above hypothesis ?

tenthirtyam commented 3 weeks ago

I've assigned it to myself to review.

Guerckan commented 3 weeks ago

Thank you ! Let me know if any additional info is needed.

hashicorp / terraform-provider-vsphere