diff --git a/homeassistant/components/proxmoxve/__init__.py b/homeassistant/components/proxmoxve/__init__.py index 7327c7563d0b..1f39c5d0a6d1 100644 --- a/homeassistant/components/proxmoxve/__init__.py +++ b/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 diff --git a/homeassistant/components/proxmoxve/coordinator.py b/homeassistant/components/proxmoxve/coordinator.py index 09b04b21d7a1..d3f47e2d9543 100644 --- a/homeassistant/components/proxmoxve/coordinator.py +++ b/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) diff --git a/homeassistant/components/proxmoxve/entity.py b/homeassistant/components/proxmoxve/entity.py index 815b4b06837e..ec852040e305 100644 --- a/homeassistant/components/proxmoxve/entity.py +++ b/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, ), ) diff --git a/tests/components/proxmoxve/test_init.py b/tests/components/proxmoxve/test_init.py index 1e60573015fb..4be8dabce9f2 100644 --- a/tests/components/proxmoxve/test_init.py +++ b/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,