diff --git a/src/middlewared/middlewared/plugins/truenas_connect/hostname.py b/src/middlewared/middlewared/plugins/truenas_connect/hostname.py index 3e4b0c9235973..9444772cd97e8 100644 --- a/src/middlewared/middlewared/plugins/truenas_connect/hostname.py +++ b/src/middlewared/middlewared/plugins/truenas_connect/hostname.py @@ -11,6 +11,8 @@ logger = logging.getLogger('truenas_connect') +_sync_lock = asyncio.Lock() + class TNCHostnameService(Service): @@ -52,46 +54,55 @@ async def register_system_config(self, websocket_port: int) -> dict: raise CallError(str(e)) async def sync_interface_ips(self, event_details=None): - tnc_config = await self.middleware.call('tn_connect.config') - - # Get interface IPs based on use_all_interfaces flag - if tnc_config['use_all_interfaces']: - interfaces_ips = await self.middleware.call('tn_connect.get_all_interface_ips') - else: - interfaces_ips = await self.middleware.call('tn_connect.get_interface_ips', tnc_config['interfaces']) - - try: - cached_ips = await self.middleware.call('cache.get', TNC_IPS_CACHE_KEY) - except KeyError: - skip_syncing = False - else: - skip_syncing = set(cached_ips) == set(interfaces_ips) - - # If cached IPs are the same as current, skip syncing - if skip_syncing: - return - - if event_details: - logger.info( - 'Updating IPs for TrueNAS Connect due to %s change on interface %s', - event_details['type'], event_details['iface'], + async with _sync_lock: + tnc_config = await self.middleware.call('tn_connect.config') + + # Get interface IPs based on use_all_interfaces flag + if tnc_config['use_all_interfaces']: + interfaces_ips = await self.middleware.call('tn_connect.get_all_interface_ips') + else: + interfaces_ips = await self.middleware.call('tn_connect.get_interface_ips', tnc_config['interfaces']) + + try: + cached_ips = await self.middleware.call('cache.get', TNC_IPS_CACHE_KEY) + except KeyError: + skip_syncing = False + else: + skip_syncing = set(cached_ips) == set(interfaces_ips) + + # If cached IPs are the same as current, skip syncing + if skip_syncing: + return + + if event_details: + logger.info( + 'Updating IPs for TrueNAS Connect due to %s change on interface %s', + event_details['type'], event_details['iface'], + ) + + logger.debug('Updating TrueNAS Connect database with interface IPs: %r', ', '.join(interfaces_ips)) + await self.middleware.call( + 'datastore.update', 'truenas_connect', tnc_config['id'], { + 'interfaces_ips': interfaces_ips, + } ) - logger.debug('Updating TrueNAS Connect database with interface IPs: %r', ', '.join(interfaces_ips)) - await self.middleware.call( - 'datastore.update', 'truenas_connect', tnc_config['id'], { - 'interfaces_ips': interfaces_ips, - } - ) - - logger.debug('Syncing interface IPs for TrueNAS Connect') - try: - await self.middleware.call('tn_connect.hostname.register_update_ips') - except CallError: - logger.error('Failed to update IPs with TrueNAS Connect', exc_info=True) - else: - await self.middleware.call('cache.put', TNC_IPS_CACHE_KEY, interfaces_ips, 60 * 60) - await self.middleware.call_hook('tn_connect.hostname.updated', await self.config()) + # Skip HTTP call if no IPs available (static + dynamic combined) + # to avoid sending an empty payload that would cause a 400 error. + # Still cache the empty result to prevent retry storms from repeated netlink events. + combined_ips = tnc_config['ips'] + interfaces_ips + if not combined_ips: + await self.middleware.call('cache.put', TNC_IPS_CACHE_KEY, interfaces_ips, 60 * 60) + return + + logger.debug('Syncing interface IPs for TrueNAS Connect') + try: + await self.middleware.call('tn_connect.hostname.register_update_ips') + except CallError: + logger.error('Failed to update IPs with TrueNAS Connect', exc_info=True) + else: + await self.middleware.call('cache.put', TNC_IPS_CACHE_KEY, interfaces_ips, 60 * 60) + await self.middleware.call_hook('tn_connect.hostname.updated', await self.config()) async def handle_update_ips(self, event_type, args): """ diff --git a/src/middlewared/middlewared/pytest/unit/plugins/test_truenas_connect.py b/src/middlewared/middlewared/pytest/unit/plugins/test_truenas_connect.py index 2d8ca2c3e575f..bfc6606555056 100644 --- a/src/middlewared/middlewared/pytest/unit/plugins/test_truenas_connect.py +++ b/src/middlewared/middlewared/pytest/unit/plugins/test_truenas_connect.py @@ -4,6 +4,7 @@ from middlewared.service import ValidationErrors from middlewared.plugins.truenas_connect.update import TrueNASConnectService from middlewared.plugins.truenas_connect.hostname import TNCHostnameService +from middlewared.plugins.truenas_connect.utils import TNC_IPS_CACHE_KEY from truenas_connect_utils.status import Status @@ -617,6 +618,7 @@ async def mock_middleware_call(method, *args, **kwargs): if method == 'tn_connect.config': return { 'id': 1, + 'ips': [], 'interfaces': ['ens3'], 'use_all_interfaces': True } @@ -663,6 +665,7 @@ async def mock_middleware_call(method, *args, **kwargs): if method == 'tn_connect.config': return { 'id': 1, + 'ips': [], 'interfaces': ['ens3', 'ens4'], 'use_all_interfaces': False } @@ -694,6 +697,95 @@ async def mock_middleware_call(method, *args, **kwargs): assert not get_all_called assert get_interfaces_called + @pytest.mark.asyncio + async def test_sync_interface_ips_empty_ips_skips_http(self): + """Test that sync skips HTTP call when both static and interface IPs are empty.""" + service = TNCHostnameService(MagicMock()) + service.config = AsyncMock() + + register_called = False + cache_put_args = None + + async def mock_middleware_call(method, *args, **kwargs): + nonlocal register_called, cache_put_args + + if method == 'tn_connect.config': + return { + 'id': 1, + 'ips': [], + 'interfaces': [], + 'use_all_interfaces': True, + } + elif method == 'tn_connect.get_all_interface_ips': + return [] + elif method == 'cache.get': + raise KeyError('Key not found') + elif method == 'datastore.update': + assert args[2]['interfaces_ips'] == [] + return None + elif method == 'tn_connect.hostname.register_update_ips': + register_called = True + return {'error': None} + elif method == 'cache.put': + cache_put_args = args + return None + else: + return None + + service.middleware.call = AsyncMock(side_effect=mock_middleware_call) + service.middleware.call_hook = AsyncMock() + + await service.sync_interface_ips() + + # HTTP call should NOT have been made + assert not register_called + # Cache should still be populated to prevent retry storms + assert cache_put_args is not None + assert cache_put_args[0] == TNC_IPS_CACHE_KEY + assert cache_put_args[1] == [] + # Hook should NOT have been called + service.middleware.call_hook.assert_not_called() + + @pytest.mark.asyncio + async def test_sync_interface_ips_empty_interface_ips_with_static_ips_syncs(self): + """Test that sync proceeds when interface IPs are empty but static IPs exist.""" + service = TNCHostnameService(MagicMock()) + service.config = AsyncMock() + + register_called = False + + async def mock_middleware_call(method, *args, **kwargs): + nonlocal register_called + + if method == 'tn_connect.config': + return { + 'id': 1, + 'ips': ['192.168.1.100'], + 'interfaces': [], + 'use_all_interfaces': True, + } + elif method == 'tn_connect.get_all_interface_ips': + return [] + elif method == 'cache.get': + raise KeyError('Key not found') + elif method == 'datastore.update': + return None + elif method == 'tn_connect.hostname.register_update_ips': + register_called = True + return {'error': None} + elif method == 'cache.put': + return None + else: + return None + + service.middleware.call = AsyncMock(side_effect=mock_middleware_call) + service.middleware.call_hook = AsyncMock() + + await service.sync_interface_ips() + + # HTTP call SHOULD have been made because static IPs exist + assert register_called + @pytest.mark.asyncio async def test_register_update_ips_uses_combined_ips(self): """Test that register_update_ips uses combined IPs when no IPs provided."""