inventory/proxmox: Added some cases for unsupported network interface and multi-nic and unsupported guest error (#2263)

* added some cases for unsupported network interface and multi-nic without IP addresses

* added changelog fragment

* lint fixes and added option for ansible_host setting

* added description about the new option

* lint fix too long line

* Update changelogs/fragments/2259-proxmox-multi-nic-and-unsupported.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/inventory/proxmox.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/inventory/proxmox.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/inventory/proxmox.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Added CommandDisabled

* refactored to code and added a test case where an interfaces doesnt have a mac address or is invalid to reset it to 00:00:00:00:00:00

* Update tests/unit/plugins/inventory/test_proxmox.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update tests/unit/plugins/inventory/test_proxmox.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* mac-address is set to None instead of 00:00:... when not defined

* changed None to empty string for mac-address

Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

changelogs/fragments/2259-proxmox-multi-nic-and-unsupported.yml

@@ -0,0 +1,5 @@
+---
+bugfixes:
+  - proxmox inventory plugin - support network interfaces without IP addresses, multiple network interfaces and unsupported/commanddisabled guest error (https://github.com/ansible-collections/community.general/pull/2263).
+minor_changes:
+  - proxmox inventory plugin - allow to select whether ``ansible_host`` should be set for the proxmox nodes (https://github.com/ansible-collections/community.general/pull/2263).

plugins/inventory/proxmox.py

@@ -70,6 +70,13 @@ DOCUMENTATION = '''
         description: Gather LXC/QEMU configuration facts.
         default: no
         type: bool
+      want_proxmox_nodes_ansible_host:
+        version_added: 3.0.0
+        description:
+          - Whether to set C(ansbile_host) for proxmox nodes.
+          - When set to C(true) (default), will use the first available interface. This can be different from what you expect.
+        default: true
+        type: bool
       strict:
         version_added: 2.5.0
       compose:
@@ -234,13 +241,22 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
                 )
             )['result']
 
+            if "error" in ifaces:
+                if "class" in ifaces["error"]:
+                    # This happens on Windows, even though qemu agent is running, the IP address
+                    # cannot be fetched, as it's unsupported, also a command disabled can happen.
+                    errorClass = ifaces["error"]["class"]
+                    if errorClass in ["Unsupported"]:
+                        self.display.v("Retrieving network interfaces from guest agents on windows with older qemu-guest-agents is not supported")
+                    elif errorClass in ["CommandDisabled"]:
+                        self.display.v("Retrieving network interfaces from guest agents has been disabled")
+                return result
+
             for iface in ifaces:
                 result.append({
                     'name': iface['name'],
-                    'mac-address': iface['hardware-address'],
+                    'mac-address': iface['hardware-address'] if 'hardware-address' in iface else '',
-                    'ip-addresses': [
+                    'ip-addresses': ["%s/%s" % (ip['ip-address'], ip['prefix']) for ip in iface['ip-addresses']] if 'ip-addresses' in iface else []
-                        "%s/%s" % (ip['ip-address'], ip['prefix']) for ip in iface['ip-addresses']
-                    ]
                 })
         except requests.HTTPError:
             pass
@@ -354,8 +370,9 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
                     self.inventory.add_child(nodes_group, node['node'])
 
                 # get node IP address
-                ip = self._get_node_ip(node['node'])
+                if self.get_option("want_proxmox_nodes_ansible_host"):
-                self.inventory.set_variable(node['node'], 'ansible_host', ip)
+                    ip = self._get_node_ip(node['node'])
+                    self.inventory.set_variable(node['node'], 'ansible_host', ip)
 
                 # get LXC containers for this node
                 node_lxc_group = self.to_safe('%s%s' % (self.get_option('group_prefix'), ('%s_lxc' % node['node']).lower()))

tests/unit/plugins/inventory/test_proxmox.py

@@ -90,6 +90,38 @@ def get_json(url):
                  "uptime": 1000,
                  "disk": 0,
                  "status": "running"},
+                {"name": "test-qemu-windows",
+                 "cpus": 1,
+                 "mem": 1000,
+                 "template": "",
+                 "diskread": 0,
+                 "cpu": 0.01,
+                 "maxmem": 1000,
+                 "diskwrite": 0,
+                 "netout": 1000,
+                 "pid": "1001",
+                 "netin": 1000,
+                 "maxdisk": 1000,
+                 "vmid": "102",
+                 "uptime": 1000,
+                 "disk": 0,
+                 "status": "running"},
+                {"name": "test-qemu-multi-nic",
+                 "cpus": 1,
+                 "mem": 1000,
+                 "template": "",
+                 "diskread": 0,
+                 "cpu": 0.01,
+                 "maxmem": 1000,
+                 "diskwrite": 0,
+                 "netout": 1000,
+                 "pid": "1001",
+                 "netin": 1000,
+                 "maxdisk": 1000,
+                 "vmid": "103",
+                 "uptime": 1000,
+                 "disk": 0,
+                 "status": "running"},
                 {"name": "test-qemu-template",
                  "cpus": 1,
                  "mem": 0,
@@ -212,6 +244,54 @@ def get_json(url):
             "scsihw": "virtio-scsi-pci",
             "smbios1": "uuid=ffffffff-ffff-ffff-ffff-ffffffffffff"
         }
+    elif url == "https://localhost:8006/api2/json/nodes/testnode/qemu/102/config":
+        # _get_vm_config (qemu)
+        return {
+            "numa": 0,
+            "digest": "460add1531a7068d2ae62d54f67e8fb9493dece9",
+            "ide2": "none,media=cdrom",
+            "bootdisk": "sata0",
+            "name": "test-qemu-windows",
+            "balloon": 0,
+            "cpulimit": "4",
+            "agent": "1",
+            "cores": 6,
+            "sata0": "storage:vm-102-disk-0,size=100G",
+            "memory": 10240,
+            "smbios1": "uuid=127301fc-0122-48d5-8fc5-c04fa78d8146",
+            "scsihw": "virtio-scsi-pci",
+            "sockets": 1,
+            "ostype": "win8",
+            "net0": "virtio=ff:ff:ff:ff:ff:ff,bridge=vmbr0",
+            "onboot": 1
+        }
+    elif url == "https://localhost:8006/api2/json/nodes/testnode/qemu/103/config":
+        # _get_vm_config (qemu)
+        return {
+            'scsi1': 'storage:vm-103-disk-3,size=30G',
+            'sockets': 1,
+            'memory': 8192,
+            'ostype': 'l26',
+            'scsihw': 'virtio-scsi-pci',
+            "net0": "virtio=ff:ff:ff:ff:ff:ff,bridge=vmbr0",
+            "net1": "virtio=ff:ff:ff:ff:ff:ff,bridge=vmbr1",
+            'bootdisk': 'scsi0',
+            'scsi0': 'storage:vm-103-disk-0,size=10G',
+            'name': 'test-qemu-multi-nic',
+            'cores': 4,
+            'digest': '51b7599f869b9a3f564804a0aed290f3de803292',
+            'smbios1': 'uuid=863b31c3-42ca-4a92-aed7-4111f342f70a',
+            'agent': '1,type=virtio',
+            'ide2': 'none,media=cdrom',
+            'balloon': 0,
+            'numa': 0,
+            'scsi2': 'storage:vm-103-disk-2,size=10G',
+            'serial0': 'socket',
+            'vmgenid': 'ddfb79b2-b484-4d66-88e7-6e76f2d1be77',
+            'onboot': 1,
+            'tablet': 0
+        }
+
     elif url == "https://localhost:8006/api2/json/nodes/testnode/qemu/101/agent/network-get-interfaces":
         # _get_agent_network_interfaces
         return {"result": [
@@ -281,6 +361,155 @@ def get_json(url):
                     "tx-errs": 0,
                     "tx-bytes": 0
                 }}]}
+    elif url == "https://localhost:8006/api2/json/nodes/testnode/qemu/102/agent/network-get-interfaces":
+        # _get_agent_network_interfaces
+        return {"result": {'error': {'desc': 'this feature or command is not currently supported', 'class': 'Unsupported'}}}
+    elif url == "https://localhost:8006/api2/json/nodes/testnode/qemu/103/agent/network-get-interfaces":
+        # _get_agent_network_interfaces
+        return {
+            "result": [
+                {
+                    "statistics": {
+                        "tx-errs": 0,
+                        "rx-errs": 0,
+                        "rx-dropped": 0,
+                        "tx-bytes": 48132932372,
+                        "tx-dropped": 0,
+                        "rx-bytes": 48132932372,
+                        "tx-packets": 178578980,
+                        "rx-packets": 178578980
+                    },
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "ip-addresses": [
+                        {
+                            "ip-address-type": "ipv4",
+                            "prefix": 8,
+                            "ip-address": "127.0.0.1"
+                        }
+                    ],
+                    "name": "lo"
+                },
+                {
+                    "name": "eth0",
+                    "ip-addresses": [
+                        {
+                            "ip-address-type": "ipv4",
+                            "prefix": 24,
+                            "ip-address": "172.16.0.143"
+                        }
+                    ],
+                    "statistics": {
+                        "rx-errs": 0,
+                        "tx-errs": 0,
+                        "rx-packets": 660028,
+                        "tx-packets": 304599,
+                        "tx-dropped": 0,
+                        "rx-bytes": 1846743499,
+                        "tx-bytes": 1287844926,
+                        "rx-dropped": 0
+                    },
+                    "hardware-address": "ff:ff:ff:ff:ff:ff"
+                },
+                {
+                    "name": "eth1",
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "statistics": {
+                        "rx-bytes": 235717091946,
+                        "tx-dropped": 0,
+                        "rx-dropped": 0,
+                        "tx-bytes": 123411636251,
+                        "rx-packets": 540431277,
+                        "tx-packets": 468411864,
+                        "rx-errs": 0,
+                        "tx-errs": 0
+                    },
+                    "ip-addresses": [
+                        {
+                            "ip-address": "10.0.0.133",
+                            "prefix": 24,
+                            "ip-address-type": "ipv4"
+                        }
+                    ]
+                },
+                {
+                    "name": "docker0",
+                    "ip-addresses": [
+                        {
+                            "ip-address": "172.17.0.1",
+                            "prefix": 16,
+                            "ip-address-type": "ipv4"
+                        }
+                    ],
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "statistics": {
+                        "rx-errs": 0,
+                        "tx-errs": 0,
+                        "rx-packets": 0,
+                        "tx-packets": 0,
+                        "tx-dropped": 0,
+                        "rx-bytes": 0,
+                        "rx-dropped": 0,
+                        "tx-bytes": 0
+                    }
+                },
+                {
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "name": "datapath"
+                },
+                {
+                    "name": "weave",
+                    "ip-addresses": [
+                        {
+                            "ip-address": "10.42.0.1",
+                            "ip-address-type": "ipv4",
+                            "prefix": 16
+                        }
+                    ],
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "statistics": {
+                        "rx-bytes": 127289123306,
+                        "tx-dropped": 0,
+                        "rx-dropped": 0,
+                        "tx-bytes": 43827573343,
+                        "rx-packets": 132750542,
+                        "tx-packets": 74218762,
+                        "rx-errs": 0,
+                        "tx-errs": 0
+                    }
+                },
+                {
+                    "name": "vethwe-datapath",
+                    "hardware-address": "ff:ff:ff:ff:ff:ff"
+                },
+                {
+                    "name": "vethwe-bridge",
+                    "hardware-address": "ff:ff:ff:ff:ff:ff"
+                },
+                {
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "name": "vxlan-6784"
+                },
+                {
+                    "name": "vethwepl0dfe1fe",
+                    "hardware-address": "ff:ff:ff:ff:ff:ff"
+                },
+                {
+                    "name": "vethweplf1e7715",
+                    "hardware-address": "ff:ff:ff:ff:ff:ff"
+                },
+                {
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "name": "vethwepl9d244a1"
+                },
+                {
+                    "hardware-address": "ff:ff:ff:ff:ff:ff",
+                    "name": "vethwepl2ca477b"
+                },
+                {
+                    "name": "nomacorip",
+                }
+            ]
+        }
 
 
 def get_vm_status(node, vmtype, vmid, name):
@@ -294,6 +523,8 @@ def get_option(option):
         return 'proxmox_'
     elif option == 'want_facts':
         return True
+    elif option == 'want_proxmox_nodes_ansible_host':
+        return True
     else:
         return False
 
@@ -313,6 +544,8 @@ def test_populate(inventory, mocker):
 
     # get different hosts
     host_qemu = inventory.inventory.get_host('test-qemu')
+    host_qemu_windows = inventory.inventory.get_host('test-qemu-windows')
+    host_qemu_multi_nic = inventory.inventory.get_host('test-qemu-multi-nic')
     host_qemu_template = inventory.inventory.get_host('test-qemu-template')
     host_lxc = inventory.inventory.get_host('test-lxc')
     host_node = inventory.inventory.get_host('testnode')
@@ -325,6 +558,20 @@ def test_populate(inventory, mocker):
     # check if qemu-test has eth0 interface in agent_interfaces fact
     assert 'eth0' in [d['name'] for d in host_qemu.get_vars()['proxmox_agent_interfaces']]
 
+    # check if qemu-multi-nic has multiple network interfaces
+    for iface_name in ['eth0', 'eth1', 'weave']:
+        assert iface_name in [d['name'] for d in host_qemu_multi_nic.get_vars()['proxmox_agent_interfaces']]
+
+    # check if interface with no mac-address or ip-address defaults correctly
+    assert [iface for iface in host_qemu_multi_nic.get_vars()['proxmox_agent_interfaces']
+            if iface['name'] == 'nomacorip'
+            and iface['mac-address'] == ''
+            and iface['ip-addresses'] == []
+            ]
+
+    # check to make sure qemu-windows doesn't have proxmox_agent_interfaces
+    assert "proxmox_agent_interfaces" not in host_qemu_windows.get_vars()
+
     # check if lxc-test has been discovered correctly
     group_lxc = inventory.inventory.groups['proxmox_all_lxc']
     assert group_lxc.hosts == [host_lxc]