Skip to content

NAS-140167 / 27.0.0-BETA.1 / Fix TNC sync_interface_ips empty IPs and repeated concurrent calls #18448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sonicaj wants to merge 1 commit into
base: master
Choose a base branch
from
+141 −38
Open

NAS-140167 / 27.0.0-BETA.1 / Fix TNC sync_interface_ips empty IPs and repeated concurrent calls #18448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 49 additions & 38 deletions src/middlewared/middlewared/plugins/truenas_connect/hostname.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

logger = logging.getLogger('truenas_connect')

_sync_lock = asyncio.Lock()


class TNCHostnameService(Service):

Expand Down Expand Up @@ -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):
"""
Expand Down
92 changes: 92 additions & 0 deletions src/middlewared/middlewared/pytest/unit/plugins/test_truenas_connect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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."""
Expand Down
Loading