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: extend d/vsphere_host_pci_device support #2049

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mristok
Copy link
Contributor

@mristok mristok commented Oct 30, 2023

Description

Add support for the vsphere_host_pci_device data source to return all PCI devices which share the same details

Provider tests

ESXi Host with ConnectX PCI Device(s)

Changes to Outputs:
  + host_devices       = {
      + class_id    = null
      + host_id     = "host-1021"
      + id          = "fdcccc4252c0516e9d54a1ee6e851ba9f336c64502d6e80c5a684072f6e02bdc"
      + name_regex  = "ConnectX"
      + pci_devices = [
          + {
              + bus           = "136"
              + class_id      = "200"
              + device_id     = "1015"
              + function      = "0"
              + id            = "0000:88:00.0"
              + name          = "MT27710 Family [ConnectX-4 Lx]"
              + parent_bridge = ""
              + slot          = "0"
              + sub_device_id = "16"
              + sub_vendor_id = "15b3"
              + vendor_id     = "15b3"
              + vendor_name   = "Mellanox Technologies"
            },
          + {
              + bus           = "136"
              + class_id      = "200"
              + device_id     = "1015"
              + function      = "1"
              + id            = "0000:88:00.1"
              + name          = "MT27710 Family [ConnectX-4 Lx]"
              + parent_bridge = ""
              + slot          = "0"
              + sub_device_id = "16"
              + sub_vendor_id = "15b3"
              + vendor_id     = "15b3"
              + vendor_name   = "Mellanox Technologies"
            },
          + {
              + bus           = "219"
              + class_id      = "200"
              + device_id     = "1015"
              + function      = "0"
              + id            = "0000:db:00.0"
              + name          = "MT27710 Family [ConnectX-4 Lx]"
              + parent_bridge = ""
              + slot          = "0"
              + sub_device_id = "16"
              + sub_vendor_id = "15b3"
              + vendor_id     = "15b3"
              + vendor_name   = "Mellanox Technologies"
            },
          + {
              + bus           = "219"
              + class_id      = "200"
              + device_id     = "1015"
              + function      = "1"
              + id            = "0000:db:00.1"
              + name          = "MT27710 Family [ConnectX-4 Lx]"
              + parent_bridge = ""
              + slot          = "0"
              + sub_device_id = "16"
              + sub_vendor_id = "15b3"
              + vendor_id     = "15b3"
              + vendor_name   = "Mellanox Technologies"
            },
        ]
      + vendor_id   = null
    }

ESXi host without ConnectX PCI Device(s)

Changes to Outputs:
  + host_devices       = {
      + class_id    = null
      + host_id     = "host-11"
      + id          = "4982008974bbce3052719253d1d87e7d07e927d8c88ee0ada5e5fcfcc140c729"
      + name_regex  = "ConnectX"
      + pci_devices = []
      + vendor_id   = null
    }

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:

$ make testacc TESTARGS="-run=TestAccDataSourceVSphereHostPciDevice_basic -count=1"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccDataSourceVSphereHostPciDevice_basic -count=1 -timeout 360m
?       github.com/hashicorp/terraform-provider-vsphere [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/administrationroles    [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/clustercomputeresource  [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/computeresource [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/contentlibrary  [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/customattribute [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/datacenter      [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/datastore       [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/dvportgroup     [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/envbrowse       [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/folder  [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/hostsystem      [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/structure       [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/testhelper      [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/ovfdeploy       [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/provider        [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/resourcepool    [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/spbm    [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/storagepod      [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/network [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/nsx     [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/utils   [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/vappcontainer   [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/virtualmachine  [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/vsanclient      [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/vsansystem      [no test files]
?       github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/vmworkflow     [no test files]
=== RUN   TestAccDataSourceVSphereHostPciDevice_basic
--- PASS: TestAccDataSourceVSphereHostPciDevice_basic (8.45s)
PASS
ok      github.com/hashicorp/terraform-provider-vsphere/vsphere 8.475s
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/viapi   0.010s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/virtualdisk     0.079s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/virtualdevice  0.058s [no tests to run]

...

Release Note

Release note for CHANGELOG:

...

References

Closes #1572

@mristok mristok requested a review from a team as a code owner October 30, 2023 19:11
@github-actions github-actions bot added documentation Type: Documentation provider Type: Provider size/l Relative Sizing: Large labels Oct 30, 2023
@tenthirtyam tenthirtyam added the enhancement Type: Enhancement label Oct 30, 2023
@tenthirtyam tenthirtyam added this to the Backlog milestone Oct 30, 2023
@tenthirtyam tenthirtyam added the new-data-source Feature: New Data Source label Oct 30, 2023
@tenthirtyam tenthirtyam changed the title Data source pci device feat: add support for the d/vsphere_host_pci_device Oct 30, 2023
@tenthirtyam tenthirtyam changed the title feat: add support for the d/vsphere_host_pci_device feat: extend d/vsphere_host_pci_device support Oct 30, 2023
@tenthirtyam tenthirtyam removed the new-data-source Feature: New Data Source label Oct 30, 2023
@tenthirtyam tenthirtyam modified the milestones: Backlog, v2.6.0 Nov 8, 2023
Copy link
Contributor

@vasilsatanasov vasilsatanasov left a comment

Choose a reason for hiding this comment

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

LGTM!

@tenthirtyam tenthirtyam modified the milestones: v2.6.0, Backlog, v2.7.0 Nov 10, 2023
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.

This LGTM, but it would be a breaking change for those using name today and would need to be noted.

@iBrandyJackson - what's the HashiCorp protocol for such a change?

@mristok
Copy link
Contributor Author

mristok commented Dec 1, 2023

This change does return a List of PCI devices (still includes name, but extends to include additional attributes), where as currently the return is a single PCI device regardless of the number of PCI device matches on the ESXi host.

@tenthirtyam
Copy link
Collaborator

But if someone is using the data source they need to change it from name to name_regex if I recall correctly. That would be a breaking change for any existing user of the data source

@mristok
Copy link
Contributor Author

mristok commented Jan 9, 2024

This would still be considered a breaking change, but not as you have stated.
The arguments remain unchanged host_id, name_regex, class_id, vendor_id. The change is that the returned attributes id and name are now contained within the pci_devices list, which is also populated with additional attributes.

@tenthirtyam tenthirtyam modified the milestones: v2.7.0, On Deck Jan 23, 2024
@iBrandyJackson iBrandyJackson added the breaking-change Status: Breaking Change label Feb 12, 2024
@iBrandyJackson
Copy link
Member

@tenthirtyam per unified process across HC supported providers, we recommend holding all breaking changes such as this to be released in our next major release version. My apologies for my delay on response here, just now seeing this.

@tenthirtyam tenthirtyam added the do-not-merge Status: Draft, Do Not Merge label Feb 12, 2024
@tenthirtyam tenthirtyam marked this pull request as draft February 12, 2024 16:30
@tenthirtyam tenthirtyam modified the milestones: On Deck, v3.0.0 Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Status: Breaking Change do-not-merge Status: Draft, Do Not Merge documentation Type: Documentation enhancement Type: Enhancement provider Type: Provider size/l Relative Sizing: Large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the vsphere_host_pci_device data source to return all PCI devices which share the same details
4 participants