From 812477359f45e8ae00111d11e42a921a148af699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janier=20Rodr=C3=ADguez?= Date: Fri, 5 Jun 2026 16:48:42 +0200 Subject: [PATCH 1/6] fix(openstack): resolve unique_id mac via Neutron port under external DHCP OpenStack assigns the mac at the platform level, so get_mac must read it from the API. With external DHCP the server 'addresses' field comes back empty, leaving unique_id blank: cache machines never reached "valid" and the actor reported "Unmanaged host". - Fall back to the Neutron port mac (mac_address is always present, regardless of DHCP/IP allocation) when 'addresses' is empty. - Tolerate not-yet-created (404) and policy/transient errors (403/...) by degrading to '' so the state checker retries instead of forcing the userservice to ERROR. - Apply the same fix to the fixed service (also fixes an IndexError on addresses[0] under external DHCP). - Add list_ports() + PortInfo to the OpenStack client/types. - Cover both services with tests (live + fixed). --- .../services/OpenStack/openstack/client.py | 23 +++ src/uds/services/OpenStack/openstack/types.py | 24 +++ src/uds/services/OpenStack/service.py | 25 ++- src/uds/services/OpenStack/service_fixed.py | 24 ++- .../openstack/test_service_fixed_get_mac.py | 129 +++++++++++++ .../openstack/test_service_get_mac.py | 173 ++++++++++++++++++ 6 files changed, 394 insertions(+), 4 deletions(-) create mode 100644 tests/services/openstack/test_service_fixed_get_mac.py create mode 100644 tests/services/openstack/test_service_get_mac.py diff --git a/src/uds/services/OpenStack/openstack/client.py b/src/uds/services/OpenStack/openstack/client.py index 003a6c178..c08c05ae1 100644 --- a/src/uds/services/OpenStack/openstack/client.py +++ b/src/uds/services/OpenStack/openstack/client.py @@ -564,6 +564,29 @@ def list_subnets(self) -> list[openstack_types.SubnetInfo]: ) ] + def list_ports( + self, + network_id: typing.Optional[str] = None, + device_id: typing.Optional[str] = None, + ) -> list[openstack_types.PortInfo]: + query = '' + if network_id is not None: + query += f'&network_id={network_id}' + if device_id is not None: + query += f'&device_id={device_id}' + if query: + query = '?' + query[1:] # Replace leading '&' with '?' + + return [ + openstack_types.PortInfo.from_dict(p) + for p in self._get_recurring_from_endpoint( + endpoint_types=NETWORKS_ENDPOINT_TYPES, + path=f'/v2.0/ports{query}', + error_message='List ports', + key='ports', + ) + ] + @decorators.cached(prefix='sgps', timeout=consts.cache.EXTREME_CACHE_TIMEOUT, key_helper=cache_key_helper) def list_security_groups(self) -> list[openstack_types.SecurityGroupInfo]: return [ diff --git a/src/uds/services/OpenStack/openstack/types.py b/src/uds/services/OpenStack/openstack/types.py index 64de6aaab..1eb62102c 100644 --- a/src/uds/services/OpenStack/openstack/types.py +++ b/src/uds/services/OpenStack/openstack/types.py @@ -245,6 +245,30 @@ def from_str(s: str) -> 'PortStatus': return PortStatus.ERROR +@dataclasses.dataclass +class PortInfo: + id: str + name: str + status: PortStatus + device_id: str # Owner server id (the VM this port is attached to) + network_id: str + mac_address: str + fixed_ips: list[str] # List of ip addresses assigned to the port (may be empty) + + @staticmethod + def from_dict(d: dict[str, typing.Any]) -> 'PortInfo': + return PortInfo( + id=d['id'], + name=d.get('name') or d['id'], + status=PortStatus.from_str(d.get('status', PortStatus.ERROR.value)), + device_id=d.get('device_id') or '', + network_id=d.get('network_id') or '', + # Neutron always assigns a mac to the port, regardless of DHCP/IP allocation + mac_address=(d.get('mac_address') or '').upper(), + fixed_ips=[fi['ip_address'] for fi in d.get('fixed_ips', []) if fi.get('ip_address')], + ) + + @dataclasses.dataclass class ServerInfo: diff --git a/src/uds/services/OpenStack/service.py b/src/uds/services/OpenStack/service.py index d4dc54bef..c2c4c0b5a 100644 --- a/src/uds/services/OpenStack/service.py +++ b/src/uds/services/OpenStack/service.py @@ -37,6 +37,7 @@ from django.utils.translation import gettext_noop as _ from uds.core import types +from uds.core import exceptions from uds.core.services.generics.dynamic.service import DynamicService from uds.core.util import validators from uds.core.ui import gui @@ -263,8 +264,28 @@ def get_mac( *, for_unique_id: bool = False, ) -> str: - net_info = self.api.get_server_info(vmid).validated().addresses - return '' if not net_info else net_info[0].mac + # Returning '' makes the state checker retry instead of forcing the userservice + # to ERROR, so degrade to it whenever the mac cannot be resolved yet. + try: + net_info = self.api.get_server_info(vmid).validated().addresses + except exceptions.services.generics.NotFoundError: + return '' # not created yet + + if net_info and net_info[0].mac: + return net_info[0].mac + + # 'addresses' is empty when OpenStack does not manage addressing (external DHCP); + # the Neutron port still carries the mac. Swallow any API error (404/403/transient) + # here too: it must never reach the state checker. + try: + ports = self.api.list_ports(device_id=vmid) + except exceptions.services.generics.Error as e: + logger.warning('Listing Neutron ports for %s failed, mac unresolved: %s', vmid, e) + return '' + for port in ports: + if port.mac_address: + return port.mac_address + return '' def is_running( self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str diff --git a/src/uds/services/OpenStack/service_fixed.py b/src/uds/services/OpenStack/service_fixed.py index cd148b4a2..83ce6bad6 100644 --- a/src/uds/services/OpenStack/service_fixed.py +++ b/src/uds/services/OpenStack/service_fixed.py @@ -34,7 +34,7 @@ from django.utils.translation import gettext_noop as _ -from uds.core import types +from uds.core import exceptions, types from uds.core.services.generics.fixed.service import FixedService from uds.core.ui import gui @@ -196,7 +196,27 @@ def get_and_assign(self) -> str: return found_vmid def get_mac(self, vmid: str) -> str: - return self.api.get_server_info(vmid).addresses[0].mac + # Returning '' lets the caller retry instead of crashing when the mac cannot be + # resolved yet (machine missing, or external DHCP leaving 'addresses' empty). + try: + net_info = self.api.get_server_info(vmid).addresses + except exceptions.services.generics.NotFoundError: + return '' + + if net_info and net_info[0].mac: + return net_info[0].mac + + # external DHCP: 'addresses' is empty, fall back to the Neutron port mac. + # Swallow any API error (404/403/transient) so it never reaches the caller. + try: + ports = self.api.list_ports(device_id=vmid) + except exceptions.services.generics.Error as e: + logger.warning('Listing Neutron ports for %s failed, mac unresolved: %s', vmid, e) + return '' + for port in ports: + if port.mac_address: + return port.mac_address + return '' def get_ip(self, vmid: str) -> str: return self.api.get_server_info(vmid).addresses[0].ip diff --git a/tests/services/openstack/test_service_fixed_get_mac.py b/tests/services/openstack/test_service_fixed_get_mac.py new file mode 100644 index 000000000..9fc4bcb04 --- /dev/null +++ b/tests/services/openstack/test_service_fixed_get_mac.py @@ -0,0 +1,129 @@ +# -*- coding: utf-8 -*- + +# +# Copyright (c) 2024 Virtual Cable S.L. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without modification, +# are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# * Neither the name of Virtual Cable S.L. nor the names of its contributors +# may be used to endorse or promote products derived from this software +# without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +""" +Tests for OpenStackServiceFixed.get_mac unique_id resolution. + +Mirrors the live-service coverage: server 'addresses' (internal DHCP), Neutron port +fallback (external DHCP), and resilience to not-found / permission errors -> ''. +""" +import copy +import typing +from unittest import mock + +from uds.core import exceptions + +from . import fixtures + +from ...utils.test import UDSTransactionTestCase + +from uds.services.OpenStack.openstack import types as openstack_types + + +def _server_with_addresses( + addresses: list[openstack_types.ServerInfo.AddresInfo], +) -> openstack_types.ServerInfo: + server = copy.deepcopy(fixtures.SERVERS_LIST[0]) + server.status = openstack_types.ServerStatus.ACTIVE + server.addresses = addresses + return server + + +def _port(mac: str, *, device_id: str = 'sid1', ip: str = '') -> openstack_types.PortInfo: + return openstack_types.PortInfo( + id='pid1', + name='port1', + status=openstack_types.PortStatus.ACTIVE, + device_id=device_id, + network_id='net1', + mac_address=mac, + fixed_ips=[ip] if ip else [], + ) + + +class TestOpenStackFixedGetMac(UDSTransactionTestCase): + def _service_with_api(self) -> tuple[typing.Any, mock.MagicMock]: + with fixtures.patched_provider() as prov: + service = fixtures.create_fixed_service(prov) + api = mock.MagicMock() + service._api = api # the api property returns the cached client without hitting the provider + return service, api + + def test_get_mac_from_server_addresses(self) -> None: + # Internal DHCP: server addresses carry the mac + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses( + [openstack_types.ServerInfo.AddresInfo(version=4, ip='10.0.0.5', mac='FA:16:3E:00:00:01', type='fixed')] + ) + + mac = service.get_mac('sid1') + + self.assertEqual(mac, 'FA:16:3E:00:00:01') + api.list_ports.assert_not_called() + + def test_get_mac_falls_back_to_neutron_port(self) -> None: + # External DHCP: server addresses empty -> use the Neutron port mac + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.return_value = [_port('FA:16:3E:AB:CD:EF', device_id='sid1')] + + mac = service.get_mac('sid1') + + self.assertEqual(mac, 'FA:16:3E:AB:CD:EF') + api.list_ports.assert_called_once_with(device_id='sid1') + + def test_get_mac_empty_when_server_not_found(self) -> None: + # Machine missing: no IndexError, no leak -> '' so the caller can retry + service, api = self._service_with_api() + api.get_server_info.side_effect = exceptions.services.generics.NotFoundError('Not found') + + mac = service.get_mac('sid1') + + self.assertEqual(mac, '') + api.list_ports.assert_not_called() + + def test_get_mac_empty_when_no_addresses_and_no_ports(self) -> None: + # External DHCP but the port is not there yet -> '' instead of IndexError + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.return_value = [] + + mac = service.get_mac('sid1') + + self.assertEqual(mac, '') + + def test_get_mac_empty_when_port_lookup_permission_denied(self) -> None: + # Neutron policy forbids listing ports (403) -> '' instead of crashing + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.side_effect = exceptions.services.generics.Error('Forbidden') + + mac = service.get_mac('sid1') + + self.assertEqual(mac, '') diff --git a/tests/services/openstack/test_service_get_mac.py b/tests/services/openstack/test_service_get_mac.py new file mode 100644 index 000000000..92bfaeba5 --- /dev/null +++ b/tests/services/openstack/test_service_get_mac.py @@ -0,0 +1,173 @@ +# -*- coding: utf-8 -*- + +# +# Copyright (c) 2024 Virtual Cable S.L. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without modification, +# are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# * Neither the name of Virtual Cable S.L. nor the names of its contributors +# may be used to endorse or promote products derived from this software +# without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +""" +Tests for OpenStackLiveService.get_mac unique_id resolution. + +Covers the two address sources: + * server 'addresses' (OpenStack-managed addressing / internal DHCP) + * Neutron port fallback (external DHCP, where 'addresses' is empty) + +and the resilience to a not-yet-created server (NotFoundError -> ''). +""" +import copy +import typing +from unittest import mock + +from uds.core import exceptions + +from . import fixtures + +from ...utils.test import UDSTransactionTestCase + +from uds.services.OpenStack.openstack import types as openstack_types + + +def _server_with_addresses( + addresses: list[openstack_types.ServerInfo.AddresInfo], +) -> openstack_types.ServerInfo: + server = copy.deepcopy(fixtures.SERVERS_LIST[0]) + server.status = openstack_types.ServerStatus.ACTIVE # not "lost", so validated() passes + server.addresses = addresses + return server + + +def _port(mac: str, *, device_id: str = 'sid1', ip: str = '') -> openstack_types.PortInfo: + return openstack_types.PortInfo( + id='pid1', + name='port1', + status=openstack_types.PortStatus.ACTIVE, + device_id=device_id, + network_id='net1', + mac_address=mac, + fixed_ips=[ip] if ip else [], + ) + + +class TestOpenStackGetMac(UDSTransactionTestCase): + def _service_with_api(self) -> tuple[typing.Any, mock.MagicMock]: + with fixtures.patched_provider() as prov: + service = fixtures.create_live_service(prov) + api = mock.MagicMock() + service.cached_api = api # property returns this without hitting the provider + return service, api + + def test_get_mac_from_server_addresses(self) -> None: + # Internal DHCP: server addresses carry the mac + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses( + [ + openstack_types.ServerInfo.AddresInfo( + version=4, ip='10.0.0.5', mac='FA:16:3E:00:00:01', type='fixed' + ) + ] + ) + + mac = service.get_mac(None, 'sid1', for_unique_id=True) + + self.assertEqual(mac, 'FA:16:3E:00:00:01') + api.list_ports.assert_not_called() # no fallback needed + + def test_get_mac_falls_back_to_neutron_port(self) -> None: + # External DHCP: server addresses empty -> use the Neutron port mac + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.return_value = [_port('FA:16:3E:AB:CD:EF', device_id='sid1')] + + mac = service.get_mac(None, 'sid1', for_unique_id=True) + + self.assertEqual(mac, 'FA:16:3E:AB:CD:EF') + api.list_ports.assert_called_once_with(device_id='sid1') + + def test_get_mac_empty_when_server_not_found(self) -> None: + # Machine still being prepared: server not queryable yet -> '' (retry, no ERROR) + service, api = self._service_with_api() + api.get_server_info.side_effect = exceptions.services.generics.NotFoundError('Not found') + + mac = service.get_mac(None, 'sid1', for_unique_id=True) + + self.assertEqual(mac, '') + api.list_ports.assert_not_called() + + def test_get_mac_empty_when_no_addresses_and_no_ports(self) -> None: + # No addresses and no ports yet -> '' so the state checker retries + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.return_value = [] + + mac = service.get_mac(None, 'sid1', for_unique_id=True) + + self.assertEqual(mac, '') + + def test_get_mac_empty_when_port_lookup_not_found(self) -> None: + # addresses empty and Neutron returns 404 -> '' (no exception leaks to caller) + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.side_effect = exceptions.services.generics.NotFoundError('Not found') + + mac = service.get_mac(None, 'sid1', for_unique_id=True) + + self.assertEqual(mac, '') + + def test_get_mac_empty_when_port_lookup_permission_denied(self) -> None: + # Neutron policy forbids listing ports (403) -> '' instead of forcing ERROR. + # A bare Error (the base class) covers 403/RBAC/availability/transient errors. + service, api = self._service_with_api() + api.get_server_info.return_value = _server_with_addresses([]) + api.list_ports.side_effect = exceptions.services.generics.Error('Forbidden') + + mac = service.get_mac(None, 'sid1', for_unique_id=True) + + self.assertEqual(mac, '') + + +class TestPortInfo(UDSTransactionTestCase): + def test_from_dict_uppercases_mac_and_extracts_ips(self) -> None: + port = openstack_types.PortInfo.from_dict( + { + 'id': 'pid1', + 'name': 'p1', + 'status': 'ACTIVE', + 'device_id': 'sid1', + 'network_id': 'net1', + 'mac_address': 'fa:16:3e:11:22:33', + 'fixed_ips': [{'ip_address': '10.0.0.9', 'subnet_id': 'sub1'}, {'subnet_id': 'sub2'}], + } + ) + self.assertEqual(port.mac_address, 'FA:16:3E:11:22:33') + self.assertEqual(port.fixed_ips, ['10.0.0.9']) + self.assertEqual(port.status, openstack_types.PortStatus.ACTIVE) + + def test_from_dict_tolerates_missing_fields(self) -> None: + port = openstack_types.PortInfo.from_dict({'id': 'pid9'}) + self.assertEqual(port.id, 'pid9') + self.assertEqual(port.name, 'pid9') # falls back to id + self.assertEqual(port.mac_address, '') + self.assertEqual(port.fixed_ips, []) + self.assertEqual(port.device_id, '') From 3198d783572886c87e220891cc42ba14c2ce28b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janier=20Rodr=C3=ADguez?= Date: Fri, 5 Jun 2026 16:55:05 +0200 Subject: [PATCH 2/6] style(openstack): use PEP 604 union syntax instead of typing.Optional/Union --- .../services/OpenStack/openstack/client.py | 40 +++++++++---------- src/uds/services/OpenStack/service.py | 20 +++++----- src/uds/services/OpenStack/service_fixed.py | 6 +-- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/uds/services/OpenStack/openstack/client.py b/src/uds/services/OpenStack/openstack/client.py index c08c05ae1..ddf6bea30 100644 --- a/src/uds/services/OpenStack/openstack/client.py +++ b/src/uds/services/OpenStack/openstack/client.py @@ -106,20 +106,20 @@ def cache_key_helper(obj: 'OpenStackClient') -> str: class OpenStackClient: # pylint: disable=too-many-public-methods _authenticated: bool # If we change the project id, we need to reauthenticate - _authenticated_projectid: typing.Optional[str] + _authenticated_projectid: str | None _identity_endpoint: str - _tokenid: typing.Optional[str] - _catalog: typing.Optional[list[dict[str, typing.Any]]] + _tokenid: str | None + _catalog: list[dict[str, typing.Any]] | None _is_legacy: bool - _access: typing.Optional[str] + _access: str | None _domain: str _username: str _password: str _auth_method: openstack_types.AuthMethod - _userid: typing.Optional[str] - _projectid: typing.Optional[str] - _project_name: typing.Optional[str] - _region: typing.Optional[str] + _userid: str | None + _projectid: str | None + _project_name: str | None + _region: str | None _timeout: int _session: 'requests.Session' @@ -135,10 +135,10 @@ def __init__( password: str, port: int = -1, # Only used for legacy use_ssl: bool = False, # Only used for legacy - projectid: typing.Optional[str] = None, - region: typing.Optional[str] = None, - access: typing.Optional[openstack_types.AccessType] = None, - proxies: typing.Optional[dict[str, str]] = None, + projectid: str | None = None, + region: str | None = None, + access: openstack_types.AccessType | None = None, + proxies: dict[str, str] | None = None, timeout: int = 10, verify_ssl: bool = True, auth_method: openstack_types.AuthMethod = openstack_types.AuthMethod.PASSWORD, @@ -289,7 +289,7 @@ def _get_recurring_from_endpoint( path: str, error_message: str, key: str, - params: typing.Optional[dict[str, str]] = None, + params: dict[str, str] | None = None, ) -> collections.abc.Iterable[typing.Any]: cache_key = ''.join(endpoint_types) found_endpoints = self._get_endpoints_iterable(cache_key, *endpoint_types) @@ -476,7 +476,7 @@ def list_regions(self) -> list[openstack_types.RegionInfo]: def list_servers( self, detail: bool = False, - params: typing.Optional[dict[str, str]] = None, + params: dict[str, str] | None = None, **kwargs: typing.Any, ) -> list[openstack_types.ServerInfo]: return [ @@ -566,8 +566,8 @@ def list_subnets(self) -> list[openstack_types.SubnetInfo]: def list_ports( self, - network_id: typing.Optional[str] = None, - device_id: typing.Optional[str] = None, + network_id: str | None = None, + device_id: str | None = None, ) -> list[openstack_types.PortInfo]: query = '' if network_id is not None: @@ -638,7 +638,7 @@ def get_snapshot_info(self, snapshot_id: str) -> openstack_types.SnapshotInfo: return openstack_types.SnapshotInfo.from_dict(r.json()['snapshot']) def create_snapshot( - self, volume_id: str, name: str, description: typing.Optional[str] = None + self, volume_id: str, name: str, description: str | None = None ) -> openstack_types.SnapshotInfo: # First, get volume info to ensure it exists self.get_volume_info(volume_id, force=True) @@ -856,8 +856,8 @@ def _get_recurring_url_json( session: 'requests.Session', headers: dict[str, str], key: str, - params: typing.Optional[collections.abc.Mapping[str, str]] = None, - error_message: typing.Optional[str] = None, + params: collections.abc.Mapping[str, str] | None = None, + error_message: str | None = None, timeout: int = 10, ) -> collections.abc.Iterable[typing.Any]: counter = 0 @@ -885,7 +885,7 @@ def _get_recurring_url_json( @staticmethod def _ensure_valid_response( - response: 'requests.Response', error_message: typing.Optional[str] = None, expects_json: bool = True + response: 'requests.Response', error_message: str | None = None, expects_json: bool = True ) -> None: if response.ok is False: if response.status_code == 404: diff --git a/src/uds/services/OpenStack/service.py b/src/uds/services/OpenStack/service.py index c2c4c0b5a..5f0d70fea 100644 --- a/src/uds/services/OpenStack/service.py +++ b/src/uds/services/OpenStack/service.py @@ -55,7 +55,7 @@ from .provider import OpenStackProvider from .provider_legacy import OpenStackProviderLegacy - AnyOpenStackProvider: typing.TypeAlias = typing.Union[OpenStackProvider, OpenStackProviderLegacy] + AnyOpenStackProvider: typing.TypeAlias = OpenStackProvider | OpenStackProviderLegacy from uds.core.services.generics.dynamic.userservice import DynamicUserService from uds.core.services.generics.dynamic.publication import DynamicPublication @@ -187,7 +187,7 @@ class OpenStackLiveService(DynamicService): prov_uuid = gui.HiddenField() - cached_api: typing.Optional['client.OpenStackClient'] = None + cached_api: 'client.OpenStackClient | None' = None # Note: currently, Openstack does not provides a way of specifying how to stop the server # At least, i have not found it on the documentation @@ -252,14 +252,14 @@ def find_duplicates(self, name: str, mac: str) -> collections.abc.Iterable[str]: yield i.id def get_ip( - self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str + self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str ) -> str: net_info = self.api.get_server_info(vmid).validated().addresses return '' if not net_info else net_info[0].ip def get_mac( self, - caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], + caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str, *, for_unique_id: bool = False, @@ -288,19 +288,19 @@ def get_mac( return '' def is_running( - self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str + self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str ) -> bool: return self.api.get_server_info(vmid).validated().power_state.is_running() def start( - self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str + self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str ) -> None: if self.api.get_server_info(vmid).validated().power_state.is_running(): return self.api.start_server(vmid) def stop( - self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str + self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str ) -> None: if self.api.get_server_info(vmid).validated().power_state.is_stopped(): return @@ -311,13 +311,13 @@ def stop( # We can anyway delete de machine even if it is not stopped def reset( - self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str + self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str ) -> None: # Default is to stop "hard" return self.stop(caller_instance, vmid) def delete( - self, caller_instance: typing.Optional['DynamicUserService | DynamicPublication'], vmid: str + self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str ) -> None: """ Removes the machine, or queues it for removal, or whatever :) @@ -339,7 +339,7 @@ def execute_delete(self, vmid: str) -> None: # default is_deleted is fine, returns True always def make_template( - self, template_name: str, description: typing.Optional[str] = None + self, template_name: str, description: str | None = None ) -> openstack_types.SnapshotInfo: # First, ensures that volume has not any running instances # if self.api.getVolume(self.volume.value)['status'] != 'available': diff --git a/src/uds/services/OpenStack/service_fixed.py b/src/uds/services/OpenStack/service_fixed.py index 83ce6bad6..da55d21c0 100644 --- a/src/uds/services/OpenStack/service_fixed.py +++ b/src/uds/services/OpenStack/service_fixed.py @@ -48,7 +48,7 @@ from .provider import OpenStackProvider from .provider_legacy import OpenStackProviderLegacy - AnyOpenStackProvider: typing.TypeAlias = typing.Union[OpenStackProvider, OpenStackProviderLegacy] + AnyOpenStackProvider: typing.TypeAlias = OpenStackProvider | OpenStackProviderLegacy logger = logging.getLogger(__name__) @@ -105,7 +105,7 @@ class OpenStackServiceFixed(FixedService): # pylint: disable=too-many-public-me prov_uuid = gui.HiddenField() - _api: typing.Optional['client.OpenStackClient'] = None + _api: 'client.OpenStackClient | None' = None @property def api(self) -> 'client.OpenStackClient': @@ -164,7 +164,7 @@ def enumerate_assignables(self) -> collections.abc.Iterable[types.ui.ChoiceItem] ] def get_and_assign(self) -> str: - found_vmid: typing.Optional[str] = None + found_vmid: str | None = None try: with self._assigned_access() as assigned: for checking_vmid in self.sorted_assignables_list(): From b221d0e4cc3feb271db2421508814af7a13944d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janier=20Rodr=C3=ADguez?= Date: Fri, 5 Jun 2026 16:58:00 +0200 Subject: [PATCH 3/6] style(openstack): use PEP 604 union syntax in provider and helpers --- src/uds/services/OpenStack/helpers.py | 2 +- src/uds/services/OpenStack/provider.py | 4 ++-- src/uds/services/OpenStack/provider_legacy.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/uds/services/OpenStack/helpers.py b/src/uds/services/OpenStack/helpers.py index d9616620a..2b284971a 100644 --- a/src/uds/services/OpenStack/helpers.py +++ b/src/uds/services/OpenStack/helpers.py @@ -48,7 +48,7 @@ def get_api(parameters: dict[str, str]) -> tuple[client.OpenStackClient, bool]: from .provider import OpenStackProvider provider = typing.cast( - typing.Union[OpenStackProviderLegacy, OpenStackProvider], + OpenStackProviderLegacy | OpenStackProvider, models.Provider.objects.get(uuid=parameters['prov_uuid']).get_instance(), ) diff --git a/src/uds/services/OpenStack/provider.py b/src/uds/services/OpenStack/provider.py index 5dd56b146..e59aa6c55 100644 --- a/src/uds/services/OpenStack/provider.py +++ b/src/uds/services/OpenStack/provider.py @@ -202,7 +202,7 @@ class OpenStackProvider(ServiceProvider): legacy = False # Own variables - _api: typing.Optional[client.OpenStackClient] = None + _api: client.OpenStackClient | None = None def initialize(self, values: 'types.core.ValuesType' = None) -> None: """ @@ -220,7 +220,7 @@ def initialize(self, values: 'types.core.ValuesType' = None) -> None: ) def api( - self, projectid: typing.Optional[str] = None, region: typing.Optional[str] = None + self, projectid: str | None = None, region: str | None = None ) -> client.OpenStackClient: projectid = projectid or self.project_id.value or None region = region or self.region.value or None diff --git a/src/uds/services/OpenStack/provider_legacy.py b/src/uds/services/OpenStack/provider_legacy.py index 9d5736472..7f496cc1e 100644 --- a/src/uds/services/OpenStack/provider_legacy.py +++ b/src/uds/services/OpenStack/provider_legacy.py @@ -175,7 +175,7 @@ class OpenStackProviderLegacy(ServiceProvider): legacy = True # Own variables - _api: typing.Optional[client.OpenStackClient] = None + _api: client.OpenStackClient | None = None def initialize(self, values: 'types.core.ValuesType') -> None: """ @@ -187,9 +187,9 @@ def initialize(self, values: 'types.core.ValuesType') -> None: self.timeout.value = validators.validate_timeout(self.timeout.value) def api( - self, projectid: typing.Optional[str] = None, region: typing.Optional[str] = None + self, projectid: str | None = None, region: str | None = None ) -> client.OpenStackClient: - proxies: typing.Optional[dict[str, str]] = None + proxies: dict[str, str] | None = None if self.https_proxy.value.strip(): proxies = {'https': self.https_proxy.value} return client.OpenStackClient( From f40da80870d60faca19ac2ea4a6b7d113772e139 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janier=20Rodr=C3=ADguez?= Date: Mon, 8 Jun 2026 14:18:33 +0200 Subject: [PATCH 4/6] refactor(openstack): trim redundant get_mac comments to essentials --- src/uds/services/OpenStack/service.py | 7 +++---- src/uds/services/OpenStack/service_fixed.py | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/uds/services/OpenStack/service.py b/src/uds/services/OpenStack/service.py index 5f0d70fea..b20cd01e4 100644 --- a/src/uds/services/OpenStack/service.py +++ b/src/uds/services/OpenStack/service.py @@ -269,17 +269,16 @@ def get_mac( try: net_info = self.api.get_server_info(vmid).validated().addresses except exceptions.services.generics.NotFoundError: - return '' # not created yet + return '' if net_info and net_info[0].mac: return net_info[0].mac - # 'addresses' is empty when OpenStack does not manage addressing (external DHCP); - # the Neutron port still carries the mac. Swallow any API error (404/403/transient) - # here too: it must never reach the state checker. + # 'addresses' is empty under external DHCP, but the Neutron port still carries the mac. try: ports = self.api.list_ports(device_id=vmid) except exceptions.services.generics.Error as e: + # never let a transient/403 error reach the state checker: '' just retries logger.warning('Listing Neutron ports for %s failed, mac unresolved: %s', vmid, e) return '' for port in ports: diff --git a/src/uds/services/OpenStack/service_fixed.py b/src/uds/services/OpenStack/service_fixed.py index da55d21c0..6a5ceca65 100644 --- a/src/uds/services/OpenStack/service_fixed.py +++ b/src/uds/services/OpenStack/service_fixed.py @@ -206,11 +206,11 @@ def get_mac(self, vmid: str) -> str: if net_info and net_info[0].mac: return net_info[0].mac - # external DHCP: 'addresses' is empty, fall back to the Neutron port mac. - # Swallow any API error (404/403/transient) so it never reaches the caller. + # 'addresses' is empty under external DHCP, but the Neutron port still carries the mac. try: ports = self.api.list_ports(device_id=vmid) except exceptions.services.generics.Error as e: + # never let a transient/403 error reach the caller: '' just retries logger.warning('Listing Neutron ports for %s failed, mac unresolved: %s', vmid, e) return '' for port in ports: From 29f734e88f813d26b182c2a11aeec17ba6a2dc54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janier=20Rodr=C3=ADguez?= Date: Wed, 10 Jun 2026 16:08:08 +0200 Subject: [PATCH 5/6] fix: avoid IndexError in OpenStack op_create_checker under external DHCP op_create_checker bypassed the guarded get_mac/get_ip and read server_info.addresses[0] directly. Under external DHCP, Nova leaves 'addresses' empty, so addresses[0] raised IndexError and pushed the userservice to ERROR. Reuse the guarded service methods instead: get_mac resolves the mac via the Neutron port and get_ip returns '' (the real ip is learned later from the actor). --- src/uds/services/OpenStack/deployment.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uds/services/OpenStack/deployment.py b/src/uds/services/OpenStack/deployment.py index 7d7889749..5c49bfad7 100644 --- a/src/uds/services/OpenStack/deployment.py +++ b/src/uds/services/OpenStack/deployment.py @@ -169,10 +169,11 @@ def op_create_checker(self) -> types.states.TaskState: """ # Checks if machine has been created # Should have a mac address assigned and be running - if self.service().get_mac(self, self._vmid) and self.service().is_running(self, self._vmid): - server_info = self.service().api.get_server_info(self._vmid).validated() - self._mac = server_info.addresses[0].mac - self._ip = server_info.addresses[0].ip + mac = self.service().get_mac(self, self._vmid) + if mac and self.service().is_running(self, self._vmid): + self._mac = mac + # external DHCP leaves Nova 'addresses' empty; the guarded getters avoid indexing it + self._ip = self.service().get_ip(self, self._vmid) return types.states.TaskState.FINISHED return types.states.TaskState.RUNNING From 5ebd5cc074261bb0c8cf3fb2b6a81aa2a60d4889 Mon Sep 17 00:00:00 2001 From: aschumann-virtualcable Date: Mon, 15 Jun 2026 16:51:53 +0200 Subject: [PATCH 6/6] feat(openstack): add get_server_mac method for MAC address resolution --- .../services/OpenStack/openstack/client.py | 22 ++++++++++++++++++ src/uds/services/OpenStack/service.py | 23 +------------------ src/uds/services/OpenStack/service_fixed.py | 22 ++---------------- 3 files changed, 25 insertions(+), 42 deletions(-) diff --git a/src/uds/services/OpenStack/openstack/client.py b/src/uds/services/OpenStack/openstack/client.py index ddf6bea30..8f20b6a71 100644 --- a/src/uds/services/OpenStack/openstack/client.py +++ b/src/uds/services/OpenStack/openstack/client.py @@ -611,6 +611,28 @@ def get_server_info(self, server_id: str, **kwargs: typing.Any) -> openstack_typ error_message='Get Server information', ) return openstack_types.ServerInfo.from_dict(r.json()['server']) + + @decorators.cached(prefix='svr_mac', timeout=consts.cache.SHORT_CACHE_TIMEOUT, key_helper=cache_key_helper) + def get_server_mac(self, server_id: str, **kwargs: typing.Any) -> str: + # Returning '' makes the state checker retry instead of forcing the userservice + # to ERROR, so degrade to it whenever the mac cannot be resolved yet. + net_info = self.get_server_info(server_id).validated().addresses + + if net_info and net_info[0].mac: + return net_info[0].mac + + # 'addresses' is empty under external DHCP, but the Neutron port still carries the mac. + try: + ports = self.list_ports(device_id=server_id) + except exceptions.services.generics.Error as e: + # never let a transient/403 error reach the state checker: '' just retries + logger.debug('Listing Neutron ports for %s failed, mac unresolved: %s', server_id, e) + return '' + for port in ports: + if port.mac_address: + return port.mac_address + return '' + @decorators.cached(prefix='vol', timeout=consts.cache.SHORTEST_CACHE_TIMEOUT, key_helper=cache_key_helper) def get_volume_info(self, volume_id: str, **kwargs: typing.Any) -> openstack_types.VolumeInfo: diff --git a/src/uds/services/OpenStack/service.py b/src/uds/services/OpenStack/service.py index b20cd01e4..a7cd0f30e 100644 --- a/src/uds/services/OpenStack/service.py +++ b/src/uds/services/OpenStack/service.py @@ -37,7 +37,6 @@ from django.utils.translation import gettext_noop as _ from uds.core import types -from uds.core import exceptions from uds.core.services.generics.dynamic.service import DynamicService from uds.core.util import validators from uds.core.ui import gui @@ -264,27 +263,7 @@ def get_mac( *, for_unique_id: bool = False, ) -> str: - # Returning '' makes the state checker retry instead of forcing the userservice - # to ERROR, so degrade to it whenever the mac cannot be resolved yet. - try: - net_info = self.api.get_server_info(vmid).validated().addresses - except exceptions.services.generics.NotFoundError: - return '' - - if net_info and net_info[0].mac: - return net_info[0].mac - - # 'addresses' is empty under external DHCP, but the Neutron port still carries the mac. - try: - ports = self.api.list_ports(device_id=vmid) - except exceptions.services.generics.Error as e: - # never let a transient/403 error reach the state checker: '' just retries - logger.warning('Listing Neutron ports for %s failed, mac unresolved: %s', vmid, e) - return '' - for port in ports: - if port.mac_address: - return port.mac_address - return '' + return self.api.get_server_mac(vmid) def is_running( self, caller_instance: 'DynamicUserService | DynamicPublication | None', vmid: str diff --git a/src/uds/services/OpenStack/service_fixed.py b/src/uds/services/OpenStack/service_fixed.py index 6a5ceca65..8e516315e 100644 --- a/src/uds/services/OpenStack/service_fixed.py +++ b/src/uds/services/OpenStack/service_fixed.py @@ -34,7 +34,7 @@ from django.utils.translation import gettext_noop as _ -from uds.core import exceptions, types +from uds.core import types from uds.core.services.generics.fixed.service import FixedService from uds.core.ui import gui @@ -198,25 +198,7 @@ def get_and_assign(self) -> str: def get_mac(self, vmid: str) -> str: # Returning '' lets the caller retry instead of crashing when the mac cannot be # resolved yet (machine missing, or external DHCP leaving 'addresses' empty). - try: - net_info = self.api.get_server_info(vmid).addresses - except exceptions.services.generics.NotFoundError: - return '' - - if net_info and net_info[0].mac: - return net_info[0].mac - - # 'addresses' is empty under external DHCP, but the Neutron port still carries the mac. - try: - ports = self.api.list_ports(device_id=vmid) - except exceptions.services.generics.Error as e: - # never let a transient/403 error reach the caller: '' just retries - logger.warning('Listing Neutron ports for %s failed, mac unresolved: %s', vmid, e) - return '' - for port in ports: - if port.mac_address: - return port.mac_address - return '' + return self.api.get_server_mac(vmid) or '' def get_ip(self, vmid: str) -> str: return self.api.get_server_info(vmid).addresses[0].ip