Skip to content

[wip] feat: extend d/host_pci_device support#2049

Draft
mristok wants to merge 1 commit intovmware:mainfrom
mristok:data-source-pci-device
Draft

[wip] feat: extend d/host_pci_device support#2049
mristok wants to merge 1 commit intovmware:mainfrom
mristok:data-source-pci-device

Conversation

@mristok
Copy link
Contributor

@mristok mristok commented Oct 30, 2023 •
edited by tenthirtyam
Loading

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 Documentation provider Provider size/l Relative Sizing: Large labels Oct 30, 2023
@tenthirtyam tenthirtyam added the enhancement Enhancement label Oct 30, 2023
@tenthirtyam tenthirtyam added this to the Backlog milestone Oct 30, 2023
@tenthirtyam tenthirtyam added the datasource Provider Datasource 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 datasource Provider Datasource label Oct 30, 2023
@tenthirtyam tenthirtyam modified the milestones: Backlog, v2.6.0 Nov 8, 2023
vasilsatanasov
vasilsatanasov approved these changes Nov 9, 2023
View reviewed changes
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
tenthirtyam
tenthirtyam reviewed Nov 30, 2023
View reviewed changes
Copy link
Contributor

@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
Contributor

tenthirtyam commented Dec 1, 2023

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 Breaking Change label Feb 12, 2024
@iBrandyJackson
Copy link
Contributor

iBrandyJackson commented Feb 12, 2024

@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 Do Not Merge label Feb 12, 2024
@tenthirtyam tenthirtyam marked this pull request as draft February 12, 2024 16:30
@github-actions github-actions bot added the needs-review Needs Review label May 7, 2025
@tenthirtyam tenthirtyam removed the needs-review Needs Review label May 7, 2025
@tenthirtyam tenthirtyam force-pushed the data-source-pci-device branch from 901e11f to b362b10 Compare May 7, 2025 15:04
@github-actions github-actions bot added documentation Documentation needs-review Needs Review labels May 7, 2025
@tenthirtyam tenthirtyam force-pushed the data-source-pci-device branch 2 times, most recently from 355e4c6 to b74fdd5 Compare May 7, 2025 15:17
@tenthirtyam tenthirtyam changed the title [wip] feat: extend d/host_pci_device support feat: extend d/host_pci_device support May 7, 2025
@tenthirtyam tenthirtyam modified the milestones: v3.0.0, Backlog May 7, 2025
@tenthirtyam tenthirtyam marked this pull request as ready for review May 7, 2025 15:22
@tenthirtyam
Copy link
Contributor

tenthirtyam commented May 7, 2025

Marking this one as ready for review after rebased post-transfer.

@tenthirtyam tenthirtyam removed breaking-change Breaking Change do-not-merge Do Not Merge merge-conflicts Merge Conflicts labels May 7, 2025
@vmware vmware deleted a comment from github-actions bot May 7, 2025
@vmware vmware deleted a comment from github-actions bot May 7, 2025
@tenthirtyam tenthirtyam force-pushed the data-source-pci-device branch from b74fdd5 to 7af9d92 Compare May 7, 2025 15:27
@tenthirtyam tenthirtyam requested a review from a team as a code owner May 19, 2025 14:34
@tenthirtyam tenthirtyam force-pushed the data-source-pci-device branch from 7af9d92 to bf70c83 Compare May 30, 2025 14:16
@tenthirtyam tenthirtyam modified the milestones: Backlog, v3.0.0 May 30, 2025
@tenthirtyam tenthirtyam added the breaking-change Breaking Change label May 30, 2025
@tenthirtyam tenthirtyam changed the title feat: extend d/host_pci_device support [wip] feat: extend d/host_pci_device support May 30, 2025
@tenthirtyam tenthirtyam marked this pull request as draft May 30, 2025 17:28
@tenthirtyam tenthirtyam added tests-limitation Testing Limitation and removed needs-review Needs Review provider Provider labels May 30, 2025
@tenthirtyam tenthirtyam force-pushed the data-source-pci-device branch from bf70c83 to 2409da5 Compare June 4, 2025 22:49
@mristok @tenthirtyam
feat: extend d/host_pci_device support
612d3e7 
Add support for the `vsphere_host_pci_device` data source to return all PCI devices which share the same details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tenthirtyam tenthirtyam Awaiting requested review from tenthirtyam

1 more reviewer

@vasilsatanasov vasilsatanasov vasilsatanasov approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@mristok mristok

Labels

breaking-change Breaking Change documentation Documentation enhancement Enhancement needs-review Needs Review provider Provider size/l Relative Sizing: Large tests-limitation Testing Limitation

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

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

4 participants

@mristok @tenthirtyam @iBrandyJackson @vasilsatanasov