Fix via_device race in proxmoxve (#177748)

homeassistant/components/proxmoxve/__init__.py

@@ -4,7 +4,7 @@ import logging
 
 from homeassistant.const import CONF_TOKEN, CONF_USERNAME, Platform
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers import entity_registry as er
+from homeassistant.helpers import device_registry as dr, entity_registry as er
 
 from .const import (
     AUTH_OTHER,
@@ -14,7 +14,7 @@ from .const import (
     CONF_REALM,
     DEFAULT_REALM,
 )
-from .coordinator import ProxmoxConfigEntry, ProxmoxCoordinator
+from .coordinator import ProxmoxConfigEntry, ProxmoxCoordinator, node_device_info
 
 PLATFORMS = [
     Platform.BINARY_SENSOR,
@@ -32,6 +32,16 @@ async def async_setup_entry(hass: HomeAssistant, entry: ProxmoxConfigEntry) -> b
     await coordinator.async_config_entry_first_refresh()
 
     entry.runtime_data = coordinator
+
+    # Register node devices before forwarding platforms so that child devices
+    # (VMs, containers, storages) can deterministically resolve their via_device.
+    device_registry = dr.async_get(hass)
+    for node_data in coordinator.data.values():
+        device_registry.async_get_or_create(
+            config_entry_id=entry.entry_id,
+            **node_device_info(coordinator, node_data),
+        )
+
     await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
 
     return True

homeassistant/components/proxmoxve/coordinator.py

@@ -10,6 +10,7 @@ from proxmoxer import AuthenticationError, ProxmoxAPI
 from proxmoxer.core import ResourceException
 import requests
 from requests.exceptions import ConnectTimeout, SSLError
+from yarl import URL
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import (
@@ -64,6 +65,32 @@ class ProxmoxNodeData:
     backups: list[dict[str, Any]] = field(default_factory=list)
 
 
+def proxmox_base_url(coordinator: ProxmoxCoordinator) -> URL:
+    """Return the base URL for the Proxmox VE."""
+    data = coordinator.config_entry.data
+    return URL.build(
+        scheme="https",
+        host=data[CONF_HOST],
+        port=data[CONF_PORT],
+    )
+
+
+def node_device_info(
+    coordinator: ProxmoxCoordinator, node_data: ProxmoxNodeData
+) -> dr.DeviceInfo:
+    """Return the device info for a Proxmox VE node device."""
+    return dr.DeviceInfo(
+        identifiers={
+            (DOMAIN, f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}")
+        },
+        name=node_data.node.get("node", str(node_data.node["id"])),
+        model="Node",
+        configuration_url=proxmox_base_url(coordinator).with_fragment(
+            f"v1:0:=node/{node_data.node['node']}"
+        ),
+    )
+
+
 class ProxmoxCoordinator(DataUpdateCoordinator[dict[str, ProxmoxNodeData]]):
     """Data Update Coordinator for Proxmox VE integration."""
 
@@ -268,6 +295,12 @@ class ProxmoxCoordinator(DataUpdateCoordinator[dict[str, ProxmoxNodeData]]):
             _LOGGER.debug("New nodes found: %s", new_nodes)
             self.known_nodes.update(new_nodes)
             new_node_data = [data[node_name] for node_name in new_nodes]
+            device_registry = dr.async_get(self.hass)
+            for node_data in new_node_data:
+                device_registry.async_get_or_create(
+                    config_entry_id=self.config_entry.entry_id,
+                    **node_device_info(self, node_data),
+                )
             for nodes_callback in self.new_nodes_callbacks:
                 nodes_callback(new_node_data)
 

homeassistant/components/proxmoxve/entity.py

@@ -2,25 +2,18 @@
 
 from typing import Any, override
 
-from yarl import URL
-
-from homeassistant.const import CONF_HOST, CONF_PORT
+from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.helpers.entity import EntityDescription
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
 from .const import DOMAIN
-from .coordinator import ProxmoxCoordinator, ProxmoxNodeData
-
-
-def _proxmox_base_url(coordinator: ProxmoxCoordinator) -> URL:
-    """Return the base URL for the Proxmox VE."""
-    data = coordinator.config_entry.data
-    return URL.build(
-        scheme="https",
-        host=data[CONF_HOST],
-        port=data[CONF_PORT],
-    )
+from .coordinator import (
+    ProxmoxCoordinator,
+    ProxmoxNodeData,
+    node_device_info,
+    proxmox_base_url,
+)
 
 
 class ProxmoxCoordinatorEntity(CoordinatorEntity[ProxmoxCoordinator]):
@@ -44,16 +37,7 @@ class ProxmoxNodeEntity(ProxmoxCoordinatorEntity):
         self.device_id = node_data.node["id"]
         self.device_name = node_data.node["node"]
         self.entity_description = entity_description
-        self._attr_device_info = DeviceInfo(
-            identifiers={
-                (DOMAIN, f"{coordinator.config_entry.entry_id}_node_{self.device_id}")
-            },
-            name=node_data.node.get("node", str(self.device_id)),
-            model="Node",
-            configuration_url=_proxmox_base_url(coordinator).with_fragment(
-                f"v1:0:=node/{node_data.node['node']}"
-            ),
-        )
+        self._attr_device_info = node_device_info(coordinator, node_data)
 
         self._attr_unique_id = (
             f"{coordinator.config_entry.entry_id}"
@@ -95,12 +79,16 @@ class ProxmoxStorageEntity(ProxmoxCoordinatorEntity):
             },
             name=f"Storage ({self.device_name})",
             model="Storage",
-            configuration_url=_proxmox_base_url(coordinator).with_fragment(
+            configuration_url=proxmox_base_url(coordinator).with_fragment(
                 f"v1:0:=storage/{self._node_name}/{storage_data['storage']}"
             ),
-            via_device=(
-                DOMAIN,
-                f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}",
+            via_device_id=dr.async_get_device_id_by_identifier(
+                coordinator.hass,
+                (
+                    DOMAIN,
+                    f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}",
+                ),
+                config_entry_id=coordinator.config_entry.entry_id,
             ),
         )
 
@@ -150,12 +138,16 @@ class ProxmoxVMEntity(ProxmoxCoordinatorEntity):
             },
             name=self.device_name,
             model="VM",
-            configuration_url=_proxmox_base_url(coordinator).with_fragment(
+            configuration_url=proxmox_base_url(coordinator).with_fragment(
                 f"v1:0:=qemu/{vm_data['vmid']}"
             ),
-            via_device=(
-                DOMAIN,
-                f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}",
+            via_device_id=dr.async_get_device_id_by_identifier(
+                coordinator.hass,
+                (
+                    DOMAIN,
+                    f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}",
+                ),
+                config_entry_id=coordinator.config_entry.entry_id,
             ),
         )
 
@@ -207,12 +199,16 @@ class ProxmoxContainerEntity(ProxmoxCoordinatorEntity):
             },
             name=self.device_name,
             model="Container",
-            configuration_url=_proxmox_base_url(coordinator).with_fragment(
+            configuration_url=proxmox_base_url(coordinator).with_fragment(
                 f"v1:0:=lxc/{container_data['vmid']}"
             ),
-            via_device=(
-                DOMAIN,
-                f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}",
+            via_device_id=dr.async_get_device_id_by_identifier(
+                coordinator.hass,
+                (
+                    DOMAIN,
+                    f"{coordinator.config_entry.entry_id}_node_{node_data.node['id']}",
+                ),
+                config_entry_id=coordinator.config_entry.entry_id,
             ),
         )
 

tests/components/proxmoxve/test_init.py

@@ -2,6 +2,7 @@
 
 from unittest.mock import MagicMock
 
+from freezegun.api import FrozenDateTimeFactory
 from proxmoxer import AuthenticationError
 from proxmoxer.core import ResourceException
 import pytest
@@ -16,6 +17,7 @@ from homeassistant.components.proxmoxve.const import (
     DOMAIN,
 )
 from homeassistant.components.proxmoxve.coordinator import (
+    DEFAULT_UPDATE_INTERVAL,
     ProxmoxNodesNotFoundError,
     ProxmoxPermissionsError,
 )
@@ -34,7 +36,11 @@ from homeassistant.helpers import device_registry as dr, entity_registry as er
 
 from . import setup_integration
 
-from tests.common import MockConfigEntry, async_load_json_array_fixture
+from tests.common import (
+    MockConfigEntry,
+    async_fire_time_changed,
+    async_load_json_array_fixture,
+)
 
 
 @pytest.mark.parametrize(
@@ -400,6 +406,108 @@ async def test_new_container_creates_entity(
     )
 
 
+@pytest.mark.parametrize(
+    "child_identifier",
+    ["vm_100", "vm_101", "container_200", "container_201", "storage_local"],
+)
+@pytest.mark.usefixtures("mock_proxmox_client")
+async def test_child_devices_link_to_node(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    device_registry: dr.DeviceRegistry,
+    child_identifier: str,
+) -> None:
+    """Test that VM/container/storage devices link to their node via via_device_id."""
+    await setup_integration(hass, mock_config_entry)
+    assert mock_config_entry.state is ConfigEntryState.LOADED
+
+    entry_id = mock_config_entry.entry_id
+    node_device = device_registry.async_get_device_by_identifier(
+        (DOMAIN, f"{entry_id}_node_node/pve1"), entry_id
+    )
+    assert node_device is not None
+
+    child_device = device_registry.async_get_device_by_identifier(
+        (DOMAIN, f"{entry_id}_{child_identifier}"), entry_id
+    )
+    assert child_device is not None
+    assert child_device.via_device_id == node_device.id
+
+
+async def test_new_node_registers_device_before_children(
+    hass: HomeAssistant,
+    freezer: FrozenDateTimeFactory,
+    mock_proxmox_client: MagicMock,
+    mock_config_entry: MockConfigEntry,
+    device_registry: dr.DeviceRegistry,
+) -> None:
+    """Test a node discovered after setup registers its device before its children.
+
+    Regression test for a race where a newly discovered node's VM/container/
+    storage entities were built before the node's own device was registered,
+    causing via_device_id resolution to raise ValueError.
+
+    Without audit permissions the node surfaces no entities of its own, so the
+    node device is only registered by the coordinator: without that explicit
+    registration its child (the configured VM, whose entities are always
+    created) cannot resolve its via_device_id.
+    """
+    mock_proxmox_client.access.permissions.get.return_value = {}
+
+    await setup_integration(hass, mock_config_entry)
+    assert mock_config_entry.state is ConfigEntryState.LOADED
+
+    # setup_integration enables disabled-by-default entities, which schedules a
+    # debounced config entry reload; let it settle so it doesn't coincide with
+    # (and mask, via a fresh setup) the refresh that discovers the new node.
+    freezer.tick(DEFAULT_UPDATE_INTERVAL)
+    async_fire_time_changed(hass)
+    await hass.async_block_till_done()
+
+    # A second node, bringing its own VM, appears on the next refresh.
+    pve2_vm = {
+        **(await async_load_json_array_fixture(hass, "nodes/qemu.json", DOMAIN))[0],
+        "vmid": 300,
+        "name": "vm-pve2",
+    }
+    pve2_node_mock = MagicMock()
+    pve2_node_mock.qemu.get.return_value = [pve2_vm]
+    pve2_node_mock.lxc.get.return_value = []
+    pve2_node_mock.storage.get.return_value = []
+    pve2_node_mock.tasks.get.return_value = []
+
+    default_node_mock = mock_proxmox_client._node_mock
+    mock_proxmox_client._nodes_mock.side_effect = lambda node: (
+        pve2_node_mock if node == "pve2" else default_node_mock
+    )
+    mock_proxmox_client.nodes.get.return_value = [
+        node
+        for node in mock_proxmox_client._all_nodes
+        if node["node"] in ("pve1", "pve2")
+    ]
+
+    freezer.tick(DEFAULT_UPDATE_INTERVAL)
+    async_fire_time_changed(hass)
+    await hass.async_block_till_done()
+
+    entry_id = mock_config_entry.entry_id
+    node_device = device_registry.async_get_device_by_identifier(
+        (DOMAIN, f"{entry_id}_node_node/pve2"), entry_id
+    )
+    assert node_device is not None
+
+    vm_device = device_registry.async_get_device_by_identifier(
+        (DOMAIN, f"{entry_id}_vm_300"), entry_id
+    )
+    assert vm_device is not None
+    assert vm_device.via_device_id == node_device.id
+
+    # The new node's VM entity was built and populated from the refresh.
+    state = hass.states.get("binary_sensor.vm_pve2_status")
+    assert state is not None
+    assert state.state == STATE_ON
+
+
 async def test_stale_devices_removed(
     hass: HomeAssistant,
     mock_proxmox_client: MagicMock,