vmware_host_passthrough: Fix to be able to specify PCI id in the devi…

…ces parameter (#1365)

vmware_host_passthrough: Fix to be able to specify PCI id in the devices parameter

Depends-On: ansible/ansible-zuul-jobs#1579
SUMMARY
If ESXi has multiple PCI devices and they are the same name, and you want to enable the passthrough, you need to specify the PCI id.
But the module hasn't been able to specify the PCI id of the PCI device.
This PR is to be able to specify the id in the devices parameter the module has.
fixes: #1364
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/modules/vmware_host_passthrough.py
ADDITIONAL INFORMATION
tested in vCenter/ESXi 7.0.0

Reviewed-by: Mario Lenz <[email protected]>

changelogs/fragments/1365-vmware_host_passthrough.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - vmware_host_passthrough - Support the PCI id in the devices parameter(https://github.com/ansible-collections/community.vmware/pull/1365).

plugins/modules/vmware_host_passthrough.py

@@ -37,13 +37,14 @@
     type: str
   devices:
     description:
-      - List of PCI device name.
+      - List of PCI device name or id.
     suboptions:
-      device_name:
+      device:
         description:
           - Name of PCI device to enable passthrough.
         aliases:
           - name
+          - device_name
         type: str
     elements: dict
     required: True
@@ -84,6 +85,18 @@
       - device_name: "Dual Band Wireless AC 3165"
     state: present
 
+- name: Enable PCI device passthrough with PCI ids
+  community.vmware.vmware_host_passthrough:
+    hostname: "{{ vcenter_hostname }}"
+    username: "{{ vcenter_username }}"
+    password: "{{ vcenter_password }}"
+    validate_certs: false
+    esxi_hostname: "{{ esxi1 }}"
+    devices:
+      - device: '0000:03:00.0'
+      - device: '0000:00:02.0'
+    state: present
+
 - name: Disable PCI device passthrough against the whole ESXi in a cluster
   community.vmware.vmware_host_passthrough:
     hostname: "{{ vcenter_hostname }}"
@@ -198,21 +211,27 @@ def check_whether_devices_exist(self):
         self.existent_devices = []
         self.non_existent_devices = []
 
+        # The keys use in checking pci devices existing.
+        keys = ['device_name', 'device_id']
+
         for host_pci_device in self.hosts_passthrough_pci_devices:
             pci_devices = []
             for esxi_hostname, value in host_pci_device.items():
                 for target_device in self.devices:
-                    device_name = target_device['device_name']
-                    if device_name in [device['device_name'] for device in value['pci_devices']]:
+                    device = target_device['device']
+                    if device in [pci_device.get(key) for key in keys for pci_device in value['pci_devices']]:
                         pci_devices.append(
-                            [device for device in value['pci_devices'] if device_name == device['device_name']]
+                            [
+                                pci_device for pci_device in value['pci_devices']
+                                if device == pci_device['device_name'] or device == pci_device['device_id']
+                            ]
                         )
                     else:
-                        self.non_existent_devices.append(device_name)
+                        self.non_existent_devices.append(device)
                 self.existent_devices.append({
                     esxi_hostname: {
                         'host_obj': value['host_obj'],
-                        'checked_pci_devices': sum(pci_devices, [])
+                        'checked_pci_devices': self.de_duplication(sum(pci_devices, []))
                     }
                 })
 
@@ -235,9 +254,9 @@ def diff_passthrough_config(self):
                     'new_configs': []
                 }
                 for target_device in self.devices:
-                    device_name = target_device['device_name']
+                    device = target_device['device']
                     for checked_pci_device in value['checked_pci_devices']:
-                        if device_name == checked_pci_device['device_name']:
+                        if device == checked_pci_device['device_name'] or device == checked_pci_device['device_id']:
                             before = dict(checked_pci_device)
                             after = dict(copy.deepcopy(checked_pci_device))
 
@@ -250,6 +269,16 @@ def diff_passthrough_config(self):
                             self.diff_config['before'][esxi_hostname].append(before)
                             self.diff_config['after'][esxi_hostname].append(after)
 
+                # De-duplicate pci device data and sort.
+                self.diff_config['before'][esxi_hostname] = sorted(
+                    self.de_duplication(self.diff_config['before'][esxi_hostname]),
+                    key=lambda d: d['device_name']
+                )
+                self.diff_config['after'][esxi_hostname] = sorted(
+                    self.de_duplication(self.diff_config['after'][esxi_hostname]),
+                    key=lambda d: d['device_name']
+                )
+
     def generate_passthrough_configurations_to_be_applied(self):
         """
         Generate configs to enable or disable PCI device passthrough.
@@ -269,6 +298,14 @@ def generate_passthrough_configurations_to_be_applied(self):
                     config.id = new_config['device_id']
                     self.host_passthrough_configs[esxi_hostname]['generated_new_configs'].append(config)
 
+    def de_duplication(self, data):
+        """
+        De-duplicate dictionaries in a list.
+        """
+        return [
+            dict(s) for s in set(frozenset(d.items()) for d in data)
+        ]
+
     def execute(self):
         self.collect_pci_device_ids_for_supported_passthrough()
         self.collect_pci_devices_able_to_enable_passthrough()
@@ -306,7 +343,7 @@ def main():
         esxi_hostname=dict(type='str'),
         devices=dict(type='list', elements='dict', required=True,
                      options=dict(
-                         device_name=dict(type='str', aliases=['name'])
+                         device=dict(type='str', aliases=['name', 'device_name'])
                      )),
         state=dict(type='str', default='present', choices=['present', 'absent'])
     )

tests/integration/targets/vmware_host_passthrough/tasks/main.yml

@@ -2,16 +2,21 @@
 # Copyright: (c) 2021, sky-joker <[email protected]>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-- import_role:
+- ansible.builtin.import_role:
     name: prepare_vmware_tests
   vars:
     setup_attach_host: true
 
 - name: Include tasks pre.yml
-  include_tasks: pre.yml
+  ansible.builtin.include_tasks: pre.yml
 
-- name: Include tasks vmware_host_passthrough_tests.yml
-  include_tasks: vmware_host_passthrough.yml
+- name: Integration test for vmware_host_passthrough_tests module
   when:
     - list_of_pci_devices_that_can_be_enabled_passthrough is defined
     - list_of_pci_devices_that_can_be_enabled_passthrough | length >= 1
+  block:
+    - name: Include tasks vmware_host_passthrough_tests.yml
+      ansible.builtin.include_tasks: vmware_host_passthrough.yml
+
+    - name: Include tasks vmware_host_passthrough_pci_id.yml
+      ansible.builtin.include_tasks: vmware_host_passthrough_pci_id.yml

tests/integration/targets/vmware_host_passthrough/tasks/pre.yml

@@ -16,7 +16,7 @@
   register: pci_devices_result
 
 - name: Set the device_ids variable that includes to be enabled passthrough devices only
-  set_fact:
+  ansible.builtin.set_fact:
     device_ids: >-
       {{ device_ids | default([])
         + [item]
@@ -30,7 +30,7 @@
     - device_ids is defined
   block:
     - name: Set the list_of_pci_devices_that_can_be_enabled_passthrough variable
-      set_fact:
+      ansible.builtin.set_fact:
         list_of_pci_devices_that_can_be_enabled_passthrough: >-
           {{ list_of_pci_devices_that_can_be_enabled_passthrough | default([])
             + [item.1]

tests/integration/targets/vmware_host_passthrough/tasks/vmware_host_passthrough.yml

@@ -3,7 +3,7 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 - name: Set the target_pci_device variable to execute the integration tests
-  set_fact:
+  ansible.builtin.set_fact:
     target_pci_device: "{{ list_of_pci_devices_that_can_be_enabled_passthrough.0.deviceName }}"
 
 - name: Enable passthrough of PCI device with check_mode and diff without the cluster
@@ -20,7 +20,7 @@
   register: enable_passthrough_check_mode_diff_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_check_mode_diff_result.changed is sameas true
 
@@ -37,7 +37,7 @@
   register: enable_passthrough_check_mode_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_check_mode_result.changed is sameas true
 
@@ -53,14 +53,14 @@
   register: enable_passthrough_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_result.changed is sameas true
       - enable_passthrough_result.passthrough_configs is defined
       - enable_passthrough_result.passthrough_configs | length >= 1
 
 - name: Wait 10 sec because need a little time to enable completely passthrough of PCI device
-  pause:
+  ansible.builtin.pause:
     seconds: 10
 
 - name: Enable passthrough of PCI device without the cluster (idempotency)
@@ -75,7 +75,7 @@
   register: enable_passthrough_indempotency_result
 
 - name: Make sure if the change hasn't occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_indempotency_result.changed is sameas false
 
@@ -92,12 +92,12 @@
   register: disable_passthrough_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - disable_passthrough_result.changed is sameas true
 
 - name: Wait 10 sec because need a little time to disable completely passthrough of PCI device
-  pause:
+  ansible.builtin.pause:
     seconds: 10
 
 - name: Disable passthrough of PCI device without the cluster (idempotency)
@@ -113,7 +113,7 @@
   register: disable_passthrough_idempotency_result
 
 - name: Make sure if the change hasn't occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - disable_passthrough_idempotency_result.changed is sameas false
 
@@ -131,7 +131,7 @@
   register: enable_passthrough_cluster_check_mode_diff_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_cluster_check_mode_diff_result.changed is sameas true
 
@@ -148,7 +148,7 @@
   register: enable_passthrough_cluster_check_mode_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_cluster_check_mode_result.changed is sameas true
 
@@ -164,14 +164,14 @@
   register: enable_passthrough_cluster_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_cluster_result.changed is sameas true
       - enable_passthrough_cluster_result.passthrough_configs is defined
       - enable_passthrough_cluster_result.passthrough_configs | length >= 1
 
 - name: Wait 10 sec because need a little time to enable completely passthrough of PCI device
-  pause:
+  ansible.builtin.pause:
     seconds: 10
 
 - name: Enable passthrough of PCI device without the esxi_hostname (idempotency)
@@ -186,7 +186,7 @@
   register: enable_passthrough_cluster_indempotency_result
 
 - name: Make sure if the change hasn't occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - enable_passthrough_cluster_indempotency_result.changed is sameas false
 
@@ -203,12 +203,12 @@
   register: disable_passthrough_cluster_result
 
 - name: Make sure if the change has occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - disable_passthrough_cluster_result.changed is sameas true
 
 - name: Wait 10 sec because need a little time to disable completely passthrough of PCI device
-  pause:
+  ansible.builtin.pause:
     seconds: 10
 
 - name: Disable passthrough of PCI device without the esxi_hostname (idempotency)
@@ -224,6 +224,6 @@
   register: disable_passthrough_cluster_idempotency_result
 
 - name: Make sure if the change hasn't occurred
-  assert:
+  ansible.builtin.assert:
     that:
       - disable_passthrough_cluster_idempotency_result.changed is sameas false