Skip to content

feat: extend d/host_pci_device support #2049

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

Open
wants to merge 1 commit 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 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
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
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

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
Collaborator

@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
@tenthirtyam tenthirtyam removed the documentation Documentation label Sep 17, 2024
@github-actions github-actions bot added the stale Stale label Mar 17, 2025
@tenthirtyam tenthirtyam removed the provider Provider label Apr 16, 2025
@github-actions github-actions bot removed the stale Stale label Apr 17, 2025
@tenthirtyam tenthirtyam added the merge-conflicts Merge Conflicts label Apr 26, 2025
@tenthirtyam tenthirtyam force-pushed the main branch 2 times, most recently from 2d0d0cd to 3ebb159 Compare May 6, 2025 00:35
@tenthirtyam tenthirtyam changed the title feat: extend d/vsphere_host_pci_device support [wip] feat: extend d/vsphere_host_pci_device support May 7, 2025
@tenthirtyam tenthirtyam changed the title [wip] feat: extend d/vsphere_host_pci_device support [wip] feat: extend d/host_pci_device support May 7, 2025
@github-actions github-actions bot added provider Provider needs-review Needs Review labels 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

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
Add support for the `vsphere_host_pci_device` data source to return all PCI devices which share the same details.
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation enhancement Enhancement needs-review Needs Review provider Provider size/l Relative Sizing: Large
Projects
None yet
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